Move search into inserter tabs #61108
Conversation
|
Size Change: +6 B (0%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 020277c. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/8911639098
|
scruffian
force-pushed
the
update/move-search-inside-tabs
branch
from
c0b2c99
to
c48a820
Compare
scruffian
added
the
[Feature] Inserter
scruffian
requested review from
draganescu,
adamziel,
kevin940726 and
ellatrix
as code owners
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
draganescu
force-pushed
the
update/move-search-inside-tabs
branch
from
c9f6812
to
cf0e71c
Compare
|
I'd have loved to have an opportunity to check this one.
Also looking at the code, it seems the PR shuffled a bit the code in the menu component but I wonder if it would have made things a lot more simple if we just moved the search into each one of the "Tabs". |
|
I can understand that the search input is now not focused when the inserter is opened because of DOM order... But I wonder if it's seen as a usability regressions. At least for me, I don't like that I have to focus it manually now to search for a block but It's also logic in the sense that you have to first select what you want to search for (blocks or patterns) |
The search got moved into the content that get rendered in each tab. Do you mean move the search component down to inside of the
I imagine some people will dislike that the search is no longer focused first. If we want the search input focused first, we'd have to have it be outside of the tabs in DOM order. Otherwise, I believe it would be an accessibility regression to skip over a large section of very relevant content. |
If search allows for all types of results as such, does it matter if tabs are not focused first? I'm hesitant to require an additional action to search, which is the centric experience within the inserter. |
I mean a search input within BlockTabs and another one within PatternTabs. Not that important though. |
We skip over the Media tab if we focus the inserter, so it wouldn't be clear that the media tab is present. I really don't see an argument for focusing the search while it's inside of the tabs. If it's important that it be focused first, it needs to be the first input within the inserter sidebar. |
Because it requires one less action to get to. Previously, pressing the + inserter focused you on the search field immediately, where you can start searching for blocks right away. With this PR, you are required to conduct an additional step of either tabbing to the search field, or selecting it manually. |
As an aside, I think media results could potentially be included in the inserter (needs more thought). |
|
Is this a result of this PR? List block selected, inserter opened.
|
I mean, I get that argument, but it's not an acceptable one as it skips over the media tab. If the search inserter is going to be focused when the panel opens, it has to be the first interactive element in the inserter DOM. |
Yup! My fault. Fixing. |
|
I noticed some flakey tests started happening after this PR was merged. It doesn't necessarily mean it was caused by this PR (it might have been one before it but this was the first time flakeyness was encountered). Seems like there's a chance though given it's inserter related. It might be something to investigate: |
|
@jeryj Thanks for the PR. The functionality is testing well for me. @richtabor If you want the search input to gain focus by default, it must be placed above the tabs. You cannot expect users with no vision to know tabs are there when focus is placed in a search field after them. One with vision would clearly see the context they are searching in or that they have other options to explore before typing anything. Thanks. |
|
@talldan, I also noticed that. Based on the traces, the target pattern jumps before the test starts moving actions. I wasn't able to find a consistent way to reproduce the issue locally. Screencastinserts.patterns.by.dragging.and.dropping.from.the.global.inserter.webm |
Thanks @alexstine, yes I'm aware. I'd like more input from others on this change. Should search be moved below tabs, to better support persisting the inserter, as detailed in #61051? In doing so, we will loose the ability to focus immediately on the search field within the inserter. @WordPress/outreach |
|
Looks like this made inserter searching 15-20% slower |
|
I created an issue for the pattern tab state here: |
|
The initial reason for adding the search on top of each media category is that it also needed the search input like what this PR did, and although there was a PR in the past for that, I don't remember exactly why we didn't land it. So, are there any plans on moving the
This has perf issues mostly because pattern directory would need to know what the allowed blocks are, to show proper results. There was some work on APIs both on pattern directory and GB, but this featured didn't move forward. The approach was for pattern directory to preparse and keep some info of the containing blocks on upload, and GB send the allowed or disallowed blocks. |
I think we should add search to the Media tab as well, to allow for searching all categories perhaps. |
What?
This moves the search box in the inserter inside the blocks and patterns tab and prioritizes the search by the tab (blocks search prioritizes blocks, for example).
Why?
To keep the inserter open after users insert patterns as this will provide a better experience for assembling pages with patterns (see #61051).
To achieve this we need to add a close button to the inserter.
To achieve this we need to move the search box below the tabs so that the close button can sit alongside the tabs like we do in other side panels, hence this PR.
How?
Testing Instructions
Do the same thing in the widget editor, site editor and post editor.
Screenshots or screencast
new-inserter-search.mp4