-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Duotone Settings: Add reset
button and improve toggle rendering in FiltersPanel
#68672
Duotone Settings: Add reset
button and improve toggle rendering in FiltersPanel
#68672
Conversation
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. |
packages/block-editor/src/components/global-styles/filters-panel.js
Outdated
Show resolved
Hide resolved
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.
Thanks for the PR!
The reset button should be visible when hovering over the dropdown button, not the reset button itself. Otherwise, people won't be able to use the reset button unless they know there is a transparent reset button there. This is probably because the structure is different from the rest of the UI.
You will probably need to move away from using ItemGroup
,Item
components and align with other implementations.
Note that you will also need to deal with focus issues. See this comment.
I reviewed the Ref:
|
23d6b4a
to
300968c
Compare
packages/block-editor/src/components/global-styles/filters-panel.js
Outdated
Show resolved
Hide resolved
Thanks for the review. I've applied all the suggestions. The changes are testing well for me. changes.movCan you please review the PR when you get a moment? 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.
Thanks for the update! I added a little bit of feedback at the end, but everything is working as expected.
packages/block-editor/src/components/global-styles/filters-panel.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/global-styles/filters-panel.js
Outdated
Show resolved
Hide resolved
Thanks for your suggestions! I've incorporated all of them. When you have a moment, could you take a look? Appreciate it! P.S. The failing test is unrelated to the changes made. |
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, Why and How?
This PR adds a Reset button to the Duotone Filter control, enhancing its functionality and ensuring consistency with other color control components.
The Reset button is conditionally rendered based on a newly introduced
clearable
prop.Testing Instructions
post edit
page.Image
block and insert an image.Duotone
filter to the image.Duotone
value can now be cleared with the reset button.Screencast
Closes: #68671