Inserter: Add block pattern category selection dropdown in main inserter #24954
Conversation
apeatling
added
the
[Feature] Inserter
|
One question for this change. Should we include an "All" category? I have concerns because the poor performance of loading all patterns when you have more than 50 patterns to show. If we did have an All category I don't think it should default to being selected. |
|
Size Change: +1.15 kB (0%) Total Size: 1.2 MB
ℹ️ View Unchanged
|
|
Worth aligning the drop down icon with other instances. It might need a smaller version but this could be true in revised Settings cases too. And I'd remove Viewing –the group (patterns), the terms, and the behaviour should be self-explanatory. Perhaps moving it to the drop down instead of saying Select a Category. |
youknowriad
added
Needs Accessibility Feedback
|
For breadcrumb purposes: #21080 |
|
Let's consider adding this same UX functionality for the "Blocks" tab as well. |
I agree with having an "All" category, though I'd say a proper solution to the performance concern would be some sort of deferred/lazy-loading a set number of patterns within the "All" category. |
|
We are already lazy loading but it's still a lot of elements on the page when you get to hundreds of patterns containing many blocks each cc @youknowriad |
Right now, the list does have asyn/lazy loading but it's not performant enough regardless, depending on browsers and number of patterns. I'm in favor of avoiding the "All" category for now until we figure out a way to improve the performance. |
|
@richtabor I'm a bit more hesitant with using the same implementation for blocks as with patterns. There's been an effort to reduce block categories in number in core, as well as having them visible by default. Patterns groupings are inherently more granular as there could be one for every block as well as subject specific ones. So i think the presentation / browsing should be optimized for each. One area where we could have a similar category dropdown is in the quick inserter for blocks, though. |
|
I've updated to us
|
|
I know it was suggested earlier to remove the "Viewing" label from the dropdown, but the lack of a label introduces accessibility issues. I strongly recommend we bring it back. It makes the component easier to use and understand. The label is also necessary for |
|
I agree there should be a label, but I'm not sure "Viewing" is the right term to use. How about "Category" or "Filter"? |
| <> | ||
| <div className="block-editor-inserter__panel-header"> | ||
| <CustomSelectControl | ||
| label={ __( 'Select a Category' ) } |
There was a problem hiding this comment.
There is still a label "Select a Category" that is readable by screen readers, just not visible on the page.
There was a problem hiding this comment.
There was a problem hiding this comment.
Visible labels should always be used when possible. aria-labels/tooltips should be a last resort.
While the efforts towards accessibility have been significant, there has been a strong dependence on methods we consider to be anti-patterns. First among these is a heavy use of ARIA attributes. ARIA, while sometimes necessary, should be considered only as a final option when no native HTML interactions are available.
https://make.wordpress.org/accessibility/2018/10/29/report-on-the-accessibility-status-of-gutenberg/
There was a problem hiding this comment.
In this instance, although aria-label="Select a Category" is set on the button we are not relying on this solely as there is also a native HTML <label>Select a Category</label> visible to screen readers. It seems that it is an acceptable practice to hide the labels from the visual UI, but I am may be missing something.
I agree with @joanrho and @apeatling that the dropdown down-arrow is sufficient here in communicating the intent in terms of the visual UI.
|
I'm open to design suggestions here. In my opinion needing the visible label so users can understand implies that users also don't understand what the category heading labels are in the block inserter view. I don't think that's the case. We have down cheverons as an indicator that the component is clickable and expandable across the UI. |
|
I agree that it makes sense to show "All" by default as well as providing the dropdown in the Blocks tab. Even if there may be five base categories (Text, Media, Design, Widgets, Embeds), other plugins may add their own blocks that extend that block menu even further, so the dropdown can only help users find what they're looking for (whether they're just browsing or searching with intention). I also think the dropdown down-arrow is sufficient in communicating the additional categorization view options. Users have already clicked into the block inserter menu, so one can assume they're looking to add a block or pattern either by searching or browsing. The screen-reader visible text cue "Select a category" that's hidden from view sounds like a good idea. |
|
This tested well for me. With some sample data of 60 patterns with 10 or so patterns per category, there was a minor delay when loading some of the categories with more complex blocks, but it was perfectly usable:
If I went up to 180 patterns, with 20+ items in some categories the performance dropped noticeably (and understandably), with marked delays in loading:
But, for current realistic use cases I think this PR is a good first step to actually make the pattern library usable with larger numbers of patterns ... and I think we are going to need this default category breakup regardless of how we decide to improve the performance for loading larger categories in the future, so those considerations don't need to delay the merging of this in my view. |
|
This works well for me 👍 I experienced the same minor (but still quite usable) delays with about a dozen patterns in a category. Scaling this up to 66 patterns in my test category, the time to display the previews reached nearly 30 seconds. This is still a much better and more usable experience than when attempting to display multiple hundreds of patterns in one list. Prior to this change, that was taking several minutes to render the previews and also made it very difficult to scroll and see the available categories in order to more quickly target a desired pattern. As a side note, throttling my connection to a much slower speed to stall some of the images loading didn't have any noticeable effect outside what you would expect. My vote is to proceed with this PR and continue to optimise the performance of the category lists in future iterations. |
|
@glendaviesnz @aaroncampbell Thanks for the reviews! Looking at the gif, I think there is an issue with lazy loading that I'm going to investigate. It seems to be waiting until all patterns are loaded before showing anything, which is not the case in the main branch. |
|
Lazy loading should be fixed now, and results in a much faster load time for the patterns show on screen:
|
…tegories for the search.
apeatling
force-pushed
the
add/block-pattern-inserter-dropdown
branch
from
a9af291 to
fa66c90
Compare
|
✅ Rebased |
@afercia, are you happy to clear your change request from the PR now? |
|
useMemo changes working well for me Glen 👍 |
|
Looks like there's a z-index issue (I'm testing on safari btw) |
packages/block-editor/src/components/inserter/block-patterns-tab.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/inserter/quick-inserter.js
Outdated
Show resolved
Hide resolved
Sorry for being late. I have a huge queue of unread emails 😞 Thanks for going ahead! |
|
For 🐝 -docs: added to Trello card. |
Description
This PR adds a dropdown menu to the Patterns tab of the main inserter. Using this dropdown will filter the displayed patterns by category.
How has this been tested?
Types of changes
Checklist: