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

updating triggers for mutation listeners, and equalizer to use them #9126

Merged

Conversation

coreysyms
Copy link
Contributor

@coreysyms coreysyms commented Aug 18, 2016

Adding this as a "bug fix / feature" to develop because if it works will fix a lot of issues.
EX: #8684 or #8556

This is the implementation of mutation in addition to scroll and resize
triggers for universal plugin usage. Foundation can now fire events
when elements that are listening for mutation events or their children
change in the DOM either by element addition, element removal, or
attribute manipulation.

This can be very useful for ajax caused changes, display none on load
issues, element manipulation, and any other DOM mutation events.

To see what I mean, I added in the mutation trigger for Equalizer, see how it interacts with interchange, and a "display:none" div. And added in a fire event for Tabs as well.

This is what Foundation currently does with interchange and equalizer, and Tabs with JS modified content. (not so nice)
http://tangerineindustries.com/Foundation/feature-mutation-trigger/without_mut.html

This is with the new mutation trigger (everything lays out proper)
http://tangerineindustries.com/Foundation/feature-mutation-trigger/with_mut.html

This is the implementation of mutation in addition to scroll and resize
triggers for universal plugin usage. Foundation can now fire events
when elements that are listening for mutation events or their children
change in the DOM either by element addition, element removal, or
attribute manipulation.

This can be very useful for ajax caused changes, display none on load
issues, element manipulation, and any other DOM mutation events.
Tabs itself has no need to listen for mutation events, but it’s content
might. Trigger a mutation event to it’s listening children after the
tabs change.
@coreysyms
Copy link
Contributor Author

@zurb/yetinauts Bump, I could really use others testing to see if this breaks anything.

@kball
Copy link
Contributor

kball commented Sep 29, 2016

@coreysyms this is super cool. It took me a while and some conversation with you to get what its doing, but I think this approach will enable some amazing 'automatic', and performant responsiveness to data changes. We can build upon this for all sorts of automatic ajax content fetching, etc.

@Owlbertz and @colin-marshall you should take a look and try this out.

We need to add some testing & documentation, but I think this will be amazing.

Lets target this for 6.2.5 or 6.3.

@kball
Copy link
Contributor

kball commented Sep 30, 2016

Would this be a natural way to address #8955 as well?

@coreysyms
Copy link
Contributor Author

coreysyms commented Sep 30, 2016

@kball

Would this be a natural way to address #8955 as well?

No. The issue seems to be with Orbit calculating images height on the initial page load, whether it be a primed cache or not Orbit can't seem to consistently calculate the image height. This listener method is intended for elements that were added, removed, or manipulated after load.

Now if Interchange was utilized for the images in the Orbit carousel, Interchange could trigger a window mutate event because it removes and or adds images to the DOM after initial page load, so if Orbit had a mutate listener, it could fire a re-calc in that instance.

Also for what it's worth, I can't replicate that issue, tried windows 7, 10 mac, nothing; loads fine every time.

@kball
Copy link
Contributor

kball commented Oct 25, 2016

@coreysyms @Owlbertz I'm super excited about the potential for this... what do we need to get it past the line?

@Owlbertz
Copy link
Contributor

Looks good to me on the first glance.

@kball
Copy link
Contributor

kball commented Oct 31, 2016

@coreysyms testing this out... THIS IS AWESOME! :) One thing I notice is that the mutate observers don't appear to be polyfilled back for IE9/IE10 in the same way scroll & resize were... does it make sense to do so? Or is the performance impact too large?

@coreysyms
Copy link
Contributor Author

coreysyms commented Oct 31, 2016

@kball

mutate observers don't appear to be polyfilled back for IE9/IE10 in the same way scroll & resize were...

The mutation observer for scroll and resize is effectively replacing thefor loop, it is meant as a more efficient way to trigger the scroll or resize event. (we are cleverly using a mutation observer)

The mutation observer for mutation events is, well, what it is, and no fallback is needed... which brings us to the fork in the road of IE9/10. There really isn't anything that we can do in this instance for those browsers. However, since it is a "trigger event", it is non-destructive, and IE9/10 can continue to exist in its terrible environment. IE11 is supported.

I do realize the confusion of using a mutation observer to observer mutation among other things, I have to re-read my code and what I'm typing as to not confuse myself let alone others.

@kball
Copy link
Contributor

kball commented Nov 1, 2016

@coreysyms I think we can merge this then... I tested it, and other than the aforementioned ie9/10, it all is looking good. I think reveal is another potential target, and there's a ton of possible applications.

Do you have any reasons this is not ready to merge?

@coreysyms
Copy link
Contributor Author

I agree, Reveal would be greatly complemented by this. I feel good about this PR. I'm good for merge!

Corey Snyder

On Oct 31, 2016, at 8:32 PM, Kevin Ball notifications@github.com wrote:

@coreysyms I think we can merge this then... I tested it, and other than the aforementioned ie9/10, it all is looking good. I think reveal is another potential target, and there's a ton of possible applications.

Do you have any reasons this is not ready to merge?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@coreysyms
Copy link
Contributor Author

Perhaps we should roadmap the plugins to update for usage? I've touched tabs. Interchange and equalizer in this PR.

The good news is. It shouldn't take long as the plugin just needs to "register" and hit whatever function it already has to reflow itself.

Accordion. Would be another plugin that "display none" effects.

So once this is merged. We can start leveraging it.

Corey Snyder

On Oct 31, 2016, at 10:31 PM, Corey Snyder corey@tangerineindustries.com wrote:

I agree, Reveal would be greatly complemented by this. I feel good about this PR. I'm good for merge!

Corey Snyder

On Oct 31, 2016, at 8:32 PM, Kevin Ball notifications@github.com wrote:

@coreysyms I think we can merge this then... I tested it, and other than the aforementioned ie9/10, it all is looking good. I think reveal is another potential target, and there's a ton of possible applications.

Do you have any reasons this is not ready to merge?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

so no errors are thrown to the console, also, no need for a timer on mutate, it is only fired once.
@coreysyms
Copy link
Contributor Author

@kball just made a quick change to wrap the mutate listener in an if so IE9/10 does not throw an error in the console window. I'm ready to let it run wild.

@coreysyms
Copy link
Contributor Author

odd, not sure what the problem is with Tabs and Travis

@kball kball merged commit 2445f76 into foundation:develop Nov 1, 2016
@kball
Copy link
Contributor

kball commented Nov 1, 2016

Found it. Was a legit bug (missing var), but I fixed it. Merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants