Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Add support for match -> unmatch transition, fix for #27 #28

Merged
merged 2 commits into from

4 participants

@WickyNilliams

I always get this wrong, always forget that github creates an issue when you make a PR, so please forgive my incompetence here!

Anyway, this is a fix for the issue I raised previously, #27. Have tested in IE9 and can confirm it works a charm. If there are code style issues or anything let me know and I will endeavour to fix.

Cheers,
Nick

@WickyNilliams

Please can someone look at this as soon as is convenient? I'm relying on this fix (or any other fix for the aforementioned issue 27) being merged in to support the next iteration of my library and currently this is a blocker.

Would be very much appreciated

@scottjehl
Collaborator

This looks good to me. How bout you, @paulirish ?

@knightdr knightdr referenced this pull request in weblinc/picture
Closed

rewrite so it works without media.js #7

@attiks

We did something similar for Drupal 8 and sovled it a bit different, see http://drupalcode.org/project/drupal.git/blob/refs/heads/8.x:/core/misc/matchmedia.js#l75

@WickyNilliams

@attiks I was literally trying to make the smallest change possible to fix.

Thanks @scottjehl, now where is that elusive @paulirish when you need him?!

@paulirish
Owner

looking.. (sorry i missed the pings)

@paulirish
Owner

i like it. Much better than putting all that logic inside of the condition. :)

Add a code comment that says something like "fire the callback when transitioning either in or out of the matched state."

then it's LGTM.

cheers

@WickyNilliams

Understandable, you're probably overwhelmed by notifications.

Code clarity above all else :) I had an internal debate about such a comment but thought the variable naming made it clear enough. Happy to comply though.

@WickyNilliams

*ahem* :)

@scottjehl
Collaborator

Tested it. Looks good to me. Pulling in now. Thanks @WickyNilliams!

@scottjehl scottjehl merged commit 6b26010 into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 7, 2013
  1. @WickyNilliams
Commits on Jan 12, 2013
  1. @WickyNilliams

    add explanatory comment

    WickyNilliams authored
This page is out of date. Refresh to see the latest.
Showing with 7 additions and 3 deletions.
  1. +7 −3 matchMedia.addListener.js
View
10 matchMedia.addListener.js
@@ -10,8 +10,12 @@
last = false,
timer,
check = function(){
- var list = oldMM( q );
- if( list.matches && !last ){
+ var list = oldMM( q ),
+ unmatchToMatch = list.matches && !last,
+ matchToUnmatch = !list.matches && last;
+
+ //fire callbacks only if transitioning to or from matched state
+ if( unmatchToMatch || matchToUnmatch ){
for( var i =0, il = listeners.length; i< il; i++ ){
listeners[ i ].call( ret, list );
}
@@ -40,4 +44,4 @@
return ret;
};
}
-}());
+}());
Something went wrong with that request. Please try again.