Add initial quick inserter #22789
Add initial quick inserter #22789
Conversation
youknowriad
added
[Type] Enhancement
youknowriad
requested review from
jasmussen,
pablohoneyhoney,
mapk,
mtias and
enriquesanchez
|
Size Change: +1.22 kB (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
|
Hi @youknowriad! 👋 Took it for a test on Firefox. I found a placement bug when the inserter is near the top of the document:
The inserter looked good when invoked from a lower place:
Did a quick test with only keyboard and I did not find any issues. |
|
Thanks @enriquesanchez yes, it seems the Popover component doesn't refresh its size when the size of the content changes (start searching), I'll try to debug it. |
|
Amazing work, Riad. I'm happy to get an initial version of this in, but I think we need to try and move a little closer to the mockups. The initial state:
The mockup:
Search:
I don't know how possible this is, can we be smarter about the height of the inserter? It was cropped by the viewport and I had to scroll to see the whole thing:
I imagine that this is very technically challenging, so as an alternative, we should look into a max-height. As it is, this is very tall compared to the mockup:
Some things that should help make it smaller:
The browse all — I can't recall where that discussion landed. Is it complicated to build? |
youknowriad
force-pushed
the
add/quick-inserter
branch
from
1a6636e
to
a3a7619
Compare
|
pushed some updates here per the comments. The Popover size and direction is still not adapting properly to the content height change. I explored some solutions but it's a bit challenging. Maybe @ellatrix could land a hand here when you find some time. |
|
Have been giving this a run through over the past few days. The things that have stand out from the designs:
|
|
I think what's remaining here is the "browse all" link to the main inserter but the main blocker remains the fact that the popover doesn't adapt its height and position to the available space. It's done on first render but not as we search. |
youknowriad
requested review from
ajitbohra,
chrisvanpatten and
mkaz
as code owners
youknowriad
force-pushed
the
add/quick-inserter
branch
from
dd59223
to
187a372
Compare
|
This is looking good. Once we address these I think we can merge:
It's also very frustrating to me that arrow keys closes the inserter. |
Am aware, and haven't forgotten the tabs. I'm still looking for a good design, but in the meantime I pushed a change that at least fixes things:
|
youknowriad
force-pushed
the
add/quick-inserter
branch
from
982cb91
to
6c929a5
Compare
These two items are implemented/fixed
Potentially, but maybe we can start without it first to avoid the added complexity (communication between block-editor and external module) and add later? I'd appreciate another round of feedback here. thanks. |
|
Tested this one again on Safari + VoiceOver. Keyboard navigation feels solid and predictable. I ran into some issues however when attempting to do a search. 1. The inserter's position shifted as soon as I started typing.From
To
2. On the main inserter, after entering a search string I hear the number of results found, but I couldn't hear that in the quick inserter.It'll be great to be able to also hear the number of returned results here. 3. The 'close' button for a search entry is labeled after the search string, and there's no indication that this is a reset or cancel button
In the above example, I only hear 'quo, button'. This problem is also present in the main inserter. I think it'll be best if the action could be communicated better. Something like 'Reset search'? 4. Focus is unpredictable after canceling/resetting a searchIf I click the 'quo' button above, I then see this:
I'm not sure where my focus landed, it looks like somewhere in between the search field and the first block because if I do I think the expected behavior here would be to have the focus back in the search field because I'm canceling a search query after all. 5. The 'Browse all' button could include a bit more info for users of assistive technology
After I click it, I immediately hear that my focus is now on a search field, but I have no clue or context that I'm on the main inserter now. I think that the 'Browse all' button could be more descriptive of what will happen by including more context. Maybe something like ´aria-label="Browse all. This will open the main inserter panel in the editor toolbar."´?
Or something along those lines... |
youknowriad
force-pushed
the
add/quick-inserter
branch
from
9d3df37
to
d479589
Compare
youknowriad
requested review from
gziolo,
nerrad,
ntwb and
talldan
as code owners
|
Thanks for all the iterations here 🎈 |
|
@youknowriad @mtias Where does the block directory logic fit in?... on the eve of the block directory launch 😆. Edit: The logic to trigger block directory searches live in the full feature inserter so it technically won't get triggered here. That may be purposeful, if so 👍 but it may be unexpected from a user standpoint. |
|
It's similar to the autocomplete inserter, which is the one that more clearly should not integrate the block directory, in my opinion. The quick inserter is in-between — perhaps we should integrate it down the road but I'd prefer to continue to refine the experience first in the main inserter. |
|
I'm seeing an error in the beta widgets screen where searching doesn't work, giving this error:
|
|
@jasmussen would you mind ticketing that, I can look later. |
|
Done: #23649 |
related #21080
This PR starts implementing the quick inserter. It's early implementation and the task has proven to be harder than expected for two reasons:
I'm still trying to figure out what would be the MVP to ship this.