Skip to content
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

Shadow Panel: Add reset button #68973

Closed
t-hamano opened this issue Jan 31, 2025 · 5 comments · Fixed by #68981
Closed

Shadow Panel: Add reset button #68973

t-hamano opened this issue Jan 31, 2025 · 5 comments · Fixed by #68981
Assignees
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.

Comments

@t-hamano
Copy link
Contributor

Related PRs:

What problem does this address?

In the color panel, if any value is set, a reset button will be displayed on the item itself:

Image

On the other hand, the Shadow panel has no reset button even though values ​​are set:

Image

What is your proposed solution?

It would be useful to have a reset button on the Shadow panel as well.

@t-hamano t-hamano added [Type] Enhancement A suggestion for improvement. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Jan 31, 2025
@yogeshbhutkar
Copy link
Contributor

I'd be happy to work on this enhancement. There's also another similar PR for the Duotone control here.

@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Jan 31, 2025
@Mamaduka
Copy link
Member

Mamaduka commented Feb 2, 2025

Can we extract this into a reusable component but keep it internal for Global Styles?

@yogeshbhutkar
Copy link
Contributor

Can we extract this into a reusable component but keep it internal for Global Styles?

I believe we can. It will reduce the redundancy.

We can introduce a reusable component in this PR and create a separate PR refactoring the previous usage to use this component.

Hi @t-hamano (apologies for tagging), since you've already reviewed two of my similar PRs, I'm tagging you for a second opinion before implementing this change. Please let me know what your thoughts are.

@t-hamano
Copy link
Contributor Author

t-hamano commented Feb 3, 2025

Can we extract this into a reusable component but keep it internal for Global Styles?

Does this mean extracting the reset button part into a reusable component? If so, I think that's possible.

@Mamaduka
Copy link
Member

Mamaduka commented Feb 3, 2025

I was thinking about the whole component. They seem to have a similar layout - icon + label + reset.

We can always abstract later - duplication is better than a bad abstraction. Let's try to land current PRs first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants