frend / frend.co

Frend — A collection of accessible, modern front-end components.
http://frend.co
MIT License
635 stars 25 forks source link

Add jQuery like closest feature to tabs component #80

Closed dnwhte closed 8 years ago

adamduncan commented 8 years ago

Thanks @danwhite85. We've been talking more about things like this as part of the NodeList #64 update. We'd obviously like to get away from the DOM traversing that's currently present in a few components. closest and matches polyfills would help a lot with that.

Tooltip, Tabs and Off-Canvas will all require something along the same lines. What are your thoughts on the best way of sharing polyfills between components? Ideally, we're keeping the compiled JS for each as lean as possible, although there will be some unavoidable repetition across components.

adamduncan commented 8 years ago

On a minor side note, we resolved the e.target issue with nested children for Off-Canvas yesterday #76. Intending to roll the same e.currentTarget fix into Tabs etc. That could be an alternative to the closest amend you've made on line 156? Thanks again.

dnwhte commented 8 years ago

Nice. e.CurrentTarget is a better solution.

I'm not sure on the best way of sharing polyfills. Perhaps having a polyfills.js distributed with the components that need a polyfill. Obviously there'll some extraneous code if they're just using one component, but it'd avoid duplication if they use multiple.

adamduncan commented 8 years ago

Cool, thanks for updating that mate.

We've just rolled the NodeList amends through for PR #82. There might be a minor tweak required to get this branch back on track, sorry.

Also, thinking about the polyfills over the weekend. The matches prefixer in Off-canvas kind of does the trick currently. We could do more of a native when available and fallback approach? E.g. matches-selector. Still in two minds as whether to augment the native Element.prototype, or play it safe and use a helper util like Tabs closest(). Thoughts?

dnwhte commented 8 years ago

Playing it safe and using a helper util feels like it's more suited to me. You have a set of isolated components. If the goal is to keep it that way then a helper util makes more sense to me. If in the long-term you plan on having some core.js for all components to share then maybe augmenting the native element makes more sense.

adamduncan commented 8 years ago

Yeah, definitely in agreement. That way these components can be included in projects, side-effect-free. That sounds good. Shall we branch off again (or merge gh-pages updates into this PR) and try this approach on Tabs? Once we've got a proof of concept, we can roll it through other components that make use of matches and closest.

thomasdigby commented 8 years ago

Hey @danwhite85, we've merged an update into tabs v1.0.5 #87 with closest/matches methods. As we already had a _closest util in the offcanvas we decided to use that for now, although adding polyfills separately is still something that we need to look into. Thanks for the feedback. 👍