Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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:
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.
Comment | File | Size | Author |
---|---|---|---|
#56 | Screenshot from 2023-08-10 21-01-56.png | 84.56 KB | roshni27 |
#54 | 2123731-afterpatch.jpg | 8.63 KB | gaurav-mathur |
#54 | 2123731-beforepatch.jpg | 8.28 KB | gaurav-mathur |
#45 | After Patch 2123731.png | 527.31 KB | chetanbharambe |
#45 | Before Patch 2123731.png | 522.53 KB | chetanbharambe |
Comments
Comment #1
InternetDevels CreditAttribution: InternetDevels commentedPatch attached.
Comment #2
LewisNymanThe 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.
Comment #4
InternetDevels CreditAttribution: InternetDevels commentedMoved changes to the Seven theme.
Comment #5
sunNote 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
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:
Comment #6
yuki77 CreditAttribution: yuki77 commentedI'm working on it
Comment #7
sunbtw, 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.
Comment #8
yuki77 CreditAttribution: yuki77 commentedComment #9
yuki77 CreditAttribution: yuki77 commentedThanks sun, I changed the title!
Comment #10
yuki77 CreditAttribution: yuki77 commentedHi,
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!
Comment #12
rteijeiro CreditAttribution: rteijeiro commented@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!
Comment #13
yuki77 CreditAttribution: yuki77 commentedHi,
So I created a new one, Hope this one works...! And Please let me know if I can remove :not from .button--primary
Comment #14
Jalandhar CreditAttribution: Jalandhar commentedChanging status to Needs review to run tests.
Comment #15
LewisNymanThanks for the patch, I reviewed it:
We don't need these extra blank lines here. Also our coding standards request a space between the .button selector and the curly bracket.
We don't need to add the :not() to the .button--primary. They aren't designed to be combined.
Careful with hoe many blank lines you leave between components/variants. We only need one.
Yeah lets delete this. Also make sure you leave a blank line at the end of the file
Comment #16
Jalandhar CreditAttribution: Jalandhar commentedUpdating patch with changes said in #15. Please review.
Comment #18
Jalandhar CreditAttribution: Jalandhar commentedAnother try at patch.
Comment #19
LewisNymanIt 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.
Comment #20
LewisNymanComment #21
nesta_ CreditAttribution: nesta_ commentedComment #22
nesta_ CreditAttribution: nesta_ commentedWith 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.
Comment #23
LewisNymanThis is definitely tidier than before, but if we use the :not() selector then we don't have to override the .button styles at all?
Comment #24
Bojhan CreditAttribution: Bojhan commented@Lewis Can this go in?
Comment #25
LewisNymanWhy are we adding an element here? Shouldn't this apply to button elements rather then links?
Comment #26
Aleksandar_P CreditAttribution: Aleksandar_P commentedI 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.
Comment #27
BarisW CreditAttribution: BarisW commentedVisually, 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.
Comment #28
BarisW CreditAttribution: BarisW commentedAnd the screenshot.
Comment #30
Aleksandar_P CreditAttribution: Aleksandar_P commented'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.
Comment #31
LewisNymanWe've changed the padding here, I don't think we should do that.
Also I noticed here that we've added a :hover selector here that was not there before.
We need to replace the selectors here, not just add to them, otherwise they still apply incorrectly to button--danger
I'm not sure why we are adding margin left here? Margin does not apply to display: inline elements.
We shouldn't need to reset these styles if we implement the styling above correctly
We are modifying the dropbutton component, which seems out of scope of this issue.
Comment #32
LewisNymanComment #33
Aleksandar_P CreditAttribution: Aleksandar_P at Develomon for FFW commentedI tried another approach to the issue. Fixed only the overriding of button--danger with button:not(.button--danger).
Comment #35
nesta_ CreditAttribution: nesta_ at La Drupalera by Emergya commentedRetest to 8.1.x and add tag for DrupalCampES
Comment #43
Sakthivel M CreditAttribution: Sakthivel M at QED42 for Drupal India Association commented#33 Patch failed
Comment #44
Sakthivel M CreditAttribution: Sakthivel M at QED42 for Drupal India Association commented#44 Please review the patch
Comment #45
chetanbharambe CreditAttribution: chetanbharambe at QED42 for Drupal India Association commentedVerified 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.
Comment #47
vsujeetkumar CreditAttribution: vsujeetkumar at Srijan | A Material+ Company for Drupal India Association commentedRe-test on #44 and its pass, Move to NR.
Comment #48
yogeshmpawarRemoving Needs reroll tag as it is no longer needed.
Comment #52
longwaveThe 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.
Comment #53
gaurav-mathur CreditAttribution: gaurav-mathur at Dotsquares Ltd. commentedComment #54
gaurav-mathur CreditAttribution: gaurav-mathur at Dotsquares Ltd. commentedI 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.
Comment #55
gaurav-mathur CreditAttribution: gaurav-mathur at Dotsquares Ltd. commentedComment #56
roshni27 CreditAttribution: roshni27 at Valuebound for Valuebound commentedDrupal 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.