Adds vertical variation to navigation menu #21296
Conversation
draganescu
requested review from
ajitbohra,
jorgefilipecosta,
Soean and
talldan
as code owners
draganescu
added
Needs Design
|
Size Change: +17.7 kB (2%) Total Size: 884 kB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
This looks pretty good, and it's great that it's only a small change.
There were a couple of things that I noticed, which are mostly comments on how variations work in general.
I think this needs a good copy/design review. I wonder if 'Navigation (Horizontal)' and 'Navigation (Vertical)' might be better titles in the context of the inserter:
The second thing is that it feels like there should be a transform or a way to switch between variation. It looks like that's tracked by #20584, and it should probably happen automatically through declaration of the variation.
Also, that the variations don't appear in the slash inserter confused me, but looks like that's tracked by #20583.
talldan
added
the
Needs Copy Review
Co-Authored-By: Daniel Richards <daniel.richards@automattic.com>
Co-Authored-By: Daniel Richards <daniel.richards@automattic.com>
Co-Authored-By: Daniel Richards <daniel.richards@automattic.com>
|
I would +1 'Navigation (Horizontal)' and 'Navigation (Vertical).' |
talldan
removed
the
Needs Copy Review
|
Shouldn't this be just an option inside a single navigation block? All that's really needed is to add a class to the element to flag if it's horizontal or vertical, so a simple radio-input in the navigation block to select orientation would do just the same |
|
hi @aristath having this as an attribute works better as this setting influences other behaviors as well (for example mover direction). The way to have attributes preset is to use the new variations API. There was some discussion about using block styles but it would complicate styling for every single difference that the vertical or horizontal layout makes. |
|
This is cool, a nice small tweak. cc @gziolo for the variations nod. Next step is to allow transforms! |
Do you think, we should allow transforms for variations? |
|
For some, absolutely. It's a way to "swap" one variation with another. It could be useful for changing a Twitter profile with Mastodon, or to change the layout arrangement of columns. It might also be exposed in its own "Variations" panel inside the transforms menu (like styles are). |
#Yes, I agree on that changes on attributes would be quite useful and in general safe to apply. With Columns block and inner blocks case we would need to decide if we provide integration with callback functions that help avoiding content loss. The implementation itself doesn’t look complex unless we want to keep the current variation highlighted in UI. |
|
This seems to have caused a regression in master. The movers for the normal navigation block are no longer horizontal. |
|
I just wanted to create a new issue for this, glad I noticed. Related issue: #22085 |
Description
Fixes #16829
How has this been tested?
Tested locally by:
Screenshots
Types of changes
Non breaking change that introduces a vertical variation to the Navigation block.