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.
Problem/Motivation
As per Gabor's comment in Dropbutton style update for Seven
Dropbuttons in Bartik need some love.
Proposed resolution
Here's how the new dropbuttons look like in Bartik:
LTR (normal, opened, selected):
RTL:
Remaining tasks
Test and review.
User interface changes
Bartik dropbuttons will look great!
API changes
None
Comment | File | Size | Author |
---|---|---|---|
#46 | bartik_dropbutton-2278415-46.patch | 5.87 KB | Upchuk |
#46 | interdiff-46.txt | 1.3 KB | Upchuk |
#42 | bartik_dropbutton-2278415-42.patch | 5.85 KB | Upchuk |
#42 | interdiff-42.txt | 1.4 KB | Upchuk |
#42 | Screen Shot 2014-11-07 at 14.23.56.png | 13.26 KB | Upchuk |
Comments
Comment #1
Gábor HojtsyYay, thanks!
Comment #2
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #3
Anonymous (not verified) CreditAttribution: Anonymous commentedI've had a stab at this with the attached patch. Any thoughts?
Comment #4
Anonymous (not verified) CreditAttribution: Anonymous commentedAlso, in this second patch, I've made a small alteration to the dropdown module's CSS in order to remove default ul margin/padding when JS is disabled.
Comment #5
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #6
Bojhan CreditAttribution: Bojhan commented@James Am I correct that your last patch is incomplete?
Comment #7
Anonymous (not verified) CreditAttribution: Anonymous commentedSorry, it's just the patch on #3 really – I'll put the second patch in a separate issue.
Comment #8
tim.plunkettSee also #2282599: Dropdown menus in most of core are broken in Stark
Comment #9
jwilson3Why introduce extra specificity here?
Comment #10
G-raph CreditAttribution: G-raph commentedGonna test this one with and without the extra specifity.
Comment #11
G-raph CreditAttribution: G-raph commentedI tested this one with and without the extra specificity, mentioned in #9. This is a correct remark, because the result wasn't changed.
I created a patch (and interdiff.txt) for this.
This is screenshot with patch #3:
This is screenshot when removed the extra specificity:
Comment #12
lauriii1) I tested this on Firefox and it looks like this:
2) Shouldn't we use
.svg
instead of.png
?Edit: there's only file removals so ignore point 2.
Comment #13
G-raph CreditAttribution: G-raph commentedgonna test and fix this.
Comment #14
G-raph CreditAttribution: G-raph commentedLauriii: I think you didn't take the last patch?
Comment #15
G-raph CreditAttribution: G-raph commentedComment #16
G-raph CreditAttribution: G-raph commentedoh nooo, that styling is also in /core/misc/dropbutton/dropbutton.css.
Going to make a new patch.
Comment #17
G-raph CreditAttribution: G-raph commentedOK, I figured it out...
I have to put the status of this issue to "postponed", because /core/misc/dropbutton/dropbutton.css overwrites the patch from #11.
The overwriting part can't be fixed in this ticket, it must be fixed in #1989470: Dropbutton style update for Seven
So, the last patch I committed (#11) is the right one for this issue!
See screenshot for more details:
Comment #18
G-raph CreditAttribution: G-raph commentedFrom postponed to need review, because I fixed the overwriting part of "#1989470: Dropdown styling" with an !important. Normally I'm not so happy with !importants, but in this case we don't have to change the css from #1989470 + bartik uses !importants a lot, so...
This issue looks fixed now. New patch in attach.
Comment #19
mgiffordAny reason not to use a SVG file? I'm not sure how to do a png to svg conversion, but...
Comment #20
G-raph CreditAttribution: G-raph commentedThe png is removed in the patch, so there is no png and no svg.
Comment #21
mgiffordTested here - http://s7945023a9d9e93f.s2.simplytest.me/node/add/article
It looks fine to me...
Comment #22
herom CreditAttribution: herom commentedAdded a bit of RTL CSS.
RTL Before:
RTL After:
Comment #23
Gábor HojtsyCan we get up to date screenshots in the issue summary of the after state in LTR and RTL? Visually it looks great, I guess we need a frontend reviewer on this for CSS.
Comment #24
herom CreditAttribution: herom commentedUpdated the issue summary with screenshots.
Comment #25
LewisNymanGood work! The dropbutton mark up is so confusing... See #2278473: Simplify Dropbutton markup in line with our CSS standards
I did find a few low level points.
We can remove -moz- and -o- lines. See http://caniuse.com/#feat=css-gradients
These units should in ems
Comment #26
G-raph CreditAttribution: G-raph commentedComment #27
G-raph CreditAttribution: G-raph commentedRemoved the -moz- and -o- lines on gradients + converted px to em.
New patch in attach.
Comment #28
LewisNymanI found a few issues that effect normal buttons:
The borders of the drop buttons on the content type table look a little weird:
Also, is it me or do the normal buttons look better with a border radius of 1em?
Comment #29
LewisNymanSorry that second screenshot should of been:
Comment #30
herom CreditAttribution: herom commentedNo, it isn't just you: #2334363: Open hoverstate dropbutton to overflow on border
Comment #31
Karmen CreditAttribution: Karmen commentedWell, I adjusted the border radius for all buttons -not only de dropdown buttons-. Correct the border-bottom for the dropdown, and its hover.
If there is something wrong, tell me please :)
Comment #32
Karmen CreditAttribution: Karmen commentedSorry, forget the patch :P
Comment #33
rteijeiro CreditAttribution: rteijeiro commented@Karmen: Could you provide an interdiff please?
Comment #34
Karmen CreditAttribution: Karmen commentedThere is :)
Comment #35
G-raph CreditAttribution: G-raph commentedComment #36
G-raph CreditAttribution: G-raph commentedI applied the above patch #32, but there was a problem with the border-radius in :hover state. See screenshot of the problem:
I fixed this issue, so new patch in attach.
Screenshots of fixed hover states RTL and LTR:
Comment #37
LewisNymanThanks for all the work! For some reason now the buttons don't look as rounded, I think we're better off keep normal buttons the same border-radius as before and just changing the dropbutton border-radius. They need to be different to look good:
Comment #38
G-raph CreditAttribution: G-raph commentedThis looks much better indeed. I reverted the border-radius of the normal button. Also changed the border-radius of the dropdown button styling from 0.9em to 1em .
New patch.
See screenshots of the result:
Comment #40
LewisNymanThanks for providing the screenshots. I also manually tested the patch to check RTL and it looks good. Just one or two picky CSS comments.
I don't think we need to make this change because the other border properties are defined on the line above
I think we should stick to defining units in ems in all cases
Comment #41
G-raph CreditAttribution: G-raph commentedThx for the follow-up!
Fixed the comments:
- changed the "border-bottom: 1px solid #b4b4b4" to "border-bottom-color: #b4b4b4".
- converted the padding and border-radius from px to em
New patch in attach.
Here some screenshots of the result (RTL + LTR):
Comment #42
Upchuk CreditAttribution: Upchuk commentedThese are some nice buttons. Found however some glitches with dropbuttons (single and multiple) created with links rather than buttons or inputs:
So these were fixed, please check it out. Let me know if there's need for some css refactoring.
D
Comment #43
LewisNymanGreat, I tested the patch to verify the changes. The code changes look good
Comment #44
Wim LeersOne less PNG more than offsets the extra CSS. Great work!
Comment #45
herom CreditAttribution: herom commentedHere's some RTL nitpick.
Add the RTL rule immediately after its LTR equivalent.
Add an /* LTR */ comment after the properties here.
Comment #46
Upchuk CreditAttribution: Upchuk commentedHere we go.
Comment #47
lauriiiSending to testbot :)
Comment #48
LewisNymanGreat work. Thanks @herom for the RTL review.
Comment #49
alexpottThis issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed f6fb464 and pushed to 8.0.x. Thanks!