-
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
Add context to the "Reset template" "Delete template" and "Edit template" commands #52989
Conversation
Size Change: +1.13 kB (0%) Total Size: 1.44 MB
ℹ️ View Unchanged
|
I like it, but it is a bit hard to scan with the added colon stop. What about if we used this breadcrumb approach, where it's a bit more clear? @WordPress/gutenberg-design what do you think of introducing a pattern like this for commands? As we get into adding additional context like in #51505 and #52509, this pattern will improve command readability. |
Can we use a typographic element instead of the chevron? The chevron reads to me like UI in that it's split in two commands vs. one, when it's the latter. Perhaps an emdash?
🤔 |
+1 for a typographical solution. Emdash or colon could work. Or wrapping the template name in quotes:
Should we include the delete action here too? |
Flaky tests detected in a7f1858. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5680636501
|
Here's another use case this would carry over to (per #52509): I'm not sure it conveys differently as an emdash: I still lean towards the chevron for clarity, emdash as a secondary option, if you both think its the right solution. |
Good point, that's certainly relevant as we attempt to establish consistency in the Command Palette actions. It actually makes me wonder if a colon might be better, with the format:
I'm going to cc @mcsf as I think you were involved in that discussion. If not feel free to ignore me 😁 |
Yep, makes sense to me. I think the need for a typographic glyph is mainly about establishing it as still a single action. |
Yes, the colon is a good choice — though for i18n purposes the chevron or dash would have been totally fine too, IMO (as long as it's rendered as a single string: |
packages/edit-site/src/hooks/commands/use-edit-mode-commands.js
Outdated
Show resolved
Hide resolved
So, what's the verdict here regarding the copy ? 😄 |
Do we actually need a separator...? The
|
I think we do for some languages, but not 100% sure. Additionally could that work well without adding any context? For example if we had |
Yes good point. For example if you had a page titled "About" and a custom template for that page also titled "About" (not an outlandish supposition), a "Delete About" command could be confusing. In this case the actions could be more specific:
|
I would avoid using emdash. This is because there may be languages that do not use emdashes, and emdashes are inherently used to separate sentences. I prefer either of the following formats:
|
Anything else to do here? I'll be AFK next week and would love to wrap this up. 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.
LGTM.
What?
Resolves: #51693
This PR adds more context to the "Reset template", "Delete template" and "Edit template" commands, by adding the template title in the form of
Delete template: ${name}
. Additionally thereset template
command icon was updated torotateLeft
.Testing Instructions
Test these three commands and observe that the template title is also part of the label.