Use ellipsis to denote when menu items have modals (secondary steps) #53696
+21
−21
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
richtabor
added
[Feature] Block settings menu
richtabor
requested review from
kevin940726,
Mamaduka,
ntwb,
nerrad,
ajitbohra and
ellatrix
as code owners
|
Size Change: +57 B (0%) Total Size: 1.51 MB
ℹ️ View Unchanged
|
Mamaduka
reviewed
Aug 15, 2023
| @@ -60,7 +60,7 @@ export default function BlockLockToolbar( { clientId, wrapperRef } ) { | |||
| <ToolbarGroup className="block-editor-block-lock-toolbar"> | |||
| <ToolbarButton | |||
| icon={ lock } | |||
| label={ __( 'Unlock' ) } | |||
| label={ __( 'Unlock…' ) } | |||
There was a problem hiding this comment.
Probably not super helpful for block toolbar buttons which are hidden by default.
I also noticed that the "Locking/Unlocking" items are missing the aria-haspopup/aria-expanded (#24796). I'll add those tomorrow.
It would be nice to get a11y feedback on this one.
There was a problem hiding this comment.
@alexstine thoughts on adding an ellipsis for the ToolbarButton text here?
There was a problem hiding this comment.
@richtabor It really doesn't change things much for screen readers. It will be highly depend it on how each screen reader decides to read "..." and this is also based on user controllable settings. Why not this for example?
Suggested change
| label={ __( 'Unlock…' ) } | |
| label={ __( 'Unlock, opens modal' ) } |
The ARIA attributes, aria-haspopup should fix this for accessibility but not sure what you do here visually.
|
In general I'm a little reluctant to add this ellipsis, as to me it just makes this text look different than other text in the dropdown, and nothing about it says that it'll add a secondary action. At the same time, I'd hate for an icon to be added to denote the same, and if that's the alternative, I'd much prefer this typographic addition which is at least consistent, it appears, with what MacOS does. So I'd love a couple of more thoughts on this one! |
|
+1 for a simple typographical solution. I think the ellipsis communicates "there will be more to this" fairly effectively. That said, this feels like the sort of change that could create more confusion if it is not introduced holistically, and maintained programatically. It would be strange (imo) for this heuristic to exist only in the block toolbar, and not in the editor toolbar, Command Palette, and so forth. |
Something worth bringing up in the systems work? |
|
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. |
|
I remember discussing this with @ciampo at some point, but I don't recall where... maybe during the Iirc the upshot was that it would be difficult to automatically codify this behavior. Maybe there could be a prop that adds the ellipsis? This still feels like a good idea to me, it's just unclear how we can ensure consistent implementation. |
That could work fine imo. |
It was discussed in #61094 :) To recap, my take is the The component aims at being as flexible as possible and aims at doing so via composition, allowing its consumers to set any text label (or React node, FWIW) they want. Hence also why I'm hesitant to adding a prop on (cc @WordPress/gutenberg-components ) |
|
I'm strongly in favor of adding the ellipsis rule in the documented design guidelines. We should be careful of the language though, "opens a modal" is too specific in my opinion. For example, the HIG uses this verbiage:
How about we open an issue and discuss how to actualize this? There are definitely pros/cons to using a prop vs. relying directly on consumer-provided strings. I can imagine there being inconsistency between faux/real ellipses ( |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Currently, you don't know if clicking a menu item within the block options toolbar will conduct an action, or open a modal instead. The expectation of requiring additional input is clearer, you don't have to know it requires additional input.
Testing Instructions
Visual
Recordings
Before
CleanShot.2023-08-15.at.14.16.55.mp4
After
CleanShot.2023-08-15.at.14.15.30.mp4