Allow using a text label instead of an icon on overlay menu toggle button #36149
Conversation
vcanales
added
[Type] Enhancement
|
Size Change: +457 B (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
|
Are there any other blocks where the text is entered in the settings sidebar, and not edited in the block it self? |
I don't know. My initial approach was editing directly in the label, but since it's a button, clicking it toggles the menu instead of allowing you to edit the text. |
|
Thank you for the PR! To me the functionality feels solid, and we're left with what's mostly an UX issue. Here's what the sidebar looks like: The right pieces are there, but the prominence isn't super well managed. It's made worse by the new named menu panel, which I hope we can move under "Advanced" like its Template Part counterpart, this one:
That's a separate issue, but it nevertheless adds complexity to having the label be input in the inspector, to Carolina's point. However due to the overlay menu being a toggle, but not offering interactivity inside, at least for the time being, your point about clicking the menu icon itself opening it, is a solid point as well. I think there might be a third solution, but also that the third solution, even if that might make this a trickier PR. Consider the Global Styles sidebar, which has this card:
Right now that card itself is not interactive — it just shows the colors and font you choose with the options below. But I did see a comment on a thread suggesting the card could be clickable, sliding to the right and letting you pick a different theme.json file, or another preset, or something other to that effect. That was part of the idea of introducing a similar card for the navigation menu, this gray card with a burger:
We could build such a card for the nav block, and make it interactive. Click it and whether in a flyout, or otherwise, we show options related to the appearance of the menu. Things like the icon, or the label, perhaps even an individual icon color. We'd want to think about how to implement this in the most basic way, but I think this card principle could both scale and work for other blocks. What do you think? |
|
Instead of creating a new flyout component for this, because I suspect that will happen as part of #34574 or #27331, perhaps we could explore a smaller temporary solution. The entire reason for surfacing these options upon clicking a visual representation of the burger icon is both to give those options context (the burger might not be visible in the canvas), but also to manage the weight of the controls in the inspector. That's the reason for the sliding mechanism in global styles (to drill in), and the flyout pattern for ItemGroups (#35093). And so we definitely should change to use a flyout component once that lands. But in absence of building such a flyout for this, we could perhaps do a small "show/hide" trick. In this mockup, you click the gray icon card to surface controls relevant to to it: What do you think? |
vcanales
force-pushed
the
navigation-menu-label
branch
from
0eee722
to
0ac7634
Compare
|
After the last PR I see this: Nice work, I think we might be able to work with it. The card needs to be a bit smaller and have the right toggled state, I can probably help with that. The benefit of this preview card is that it will work whether or not you have the menu set to responsive on mobile or always. In that vein, we might want to toggle that card off, at least for now, for when you have it set to never collapse. Finally, this change doesn't preclude a future iteration where the in-canvas field becomes editable. |
vcanales
force-pushed
the
navigation-menu-label
branch
from
0ac7634
to
0912320
Compare
|
I spent some time trying to figure out why e2e tests were consistently failing, and then they didn't anymore :)
I would appreciate the help! |
|
I'll take a stab at some polish when I get a moment!
Yes, I do think we need to do something there. In fact, since this will intentionally be a temporary step until we can have some flyout mechanism, I/we probably need to look at what this will look at for that flyout, even if we aren't implementing that bit yet, just so we know where we need to aim. I'll look at it. |
|
I pushed a little polish: This needs a good rebase, and then let's take stock and get a technical review. Thanks! |
vcanales
force-pushed
the
navigation-menu-label
branch
from
6b4ec54
to
26649f3
Compare
aristath
force-pushed
the
navigation-menu-label
branch
from
dbe4079
to
11962df
Compare
|
PR rebased and conflicts resolved 👍 |
vcanales
force-pushed
the
navigation-menu-label
branch
from
11962df
to
114ddb1
Compare
|
Conflicts fixed again; this is ready for a final code review ✅. @aristath could I ask for your help with that? |
|
Just as a small update, here's a fresher mockup for what this interface might grow to become in the future: |
vcanales
force-pushed
the
navigation-menu-label
branch
from
114ddb1
to
0b10669
Compare
|
Nice to see this one land! |
Description
Closes #33985.
Allow replacing the toggle button for the Overlay menu with a text label.
How has this been tested?
Screenshots
Types of changes
New feature.