Follow-up from #1986074: Buttons style update.

When restyling the buttons in the Seven theme we added a 'danger' variant that actually looks more like a link:

Screen Shot 2013-10-30 at 09.12.22.png

This required us to override all the default button styling and undo it:

.button--danger {
  display: inline;
  cursor: pointer;
  padding: 0;
  border: 0;
  border-radius: 0;
  box-shadow: none;
  background: none;
  -webkit-appearance: none;
     -moz-appearance: none;
  color: #c72100;
  font-weight: 400;
  text-decoration: underline;
}

The ideal markup here would be to to remove the button classes completely and simply style it as a link, without having to undo the base styling.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

InternetDevels’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
3.22 KB

Patch attached.

LewisNyman’s picture

Status: Needs review » Needs work

The tricky part here is that we can't change the original function in form.inc. We have to modify the classes in the theme layer, just in Seven.

The last submitted patch, 1: replacebuttonwithlink-2123731-1.patch, failed testing.

InternetDevels’s picture

Status: Needs work » Needs review
FileSize
3.31 KB
2.14 KB

Moved changes to the Seven theme.

sun’s picture

Note that #216064: Entity form "Delete" button triggers server-side + HTML5 form validation; change "Delete" button to a link is about to replace all Delete buttons with

a.button.button--danger

That change is both a win and also a conflict for Seven. ;)

You want the a.button--danger part.

You do not want the .button part. :)

If you're overriding the full button CSS anyway already, then you could simply do this though:

.button:not(a.button--danger) {
  [button styles here]
}
.button--danger {
  color: red;
}
a.button--danger {
  text-decoration: underline;
}
yuki77’s picture

Assigned: Unassigned » yuki77

I'm working on it

sun’s picture

btw, I would not recommend to do what the issue title suggests (using link--danger instead of button--danger).

That is, because doing so would require you to write some very sophisticated preprocess (or whatever) code in seven.theme that is guaranteed to replace all buttons of #button_type 'danger' with links. — Contributed modules will use danger buttons in their forms, so you'd have to ensure that every possible instance is replaced accordingly.

I'd recommend to change the issue title + summary; basically to implement #5.

yuki77’s picture

Title: Replace button--danger with link--danger » edit CSS file using .button:not(a.button--danger) so that no reset is needed on button--danger
yuki77’s picture

Thanks sun, I changed the title!

yuki77’s picture

Hi,

So I changed the css, added :not on button css so that the reset on the button--danger is minimum. I wasn't sure if there would be any occasion where .button-danger and .button-primary so I also added :not on Button primary, but if there would be no such occasion I will remove it.

Hope it works!

Status: Needs review » Needs work

The last submitted patch, 10: drupal8-button--dangercss-2123731-10.patch, failed testing.

rteijeiro’s picture

@yuki77 it seems that you have to update your codebase and try to apply the patch. It's what we call a Re-roll.

Try: git fetch (to get latest codebase)
And then try to apply your patch changes. It seems that some files has been changed while you were working on this patch.

Let me know if you need more help and thanks for your contribution!

yuki77’s picture

Hi,

So I created a new one, Hope this one works...! And Please let me know if I can remove :not from .button--primary

Jalandhar’s picture

Status: Needs work » Needs review

Changing status to Needs review to run tests.

LewisNyman’s picture

Status: Needs review » Needs work

Thanks for the patch, I reviewed it:

  1. +++ b/core/themes/seven/css/components/buttons.theme.css
    @@ -11,7 +11,22 @@
    -.button {
    +
    +
    +.button{
    

    We don't need these extra blank lines here. Also our coding standards request a space between the .button selector and the curly bracket.

  2. +++ b/core/themes/seven/css/components/buttons.theme.css
    @@ -63,7 +78,7 @@
    -.button--primary {
    +.button--primary:not(.button--danger){
       border-color: #1e5c90;
    

    We don't need to add the :not() to the .button--primary. They aren't designed to be combined.

  3. +++ b/core/themes/seven/css/components/buttons.theme.css
    @@ -149,12 +164,34 @@
    +.button--danger:disabled,
    +.button--danger.is-disabled {
    + color: #737373;
    + cursor: default;
    +}
    +
    +
    

    Careful with hoe many blank lines you leave between components/variants. We only need one.

  4. +++ b/core/themes/seven/css/components/buttons.theme.css
    @@ -149,12 +164,34 @@
    - */
    + *
    + * we no longer need this since we added :not?
    +
     .button--danger {
       display: inline;
       cursor: pointer;
    @@ -191,3 +228,4 @@
    
    @@ -191,3 +228,4 @@
       box-shadow: none;
       background: none;
     }
    + */
    \ No newline at end of file
    

    Yeah lets delete this. Also make sure you leave a blank line at the end of the file

Jalandhar’s picture

Assigned: yuki77 » Unassigned
Status: Needs work » Needs review
FileSize
3.33 KB

Updating patch with changes said in #15. Please review.

Status: Needs review » Needs work

The last submitted patch, 16: drupal8-css_button_danger-2123731-16.patch, failed testing.

Jalandhar’s picture

Status: Needs work » Needs review
FileSize
2.49 KB

Another try at patch.

LewisNyman’s picture

+++ b/core/themes/seven/css/components/buttons.theme.css
@@ -12,6 +12,19 @@
 .button {
+  padding: 0;
+  margin: 0;
+  border: 0;
+  border-radius: 0;
+  box-shadow: none;
+  background: none;
+  display: inline;
+  -webkit-appearance: none;
+     -moz-appearance: none;
+  -webkit-font-smoothing: antialiased; /* 3 */
+}
+

It seems a bit weird to add such a heavy reset here? Especially when one exists already in buttons.css. Do we need all of these? Maybe we only need a few more lines in buttons.css.

LewisNyman’s picture

Status: Needs review » Needs work
nesta_’s picture

Assigned: Unassigned » nesta_
Issue tags: +DrupalCampSpain
nesta_’s picture

Assigned: nesta_ » Unassigned
Status: Needs work » Needs review
FileSize
2.1 KB
1.71 KB

With this patch I add the code from the class ".button--danger" to "button.css", witch contains the same property the clases format that it was declarated before.

I've cleaned the "buttons.theme.css" and the other elements with "button" class will not be rendered.

LewisNyman’s picture

This is definitely tidier than before, but if we use the :not() selector then we don't have to override the .button styles at all?

Bojhan’s picture

Assigned: Unassigned » LewisNyman

@Lewis Can this go in?

LewisNyman’s picture

Status: Needs review » Needs work
+++ b/core/themes/seven/css/components/buttons.css
@@ -45,3 +45,16 @@
+a.button--danger {

Why are we adding an element here? Shouldn't this apply to button elements rather then links?

Aleksandar_P’s picture

I combined all the previous patches and created one which uses :not(.button--danger) because :not selector can accept a simple selector as argument.
No additional reset of .button--danger is needed.

BarisW’s picture

Status: Needs review » Needs work

Visually, it looks good on desktop. It doesn't look good on mobile though (see screenshot), so it needs a bit of work there.

Apart from this, I really get a bit nauseous from using a 'button' class on a link, that looks like a link (not a button). Don't know of a better solution now thought.

BarisW’s picture

FileSize
27.13 KB

And the screenshot.

Mobile button layout

Aleksandar_P’s picture

'Save' and 'Delete' buttons styled for rwd.
'Save' centered with parent element text alignment set to center.
Padding and margins on buttons changed in order to fit 780px screen and to be centered for mobile devices.

LewisNyman’s picture

Status: Needs review » Needs work
  1. +++ b/core/themes/seven/css/components/buttons.theme.css
    @@ -11,8 +11,8 @@
    -.button {
    -  padding: 4px 1.5em;  /* 1 */
    +.button:not(.button--danger) {
    +  padding: 4px 0.7em;  /* 1 */
    

    We've changed the padding here, I don't think we should do that.

  2. +++ b/core/themes/seven/css/components/buttons.theme.css
    @@ -29,7 +29,9 @@
    -.button:focus {
    +.button:focus,
    +.button:not(.button--danger):hover,
    +.button:not(.button--danger):focus {
    

    Also I noticed here that we've added a :hover selector here that was not there before.

  3. +++ b/core/themes/seven/css/components/buttons.theme.css
    @@ -37,14 +39,17 @@
    -.button:hover {
    +.button:hover,
    +.button:not(.button--danger):hover {
    ...
    -.button:focus {
    +.button:focus,
    +.button:not(.button--danger):focus {
    ...
    -.button:active {
    +.button:active,
    +.button:not(.button--danger):active {
    

    We need to replace the selectors here, not just add to them, otherwise they still apply incorrectly to button--danger

  4. +++ b/core/themes/seven/css/components/buttons.theme.css
    @@ -135,45 +140,37 @@
     .button--danger {
       display: inline;
    +  margin-left: 0;
    

    I'm not sure why we are adding margin left here? Margin does not apply to display: inline elements.

  5. +++ b/core/themes/seven/css/components/buttons.theme.css
    @@ -135,45 +140,37 @@
    +  .button--danger:hover,
    +  .button--danger:focus,
    +  .button--danger:active {
    +    background: none;
    +    border: none;
    +    box-shadow: none;
    +  }
    

    We shouldn't need to reset these styles if we implement the styling above correctly

  6. +++ b/core/themes/seven/css/components/dropbutton.component.css
    @@ -222,6 +222,14 @@
    +@media screen and (max-width: 600px) {
    +  .js .form-actions .dropbutton .dropbutton-action > * {
    +    text-align: center;
    +    padding: 4px 0 4px 26px;
    +  }
    +}
    

    We are modifying the dropbutton component, which seems out of scope of this issue.

LewisNyman’s picture

Assigned: LewisNyman » Unassigned
Aleksandar_P’s picture

Status: Needs work » Needs review
FileSize
2.15 KB

I tried another approach to the issue. Fixed only the overriding of button--danger with button:not(.button--danger).

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

nesta_’s picture

Issue tags: +DrupalCampES

Retest to 8.1.x and add tag for DrupalCampES

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Sakthivel M’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

#33 Patch failed

Sakthivel M’s picture

Status: Needs work » Needs review
FileSize
2.1 KB

#44 Please review the patch

chetanbharambe’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
522.53 KB
527.31 KB

Verified and tested patch #44.
Patch applied successfully and looks good to me.

Testing Steps:
# Goto: Any content
# Edit the content
# Check the delete button
# Inspect it and check the CSS classes

Expected Results:
# Remove the button classes completely.

Looks good to me.
Can be a move to RTBC.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 44: button-danger-no-reset-2123731-42.patch, failed testing. View results

vsujeetkumar’s picture

Status: Needs work » Needs review

Re-test on #44 and its pass, Move to NR.

yogeshmpawar’s picture

Issue tags: -Needs reroll

Removing Needs reroll tag as it is no longer needed.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

longwave’s picture

Project: Drupal core » Seven
Version: 9.4.x-dev » 1.0.0-alpha1
Component: Seven theme » Code

The Seven theme has been removed from Drupal 10 core. I confirmed that this issue only affects Seven and no other themes included with Drupal core, so I am moving this to the contributed Seven project.

gaurav-mathur’s picture

Assigned: Unassigned » gaurav-mathur
gaurav-mathur’s picture

I applied patch #44 With this patch i add the code from the class like ".button--danger" witch contains the same property the clases format that it was declarated before. its work fine.

gaurav-mathur’s picture

Assigned: gaurav-mathur » Unassigned
roshni27’s picture

Issue tags: -
FileSize
84.56 KB

Drupal Version :10.1.2
PHP Version 8.1.16
Seven Theme : 1.0.0-alpha1
No patch applied.
I have shared a screenshot in which the 'delete' button style appeared correct.