-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Redesign the main pattern inserter #44028
Conversation
Size Change: +665 B (0%) Total Size: 1.27 MB
ℹ️ View Unchanged
|
This is glorious, and I look forward to reviewing and helping in any way I can. This would be excellent to land soon! I'm encountering this issue testing the branch:
Any thoughts? |
@jasmussen I think that issue is unrelated, probably due to some on going core merges. So try to destroy your environment or use WP 6.0 as a base WP install. |
As an environmentalist, I prefer not to destroy the environment 😜 — however I was able to downgrade, and yes, 6.0 works great. Diving in, thanks. |
As a single PR, this feels incredible already: A few small tweaks I think we can make. The "Explore all patterns" button: can we make it more like the "Manage your library" button here? The button style feels like a nice bookend to the list. I happen to be running a light-gray theme, so the light gray background makes things blend in here a bit: I'm going to explore what we can do here to separate things out a bit better. One open question is whether we can improve some of the space usage. I think it's okay that the pattern flyout covers content, but I wonder if we could reduce the width of the inserter when the pattern is opened 🤔 |
Alright a couple of quick mockups.
One cool thing would be to leverage the recently merged zooming, and invoke that when this pattern library is opened: Let me know if you'd like me to try and push some of the border/background-color/filter tweaks! |
@jasmussen Feel free to push the design tweaks.
I tried this but the way the inserter is styled / organized, it's not straight forward. I'll get it another try though. |
I pushed a little bit of polish around paddings, border, background colors, styles. It now looks like this: This adds a backdrop-filter which works decently well for making the patterns feel separate from the content, yet still overlay them. However I want to note a performance concern here, blur is expensive to animate. Seems like we could either remove the blur (the blur especially becomes useless if we zoom out), or we can remove the initial animation, which would also be unfortunate. Any alternatives? One thought on the animation, though — right now it's a bit bouncy. If we can choose a "ease-out" that would likely feel better. Did you have any thoughts on whether we could animate the main inserter to be smaller? |
What's the "blur" that you're talking about here?
Yes, we can tweak it, that's just the default slide in from framer motion.
I'm hopeful that we can do that. I'll try a bit later. I'm focused on this for now #44088 which is a blocker to move the "browse patterns" to the bottom. |
I lowered the opacity of the white bg color to make this more visible: It's mostly visible when the pattern flyout covers a wide range of content. We want to find a balance between the blur being visible and the content inside being legible. We can also drop that blur. CC: @SaxonF. |
be3ef14
to
0d42722
Compare
I tested the last changes a bit, I personally preferred the gray background over the blurry white, especially in white bg themes (which I think are probably the most common). I also like the contrast between the inserter tabs (white) and the patterns list (gray) |
Quick note that this isn't working properly in the Site Editor. The pattern panel doesn't appear when clicking a category, unless you first navigate away from the Patterns tab (to Blocks for example) in the Inserter. |
I was not able to reproduce this. |
Here's what I'm seeing in case it helps: bug.mp4 |
@jameskoster yeah nvm, I managed to break it now :) |
@jameskoster the bug should be fixed. @jasmussen The inserter scales down a bit now |
This is excellent! I notice that there's also animation on the sidebar as it appears initially. The animation from the left side seems okay enough to indicate directionality, and it's a nice little bit of flair. However it would be good to test the performance of this with very long and complex pages, as it essentially also animates the viewport/iframe itself — I don't know if that should be expensive, it might not be? But worth testing. But in any case, we should replace the bouncy animation with a simpler ease-out, or it becomes a bit disorienting. Really nice work, I honestly think we can land this in short order! 🔥 |
The only thing that I'm uncertain about and to be honest, not sure how to solve is keyboard navigation. What should be the behavior here? |
@youknowriad Okay, awesome. Focus did not end up working as I thought. You placed focus on the first item of the category list which should have worked, but sadly, it does not work due to how NVDA has changed the screen reader to behave in these situations. The navigation landmark is a much better idea. Following improvements are needed.
Other than that, I would be okay doing a final check after this and then green lighting it. For the future, I guess we're going to have to wrap the inserter in a Nice work. Thanks. |
I'm not entirely certain what tabindex are you talking about, also it seems that you're saying that we should remove the "arrow key navigation" (Composite items) from here right? Do you think you would be able to make the change as I'm not 100% clear?
We can, it's close in the component tree, but it does feel very different to me in terms of interaction and behavior, in this sense, it doesn't feel like it belongs to the same list as the "categories". |
@youknowriad More info.
You just need to remove the wrapping composite component from the pattern categories.
This is not necessary since we now have a list of buttons in a I think moving the Explore all patterns button will help out. I know it is not really a category, but we are trying to communicate relationship between navigation and what will soon be the rendered categories. The WordPress add plugins page follows the same model I think? Showing all plugins but then it is filterable? Even though all is not technically a category, it is still grouped together for better screen reader relationship. Thanks. |
@alexstine I've removed the composite items, let me know how it feels now. Thanks |
@youknowriad Okay, time to get picky.
Is it really necessary to have a heading here? The pattern category links need After these two issues, we can refine some other stuff in another PR and get this one in. I mainly want to find out why constrained tabbing is acting a little weird with the Thanks. |
I'm following the design mockups here, that said, I can definitely make it a regular div or role="presentation" or something like that |
I added the aria-current |
@youknowriad Regular div instead of the heading would be good. I try to avoid After that, this is good to go. |
Heading removed. |
@youknowriad Signing off on the A11Y review. We'll make further improvements to the UX in the future but this now has know blocks from being tested and eventually released. Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥 🔥
Hi @jameskoster
pattern panel from the canvas in first place ? |
What?
See #41379
This PR redesigns the patterns inserter. The first step is just to show the categories before actually rendering the patterns.
Here's the current state
pattern-inserter.mov
As you can see it's still very rough but we can get an idea of what we're trying to achieve, here are some of the remaining challenges:
Testing Instructions