Global Styles: Move the 'Edit colors' button to a standard menu item #36842
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
glendaviesnz
added
the
Global Styles
|
Size Change: +30 B (0%) Total Size: 1.11 MB
ℹ️ View Unchanged
|
|
The styling of the disabled menu is not quite right, but will fix that tomorrow if there is agreement to go ahead with this interim fix |
|
Thanks for working on this fix, @glendaviesnz!
Agree, and there's also something weird with the width of the items:
Also, if you open the menu and there are no colors, the option to remove them is enabled. However, it's a bit weird to open a menu and find that none of the options can be clicked, so I wondered if we could disable the ellipsis in that initial state. |
glendaviesnz
force-pushed
the
update/edit-custom-color-menu
branch
from
86788d7
to
5b7c67b
Compare
|
@javierarce there isn't currently a disable prop on that dropdown component, but just removing the menu when not needed seems to provide a reasonably smooth experience:
What do you think about that approach? Have pushed this to the PR if you want to test it. |
|
This looks good to me. The styling looks good to me, and I quite like removing the ellipsis when there are no custom colors. I think this makes a lot of sense as an interim fix; definitely a big improvement. |
glendaviesnz
force-pushed
the
update/edit-custom-color-menu
branch
from
7a65672
to
28f1581
Compare
noisysocks
added
the
Backport to WP 6.7 Beta/RC
ramonjd
reviewed
Dec 12, 2021
| onClose(); | ||
| } } | ||
| className="edit-palette-menu-button" | ||
| > |
There was a problem hiding this comment.
Would it be possible to make this CSS class name more consistent with others in the component, e.g., components-palette-edit__menu-button
There was a problem hiding this comment.
good point - have made this change
ramonjd
reviewed
Dec 12, 2021
ramonjd
approved these changes
Dec 12, 2021
glendaviesnz
force-pushed
the
update/edit-custom-color-menu
branch
from
db7de8a
to
a388d9e
Compare
|
Thanks for the review, @ramonjd, I have changed the class name as suggested. |
noisysocks
removed
the
Backport to WP 6.7 Beta/RC
noisysocks
pushed a commit
that referenced
this pull request
Dec 13, 2021
…36842) * Move changes to the new palette edit component * Remove redundant style * Change class name to match existing Co-authored-by: Glen Davies <[email protected]>
57 tasks
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.
Description
Moves the
Edit colorsbutton from the ellipsis to a standard menu item to reduce confusion around the UI for editing custom colors.Immediate fix for #36519 ahead of wider redesign of this panel.
How has this been tested?
Remember, as noted in #36519 this is not intended to be the full and final fix, but a quick update to reduce confusion ahead of more detailed UX fixes.
Screenshots
Before:
After: