Opened 7 years ago
Closed 7 years ago
#42212 closed defect (bug) (fixed)
Customize Themes: Unstick open filters
Reported by: | melchoyce | Owned by: | celloexpressions |
---|---|---|---|
Milestone: | 4.9 | Priority: | normal |
Severity: | normal | Version: | 4.9 |
Component: | Customize | Keywords: | needs-testing needs-patch |
Focuses: | ui | Cc: |
Description
When you open the theme filters, they "stick" so when you scroll, there amount of vertical space you have to view themes is significantly decreased. We should unstick the filters when open so that when you scroll, your theme isn't blocked by them. (The search bar/filter button container should still remain fixed.)
Attachments (8)
Change History (36)
#3
@
7 years ago
Added a patch that unticks the filters, allowing for scrolling to happen normally.
Also updated the JS toggling to facilitate easier access to the filters.
- The filters are now only accessible at the top of the screen. Clicking the "Filter themes" button will automatically scroll you to the top of the page if you are not already there.
- If you have opened the filters, but scrolled down the page, clicking the "Filter themes" button will leave the filters open, but scroll you up to them. Clicking a second time will close them.
- If you have not opened the filters, clicking the button will scroll you up to the top and open the filters.
Also updated the positioning of the "Filter themes" button on mobile, since it looked a little out of place
#4
@
7 years ago
- Keywords has-patch added; needs-patch removed
- Status changed from assigned to reviewing
#5
@
7 years ago
2 additions in the latest patch:
- fixes offset for filter drawer on large screens
- fixes unrelated issue w/ screens > 1670px wide, where the themes container was overlapping the left sidebar.
- The left sidebar is set to 18%, with a min-width of 300px.
- The themes container was set to (100% - 300px), which breaks once you reach 1670px and the left sidebar grows larger than it's min-width.
An additional note here: The theme container offset (used to make room for the filter drawer) is being set on load using js. Resizing the browser can break the display on the filter drawer as a result, with themes overlapping it.
See https://github.com/WordPress/WordPress/blob/master/wp-admin/js/customize-controls.js#L1812
Is this something we want to address?
#6
@
7 years ago
Another day, another patch
- Fixes offset for filter drawer on very small screens.
- Updates js to calculate filter drawer size each time the drawer is opened, which allows for the display to work properly after resizing your browser window.
- Updates filter group legend, increasing top padding to match bottom padding. This also updates the filter group display at
/wp-admin/theme-install.php
(screenshots below)
Before / After
Mobile display with updated legend styles
#8
@
7 years ago
Okay, I was wrong about the spacing around layout/features/etc., we should probably revert those to what they were previously.
#10
follow-up:
↓ 11
@
7 years ago
@melchoyce I iterated on RC's patch and removed the top padding like you said. Let me know what you think?
It currently breaks if the browser window gets resized while the drawer is open (the height only gets recalculated when it's toggled). Not sure how much of an edge case that is and if you feel like that needs more attention.
#11
in reply to:
↑ 10
@
7 years ago
Replying to obenland:
@melchoyce I iterated on RC's patch and removed the top padding like you said. Let me know what you think?
Looks good!
It currently breaks if the browser window gets resized while the drawer is open (the height only gets recalculated when it's toggled). Not sure how much of an edge case that is and if you feel like that needs more attention.
Yeah, would be good to fix, but it definitely is more of an edge case. I'm okay with leaving it as-is for now.
@
7 years ago
Relocate the conditional to load WordPress.org filters for the wporg
section for improved extensibility and future compatibility.
#13
@
7 years ago
- Keywords commit added; needs-testing removed
- Resolution fixed deleted
- Status changed from closed to reopened
[41903] introduced a new method in WP_Customize_Themes_Section
for rendering the filter drawer template. However, the call to that function was only made if it was the wporg
instance. To make the distinct function useful for modular extensibility, the conditional should be located within the new method, and it should always be called. That way, custom themes sections can use custom filter bars and custom filter drawers by overriding those specific methods, without overriding the entire section template. If core introduces a filter drawer for installed themes (for multisite, for example), the new method can similarly be used without changing the entire template. This will also provide better future compatibility for any custom section types.
See 42212.2.diff.
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
7 years ago
#16
@
7 years ago
- Keywords needs-patch added; has-patch commit removed
- Resolution fixed deleted
- Status changed from closed to reopened
- Version set to trunk
Following [41903], the filter bar should always be sticky, regardless of screen size. Previously, it was only sticky if the sticky filter bar would fit on the screen when toggled. It is now possible to always stick the filter bar because the filter drawer is never sticky, and will be able to fit regardless of the screen size. The UX is very odd with it unsticking at many screen sizes now (including mobile, where there is no easy way to scroll to the top).
This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.
7 years ago
#18
@
7 years ago
- Keywords has-patch needs-testing added; needs-patch removed
42212.3.diff is a first pass at removing the conditional stickiness. Could use more eyes on the JS in particular, based on how that was intended to work. Some magic numbers in here currently.
#19
@
7 years ago
@melchoyce I've deployed this change to a test env for review: http://trac-39896-xwp-testbed.pantheonsite.io/wp-admin/customize.php
#21
@
7 years ago
@celloexpressions Can you add screenshots or a video of how you're seeing it work now, vs. your patch's intended results? Hard to understand your intention without a visual.
This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.
7 years ago
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
7 years ago
#24
@
7 years ago
- Milestone changed from 4.9 to 4.9.1
I'm also noticing issues with getting the theme filters pane to toggle on or off. The list of checkboxes seems to be hidden sometimes when expanding the list. If I hit it repeatedly then the filter list can appear behind the list of themes.
I'm going to punt this as it's not likely we'll have a tested updated patch before RC1.
#25
@
7 years ago
The version in 4.9RC1 is still broken - the filter bar disappears on screens shorter than a magic number (or narrower than a different, also magic number), unless you scroll all the way up to the top (quite difficult with one-column infinite scroll of themes). I don't happen to use this feature on any devices that would be affected by this problem, but the experience is quite subpar for those users currently. I'm guessing there might be some issues with CSS caching in the above screenshots, or, those that worked on the previous changes in this ticket may have a better idea of why certain browsers don't like something here. Probably worth exploring before 4.9 release if anyone is able to do that, though.
The design intent here is to toggle the filters open and closed to switch between changing the filters and viewing themes. Note that nothing is sticky if there isn't room to view all of the filters vertically on your window size.
Maybe we should instead work on the styling of the feature filter toggle, particularly in the expanded state. Maybe adding a close icon could help?
I don't think that it would work to unstick when you scroll, because you couldn't scroll up in the themes list by doing this (past where you were when the filters opened). An exception is that changing the filters auto-scrolls to the top of the list (and pushes the themes grid down below the filters) - we could do that here, but would need to restick the filters when going back to the button. It's probably cleaner to keep the behavior simple and improve the interaction of the actual toggle.