Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Featuretest pagevisibility #700

Merged
merged 1 commit into from
Oct 5, 2012
Merged

Featuretest pagevisibility #700

merged 1 commit into from
Oct 5, 2012

Conversation

helgri
Copy link
Contributor

@helgri helgri commented Sep 23, 2012

Added simple test for PageVisibility API support

Tested w/ Chrome Canary, FIrefox 15, Safari 6, IE9/10

paulirish
paulirish reviewed Sep 23, 2012
View reviewed changes
feature-detects/pagevisibility-api.js
@@ -0,0 +1,12 @@
Modernizr.addTest('pagevisibility', function(){
for(var i = 0, len = Modernizr._domPrefixes.length; i < len; ++i) {
if(typeof document[Modernizr._domPrefixes[i].toLowerCase() + "Hidden"] !== "undefined") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add one of these before this:

var prefix = Modernizr._domPrefixes[i].toLowerCase();

it'll look a lot better

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (

@paulirish
Copy link
Member

thanks. was just talking to @ebidel about how we didnt have this, surprisingly.

@paulirish
Copy link
Member

Could you quickly test in ie8 too?

sindresorhus
sindresorhus reviewed Sep 23, 2012
View reviewed changes
feature-detects/pagevisibility-api.js
@@ -0,0 +1,12 @@
Modernizr.addTest('pagevisibility', function(){
for(var i = 0, len = Modernizr._domPrefixes.length; i < len; ++i) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for (

@sindresorhus
Copy link
Contributor

and single-quotes

@helgri
Copy link
Contributor Author

helgri commented Sep 23, 2012

added your change requests. test on IE8 works fine.

@joemarini
Copy link

Doh! Just wrote one myself, didn't realize I was duplicating effort.

@sindresorhus sindresorhus mentioned this pull request Sep 23, 2012
Closed
@helgri
Copy link
Contributor Author

helgri commented Sep 26, 2012

is something still missing?

@paulirish
Copy link
Member

yeah that slice of pie you promised to get me to accept your PR instead of
joe's!

sindresorhus
sindresorhus reviewed Sep 28, 2012
View reviewed changes
feature-detects/pagevisibility-api.js
// http://dvcs.w3.org/hg/webperf/raw-file/tip/specs/PageVisibility/Overview.html

Modernizr.addTest('pagevisibility', function(){
for (var i = 0, len = Modernizr._domPrefixes.length; i < len; ++i) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that it makes a difference, but i++ is clearer.

@helgri
Copy link
Contributor Author

helgri commented Sep 28, 2012

Paul, the cake is a lie, but here you go :)


                        #,
                        ###
                       ## ##
                      ##  ##
                       ####
                         :
                        #####
                       ######
                       ##  ##
                       ##  ##
                       ##  ##
                       ##  ##########
                       ##  #############
                  #######  ###############
              #############################
        .###################################
       #####################################;
       ##                                 ##.
       ##                                 ##
       #####################################
       ##                                 ##
       ##                                 ##
       ##                                 ###
    #####                                 #####
   ### ##################################### ###
  ###  ##                                 ##  ###
  ##   ## ,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,, ##   ##
   ##  #####################################  ##
    ##                                       ##
     ####                                 ####
       ######                         ######
          ###############################

@helgri
Copy link
Contributor Author

helgri commented Sep 28, 2012

made ++i to i++

@helgri
Copy link
Contributor Author

helgri commented Sep 28, 2012

@sindresorhus test is now using hidden in document

I think Modernizr.prefixed() doesn't work, because the value of document.hidden can be false by itself, does it?

@ryanseddon
Copy link
Member

@helgri if you do Modernizr.prefixed("hidden", document, false) that will either return [prefix]Hidden or false. The last argument will force a string rather than try and bind.

@helgri
Copy link
Contributor Author

helgri commented Oct 1, 2012

@ryanseddon Oh, that's really convenient...

@ryanseddon
Copy link
Member

@helgri looks good can you squash your commits into one please. If you're not sure how to do that check out this post on commit squashing

@helgri
Copy link
Contributor Author

helgri commented Oct 4, 2012

squashed like hell :)

ryanseddon added a commit that referenced this pull request Oct 5, 2012
@ryanseddon
Merge pull request #700 from helgri/featuretest-pagevisibility
def11e6 
Feature detect for Page Visibility API
@ryanseddon ryanseddon merged commit def11e6 into Modernizr:master Oct 5, 2012
@ryanseddon
Copy link
Member

Thanks @helgri 👍

@alibby251 alibby251 mentioned this pull request Jan 13, 2013
Closed
patrickkettner pushed a commit to patrickkettner/Modernizr that referenced this pull request Feb 22, 2015
@ryanseddon
Merge pull request Modernizr#700 from helgri/featuretest-pagevisibility
245b6cb 
Feature detect for Page Visibility API
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@helgri @paulirish @sindresorhus @joemarini @ryanseddon