Closed (won't fix)
Project:
Drupal core
Version:
8.0.x-dev
Component:
base system
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
2 Jun 2014 at 22:12 UTC
Updated:
24 Apr 2015 at 11:03 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
lewisnymanComment #2
cfox612 commentedIs this for a specific theme? Both Bartik and Seven? All core themes?
Comment #3
xanoThis must be theme-agnostic, because the classes will be set in module code.
Comment #4
lewisnymanThe button_types are set in module code, but what a theme chooses to do with it is up to them. We only need to implement the CSS in Seven for now in my opinion.
Comment #5
cmanalansan commentedI added the CSS classes only, but did not update any styles to any themes.
Comment #6
lewisnymanAll the other button class variants are set with using #button-type correct? Shouldn't we be using that here? In any case the class that would need to be added would be
button--primary-dangerComment #7
lauriiiCreated a style for the button with the danger color presented in the Seven style guide.
I think we should use the button type functionality because it doesn't require any extra code.
Here's pictures of the button:
Normal:

Hover:

Focus:

Comment #8
cmanalansan commentedI changed the class to
button--primary-danger.One thing to consider before deriving this class from #button-type:
#button-type only stores one value. From what I can tell, it can be either 'primary' or 'danger' but not both.
We can address this in several ways:
primary-danger. Any elements with this #button type will have the classbutton--primary-dangerbutton--primary-danger- OR -button--primaryandbutton--danger- OR -button--primary,button--danger, andbutton-primary-dangerThe best approach here depends on this question: what do you aim to benefit from changing the #button_type?
Comment #9
cmanalansan commentedPardon me, did not mean to remove focus.png file
Comment #10
cmanalansan commentedActually, I agree with lauriii, I'm currently extending his patch to include ContentEntityConfirmFormBase and EntityConfirmFormBase
Comment #11
cmanalansan commentedBased on lauriii's patch.
Comment #12
Bojhan commentedI am not following this, this is about adding a class. The Seven style guide already has a style for remove actions. This patch shouldn't change anything about Seven.
Comment #13
xanoI am not familiar with the existing class for remove actions, but this issue is about a generic approach for any primary button that performs a dangerous action. Would you say that is unnecessary, an addition to the remove class, or a replacement for it?
Comment #14
lewisnyman@Bojhan Can we consistently call the 'remove' class the 'danger' class to reduce confusion? That's how it's set up in the CSS files and we should be describing it's purpose instead of one use case.
Comment #15
Bojhan commented@Lewis Sure :)
Comment #16
cmanalansan commentedI went ahead and removed the changes to the Seven theme.
Comment #17
cmanalansan commentedAck, disregard patch 16, this is the correct one.
Comment #18
idebr commentedHow does this relate to the danger button that is being added in #2123731: Add .button:not(a.button--danger) so that no reset is needed on button--danger? Are we creating both a link--danger and a button--danger?
Comment #19
lewisnyman@idebr yes maybe that is the most sensible way to do it, as this danger button styling shares a lot of styling with other buttons
Comment #20
idebr commentedWhat would be the use case for 'link--danger' if there is already a 'button--danger' with the exact same styling? Is the scope for this issue limited to link--danger or does it also touch button--danger?
Either way:
It doesn't seem like we should introduce a new UI element at this stage.
Comment #21
mondrakeJust had a try combining #7 with parts of #2469929-18: The confirmation forms' cancel link should be styled as a button, just to see how it looks. Screenshot on top of #2469929: The confirmation forms' cancel link should be styled as a button.
Screenshot:
Comment #24
mondrakeGreen.
Comment #25
Bojhan commentedNope, we are not doing this. Please see the style guide https://groups.drupal.org/node/283223
Comment #26
lewisnyman@Bojhan But the styleguide is not a static design, we're proposing an addition to fill a gap in the styleguide. Or we should add guidance on which button type to use when the action is both primary and dangerous. What do you think?
Comment #27
Bojhan commentedI don't think we should have "red" buttons. I think we should only have deletes that are styled as links.
Comment #28
lewisnyman@Bojhan Do you mean like this?
Comment #29
lauriiiLOL thats awesome UX!
Comment #30
Bojhan commentedHmmpf, I guess neither options are really nice.
I am not sure if the delete button class is the right direction though, it will most likely be abused a lot. When its "Delete | Cancel" its kinda fine. But when its Save | Delete for example, you really don't want this.
Comment #31
yoroy commentedWhat I see under OS X is that even the confirmation dialog of a delete action does not make the delete action the primary one.
If it's a destructive action, maybe it's indeed better to deliberately *not* present the second button/link for confirming the deletion as the primary one. In short, I'm not convinced we need to do something about this.
Comment #32
lewisnymanMakes sense to me.