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.

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

changed.zf.mediaquery improvements #8644

Merged
merged 2 commits into from
May 11, 2016

Conversation

designerno1
Copy link
Contributor

this.current = newSize; (should be set before window triggered because if something goes wrong changed.zf.mediaquery fires constantly, because current is neer set

this.current = newSize; (should be set before window triggered because if something goes wrong changed.zf.mediaquery fires constantly)
// Change the current media query
this.current = newSize;

// Broadcast the media query change on the window
$(window).trigger('changed.zf.mediaquery', [newSize, this.current]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Triggering it here means this.current is newSize, rather than the old size. I think moving this to after the set is reasonable, but if we do we should capture the previous value of this.current and pass it as the second value.

@designerno1
Copy link
Contributor Author

ah ok i see, the only thing this PR was for i ran into an error on my .on('changed.zf.mediaquery',function(){})
and wondering afer the error why changed.zf.mediaquery is fired several times, because this.current didn't gets a update because of the error

@kball
Copy link
Contributor

kball commented May 4, 2016

@designerno1 do you have time to clean up this PR to address the problem raised in the comment? I'd love to get this fix in place

@kball
Copy link
Contributor

kball commented May 11, 2016

👍

@kball kball merged commit 918e8d6 into foundation:develop May 11, 2016
@designerno1 designerno1 deleted the util-mediaquery branch May 11, 2016 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants