Filtering styles and animation improvements

[tyleralsbury]

Why are these changes introduced?

A few more of the points in #100 are fixed here

• Animation when opening/closing mobile filters
• Checkbox styles (Excluded hover state)
• Sorting now syncs between desktop and mobile
• Small refactor to remove the need for any logic in the JS for the reset, clear all, and active filter remove buttons

What approach did you take?

The only thing where the approach changed significantly was the reset, clear all, and active filter buttons. They are now using a custom element so that when they re-render they automatically get their events bound instead of needing to call a function to loop and bind the events.

Demo links

• Store

Checklist

• Followed theme code principles
• Linted with Theme Check
• Tested on mobile
• Tested on multiple browsers
• Tested for accessibility

[tauthomas01]

Looks good, desktop and mobile.

I left one small question

[ludoboludo]

Works well! A few comments but some not related to this PR but the general functionality.

Also I noticed if you open a filter summary then click on the hamburger menu, they overlap: screenshot.

[tyleralsbury]

Finally got around to your comments, @ludoboludo - thanks for the review. Ready for round 2.

[ludoboludo]

LGTM, just left one comment that can be tweaked before merging.

[ludoboludo]

I wonder if we should handle the click outside to close the menus with JS. And keep this CSS approach for a no JS experience.
The drawer menu doesn't seem to have a click outside closes the menu kinda thing and when using the facets__summary it could reduce the amount of clicks to open others.
But might be overkill 🤷 Just think it would like a smoother experience.

[tyleralsbury]

I think this might be too much complexity on the JS side, personally. There's also more risk of a "time-warp" sort of effect if we do it this way. Remember that when filters are changed with JS enabled, the other details elements get re-rendered. If we were to re-render an element after they've clicked the element to open it, then it will close when it re-renders.

We might be able to get around that concern by re-rendering the contents of the details element instead of the whole element, which might be one option. 🤔

We can add it to the issue as an idea and think about it.

[ludoboludo]

It also needs to be updated for input.mobile-facets__checkbox. When on my iPad, it looks fine on a large screen now but when it goes to the drawer I get the same thing. It's not ideal to tweak it like this as now the checkbox receiving focus isn't quite aligned with the svg but 🤷

So from what I was testing it looks like left 4.31rem is what worked to hide the default checkboxes behind the svg on mobile for iOS.

[tyleralsbury]

Hmmm... Ok, for now I'm gonna revert it to 0.8rem and then see if I can find some trick for iOS. I don't want the focus ring to be off-center on other browsers just cause iOS is being awkward. I'll add it to the issue and try to tackle it later.

[ludoboludo]

Sounds good 👍

[ludoboludo]

🚀