Skip to content

Duotone Settings: Add reset button and improve toggle rendering in FiltersPanel#68672

Merged
t-hamano merged 6 commits into
WordPress:trunkfrom
yogeshbhutkar:68671-add-reset-button-duotone
Feb 7, 2025
Merged

Duotone Settings: Add reset button and improve toggle rendering in FiltersPanel#68672
t-hamano merged 6 commits into
WordPress:trunkfrom
yogeshbhutkar:68671-add-reset-button-duotone

Conversation

@yogeshbhutkar

@yogeshbhutkar yogeshbhutkar commented Jan 15, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

  1. Navigate to post edit page.
  2. Create an Image block and insert an image.
  3. Add Duotone filter to the image.
  4. Confirm, the Duotone value can now be cleared with the reset button.

Screencast

Demo

Closes: #68671

@yogeshbhutkar yogeshbhutkar marked this pull request as ready for review January 15, 2025 06:27
@github-actions

github-actions Bot commented Jan 15, 2025 •
edited
Loading

Copy link
Copy Markdown

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: yogeshbhutkar <yogeshbhutkar@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

t-hamano
t-hamano reviewed Feb 1, 2025
View reviewed changes
Comment thread packages/block-editor/src/components/global-styles/filters-panel.js Outdated
Comment thread packages/block-editor/src/components/global-styles/style.scss Outdated
@Mamaduka Mamaduka added [Type] Enhancement A suggestion for improvement. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Feb 1, 2025
t-hamano
t-hamano reviewed Feb 3, 2025
View reviewed changes

@t-hamano t-hamano left a comment

Copy link
Copy Markdown
Contributor

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.

@yogeshbhutkar

yogeshbhutkar commented Feb 3, 2025

Copy link
Copy Markdown
Contributor Author

You will probably need to move away from using ItemGroup,Item components and align with other implementations.

I reviewed the border-panel implementation and noticed that ItemGroup is used as a parent for the dropdown. Since ItemGroup provides the necessary border and separation, I refactored the code accordingly to align with the border-panel coding style.

Ref:

gutenberg/packages/block-editor/src/components/global-styles/border-panel.js

Line 301 in 23d6b4a

<ItemGroup isBordered isSeparated>

@yogeshbhutkar yogeshbhutkar force-pushed the 68671-add-reset-button-duotone branch from 23d6b4a to 300968c Compare February 3, 2025 09:59
t-hamano
t-hamano reviewed Feb 3, 2025
View reviewed changes
Comment thread packages/block-editor/src/components/global-styles/filters-panel.js Outdated
@yogeshbhutkar

yogeshbhutkar commented Feb 6, 2025

Copy link
Copy Markdown
Contributor Author

Thanks for the review.

I've applied all the suggestions. The changes are testing well for me.

changes.mov

Can you please review the PR when you get a moment?

Thanks!

t-hamano
t-hamano reviewed Feb 6, 2025
View reviewed changes

@t-hamano t-hamano left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread packages/block-editor/src/components/global-styles/filters-panel.js Outdated
Comment thread packages/block-editor/src/components/global-styles/filters-panel.js Outdated
Comment thread packages/block-editor/src/components/global-styles/style.scss Outdated
@t-hamano t-hamano mentioned this pull request Feb 6, 2025
Merged
@yogeshbhutkar

yogeshbhutkar commented Feb 7, 2025

Copy link
Copy Markdown
Contributor Author

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.

t-hamano
t-hamano approved these changes Feb 7, 2025
View reviewed changes

@t-hamano t-hamano left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@t-hamano t-hamano merged commit 1777430 into WordPress:trunk Feb 7, 2025
@github-actions github-actions Bot added this to the Gutenberg 20.3 milestone Feb 7, 2025
Kallyan01 pushed a commit to Kallyan01/gutenberg that referenced this pull request Feb 24, 2025
@yogeshbhutkar @t-hamano @Kallyan01
Duotone Settings: Add reset button and improve toggle rendering in …
8094fa3 
…FiltersPanel (WordPress#68672)

* Duotone Settings: Add reset button and improve toggle rendering in FiltersPanel

* refactor: revert usage of `clearable` prop

* refactor: hover on button to show `reset`

* refactor: remove the usage of `ItemGroup` and add `css` for border

* refactor: update button label to 'Reset' and adjust border color in filters panel

Co-authored-by: yogeshbhutkar <yogeshbhutkar@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
@im3dabasia im3dabasia mentioned this pull request Feb 24, 2025
Open
@afercia afercia mentioned this pull request Mar 5, 2025
Open
6 tasks
chriszarate pushed a commit to chriszarate/gutenberg that referenced this pull request Jul 1, 2025
@yogeshbhutkar @t-hamano @chriszarate
Duotone Settings: Add reset button and improve toggle rendering in …
a98a082 
…FiltersPanel (WordPress#68672)

* Duotone Settings: Add reset button and improve toggle rendering in FiltersPanel

* refactor: revert usage of `clearable` prop

* refactor: hover on button to show `reset`

* refactor: remove the usage of `ItemGroup` and add `css` for border

* refactor: update button label to 'Reset' and adjust border color in filters panel

Co-authored-by: yogeshbhutkar <yogeshbhutkar@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@t-hamano t-hamano t-hamano approved these changes
@tellthemachines tellthemachines Awaiting requested review from tellthemachines
@ellatrix ellatrix Awaiting requested review from ellatrix ellatrix is a code owner

Assignees

No one assigned

Labels

Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Enhancement A suggestion for improvement.

Projects

None yet

Milestone

Gutenberg 20.3

Development

Successfully merging this pull request may close these issues.

Duotone Filter: Add Reset button to maintain consistency with Background and Text color controls

3 participants

@yogeshbhutkar @t-hamano @Mamaduka