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

Remove "destructive" variant of Components #51639

Closed
getdave opened this issue Jun 19, 2023 · 10 comments
Closed

Remove "destructive" variant of Components #51639

getdave opened this issue Jun 19, 2023 · 10 comments
Labels
[Package] Components /packages/components [Type] Code Quality Issues or PRs that relate to code quality

Comments

@getdave
Copy link
Contributor

getdave commented Jun 19, 2023

As mentioned by @jasmussen in #50843 we should avoid the destructive variant of components.

Let's codify this convention by removing the props so as to avoid developers being able to introduce it into the codebase.


> You should never see this. When there's only one menu, show it directly on this top level of the Navigation section, as per #50396:

This has been noted in #51561 and we are fixing in #51565

Let's not use the red color for the button here. In general I think we should move away from "destructive colors" as they do not work well for color blindness

If that's the case then let's deprecate the props of the relevant @wordpress/components so that we codify the convention 🙏

Originally posted by @getdave in #50843 (comment)

@getdave getdave added [Type] Code Quality Issues or PRs that relate to code quality [Package] Components /packages/components labels Jun 19, 2023
@getdave
Copy link
Contributor Author

getdave commented Jun 19, 2023

Pinging @ciampo for his opinion here.

@jasmussen
Copy link
Contributor

To add a little nuance, I think an argument can be made to keep the destructive color, for historical usage and for use cases like this one:

Screenshot 2023-06-19 at 10 22 53

My point with the comment was specifically that for Confirm dialogs like the one employed for "Are you sure you want to delete this navigation menu", I consider that component to be the warning that it is destructive, and not in need of further warning signals. To an extent I think of it as an extension of the JS confirm dialog, with very little to no styling options.

@getdave
Copy link
Contributor Author

getdave commented Jun 19, 2023

Ah ok so this Issue isn't warranted then and can be closed?

@jasmussen
Copy link
Contributor

Not sure — I'd love to codify the practice, but I'm not sure if it should be in guidelines or whether the buttons inside a Confirm dialog can be locked down.

@getdave
Copy link
Contributor Author

getdave commented Jun 19, 2023

I was going to say I didn't think we have a standard "dialog" component but it looks like we do

https://wordpress.github.io/gutenberg/?path=/story/components-experimental-confirmdialog--default

So therefore we should check we don't have a prop on that component that allows for destructive.

I think we should close this Issue however as I don't think it makes sense anymore.

@getdave getdave closed this as completed Jun 19, 2023
@ciampo
Copy link
Contributor

ciampo commented Jun 19, 2023

A bit late to this conversation!

isDestructive is a prop of the Button component. It serves uniquely to add visual styles to the Button component.

If we want to discourage using that prop, we could:

  1. search across the Gutenberg and remove the prop from all usages of Button
  2. deprecate the prop
  3. We would need to understand if we also want to give a notice that, withing X WordPress released, the isDestructive prop (and related styles) will be removed from the component

@getdave
Copy link
Contributor Author

getdave commented Jun 19, 2023

@ciampo Apologies. It opened and closed too quickly. I was a bit hasty!

It looks like Joen was justing wanting to avoid "destructive" on confirmation dialogs. As we don't appear to have that prop exposed on the experimental dialogue component I think we're ok on that front now 🙇

@jasmussen
Copy link
Contributor

jasmussen commented Jun 19, 2023

Sorry about the detour! I think the conversation remains valid (but maybe it can be primarily conversation at this stage), "red" for "bad" isn't necessarily going to help any flows other than make one pause, and for actually destructive (no trash/revisions situations), an "Are you sure" seems better.

@ciampo
Copy link
Contributor

ciampo commented Jun 20, 2023

In the case of "destructive" actions, showing a confirmation dialog definitely sounds like a good pattern.

IMO, color can aid users in setting expectations around a piece of UI, but it should not be considered the main way that meaning is conveyed.

If at a later point we wanted to remove the destructive styles from Button, I reckon that the plan highlighted above should work (cc @mirka )

@jasmussen
Copy link
Contributor

Yep, sounds good. We can shelve the conversation and revisit, my main purpose was mainly to omit the destructive color from modals and confirm dialogs, since in those cases you are already in the "are you sure" part of the flow, with no need to warn further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

No branches or pull requests

3 participants