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

Merged
merged 4 commits into from Nov 1, 2016

Projects

None yet

3 participants

@coreysyms
Contributor
coreysyms commented Aug 18, 2016 edited

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

coreysyms added some commits Aug 18, 2016
@coreysyms coreysyms updating triggers for mutation listeners, and equalizer to use them
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.
e282a16
@coreysyms coreysyms forgot a comma 70fc40e
@coreysyms coreysyms Include tabs in mutation triggers
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.
bdda0b0
@coreysyms
Contributor

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

@kball
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
Contributor
kball commented Sep 30, 2016

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

@coreysyms
Contributor
coreysyms commented Sep 30, 2016 edited

@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
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
Member

Looks good to me on the first glance.

@kball
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
Contributor
coreysyms commented Oct 31, 2016 edited

@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
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
Contributor

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
Contributor

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.

@coreysyms coreysyms Just a quick IE9 IE10 wrap
so no errors are thrown to the console, also, no need for a timer on mutate, it is only fired once.
2445f76
@coreysyms
Contributor

@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
Contributor

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

@kball kball merged commit 2445f76 into zurb:develop Nov 1, 2016

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
@kball
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