Problem/Motivation

As per Gabor's comment in Dropbutton style update for Seven

Dropbutton looking bad in Bartik
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

CommentFileSizeAuthor
#46 bartik_dropbutton-2278415-46.patch5.87 KBUpchuk
#46 interdiff-46.txt1.3 KBUpchuk
#42 bartik_dropbutton-2278415-42.patch5.85 KBUpchuk
#42 interdiff-42.txt1.4 KBUpchuk
#42 Screen Shot 2014-11-07 at 14.23.56.png13.26 KBUpchuk
#42 Screen Shot 2014-11-07 at 13.34.44.png13.23 KBUpchuk
#41 bartik_dropbutton-2278415-41.patch4.84 KBG-raph
#41 interdiff.txt785 bytesG-raph
#41 Schermafbeelding 2014-11-06 om 13.14.14.png20.55 KBG-raph
#41 Schermafbeelding 2014-11-06 om 13.13.50.png20.02 KBG-raph
#38 interdiff.txt3.07 KBG-raph
#38 bartik_dropbutton-2278415-38.patch4.52 KBG-raph
#38 Schermafbeelding 2014-10-01 om 17.05.20.png30.64 KBG-raph
#38 Schermafbeelding 2014-10-01 om 17.05.14.png23.15 KBG-raph
#38 Schermafbeelding 2014-10-01 om 17.05.10.png23 KBG-raph
#37 Screenshot 2014-09-29 15.33.05.jpg18.6 KBLewisNyman
#36 interdiff.txt544 bytesG-raph
#36 bartik_dropbutton-2278415-36.patch4.88 KBG-raph
#36 Schermafbeelding 2014-09-29 om 12.09.33.png11.31 KBG-raph
#36 Schermafbeelding 2014-09-29 om 11.44.20.png11.27 KBG-raph
#36 Schermafbeelding 2014-09-29 om 11.26.17.png12.08 KBG-raph
#36 Schermafbeelding 2014-09-29 om 11.25.48.png11.12 KBG-raph
#34 interdiff.txt1.12 KBKarmen
#32 2278415-bartik_buttons-32.patch4.72 KBKarmen
#29 Screenshot 2014-09-23 22.22.45.jpg51.54 KBLewisNyman
#28 Screenshot 2014-09-23 22.22.45.jpg51.54 KBLewisNyman
#28 Screenshot 2014-09-23 22.22.25.jpg8.37 KBLewisNyman
#27 Schermafbeelding 2014-09-19 om 13.09.39.png9.45 KBG-raph
#27 Schermafbeelding 2014-09-19 om 13.09.34.png7.32 KBG-raph
#27 bartik_dropbutton-2278415-27.patch4.54 KBG-raph
#27 interdiff.txt2.06 KBG-raph
#22 bartik-dropdown-rtl-fixed.png2.99 KBherom
#22 bartik-dropdown-rtl-bad.png2.67 KBherom
#22 bartik_dropbutton-2278415-22.patch4.67 KBherom
#22 interdiff-2278415-18-22.txt1.85 KBherom
#21 Screen Shot 2014-09-16 at 5.33.31 PM.png135 KBmgifford
#18 interdiff.txt445 bytesG-raph
#18 bartik_dropbutton-2278415-18.patch4.09 KBG-raph
#17 2278415-postponed.jpg283.15 KBG-raph
#12 Screen Shot 2014-08-31 at 14.52.02.png19.51 KBlauriii
#11 bartik_dropbutton-2278415-11.patch4.08 KBG-raph
#11 interdiff.txt472 bytesG-raph
#11 2278415_without_ extra_spec.png19.23 KBG-raph
#11 2278415_with extra_spec.png19.24 KBG-raph
#4 2278415-bartik-dropbutton-4.patch826 bytesAnonymous (not verified)
#3 after-open-selected.png13.69 KBAnonymous (not verified)
#3 after-open.png12.8 KBAnonymous (not verified)
#3 after.png10.84 KBAnonymous (not verified)
#3 2278415-bartik-dropbutton-3.patch4.13 KBAnonymous (not verified)
DropButtonBartik.png21.41 KBemma.maria
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Gábor Hojtsy’s picture

Yay, thanks!

Anonymous’s picture

Assigned: Unassigned »
Anonymous’s picture

Status: Active » Needs review
FileSize
4.13 KB
10.84 KB
12.8 KB
13.69 KB

I've had a stab at this with the attached patch. Any thoughts?

Anonymous’s picture

Also, 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.

Anonymous’s picture

Assigned: » Unassigned
Bojhan’s picture

@James Am I correct that your last patch is incomplete?

Anonymous’s picture

Sorry, it's just the patch on #3 really – I'll put the second patch in a separate issue.

tim.plunkett’s picture

jwilson3’s picture

Status: Needs review » Needs work
+++ b/core/themes/bartik/css/style.css
@@ -1763,18 +1767,60 @@ div.admin-panel .description {
-.js .dropbutton-widget {
-  background-color: white;
-  border-radius: 5px;
...
+.js .form-actions .dropbutton-widget {

Why introduce extra specificity here?

G-raph’s picture

Assigned: Unassigned » G-raph

Gonna test this one with and without the extra specifity.

G-raph’s picture

Assigned: G-raph » Unassigned
Status: Needs work » Needs review
Issue tags: +FUDK
FileSize
19.24 KB
19.23 KB
472 bytes
4.08 KB

I 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:

lauriii’s picture

Status: Needs review » Needs work
FileSize
19.51 KB

1) 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.

G-raph’s picture

Assigned: Unassigned » G-raph

gonna test and fix this.

G-raph’s picture

Assigned: G-raph » Unassigned

Lauriii: I think you didn't take the last patch?

G-raph’s picture

Status: Needs work » Needs review
G-raph’s picture

Assigned: Unassigned » G-raph
Status: Needs review » Needs work

oh nooo, that styling is also in /core/misc/dropbutton/dropbutton.css.

Going to make a new patch.

G-raph’s picture

Assigned: G-raph » Unassigned
Status: Needs work » Postponed
FileSize
283.15 KB

OK, 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:

G-raph’s picture

Status: Postponed » Needs review
FileSize
4.09 KB
445 bytes

From 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.

mgifford’s picture

Any reason not to use a SVG file? I'm not sure how to do a png to svg conversion, but...

G-raph’s picture

The png is removed in the patch, so there is no png and no svg.

mgifford’s picture

herom’s picture

Added a bit of RTL CSS.

RTL Before:



RTL After:

Gábor Hojtsy’s picture

Can 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.

herom’s picture

Issue summary: View changes

Updated the issue summary with screenshots.

LewisNyman’s picture

Status: Needs review » Needs work
Related issues: +#2278473: Simplify Dropbutton markup in line with our CSS standards

Good 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.

  1. +++ b/core/themes/bartik/css/style.css
    @@ -1373,9 +1373,13 @@ div.messages,
    +  background-image: -webkit-linear-gradient(top, #f3f3f3, #e8e8e8);
    +  background-image: -moz-linear-gradient(top, #f3f3f3, #e8e8e8);
    +  background-image: -o-linear-gradient(top, #f3f3f3, #e8e8e8);
    +  background-image: linear-gradient(to bottom, #f3f3f3, #e8e8e8);
    
    @@ -1855,18 +1859,74 @@ div.admin-panel .description {
    +  background-image: -webkit-linear-gradient(top, #f3f3f3, #e8e8e8);
    +  background-image: -moz-linear-gradient(top, #f3f3f3, #e8e8e8);
    +  background-image: -o-linear-gradient(top, #f3f3f3, #e8e8e8);
    +  background-image: linear-gradient(to bottom, #f3f3f3, #e8e8e8);
    ...
    +  background-image: -webkit-linear-gradient(top, #e8e8e8, #d2d2d2);
    +  background-image: -moz-linear-gradient(top, #e8e8e8, #d2d2d2);
    +  background-image: -o-linear-gradient(top, #e8e8e8, #d2d2d2);
    +  background-image: linear-gradient(to bottom, #e8e8e8, #d2d2d2);
    

    We can remove -moz- and -o- lines. See http://caniuse.com/#feat=css-gradients

  2. +++ b/core/themes/bartik/css/style.css
    @@ -1855,18 +1859,74 @@ div.admin-panel .description {
    +  margin: 2px 0;
    +  border-radius: 15px;
    ...
    +  padding: 4px 17px;
    

    These units should in ems

G-raph’s picture

Assigned: Unassigned » G-raph
G-raph’s picture

Assigned: G-raph » Unassigned
Status: Needs work » Needs review
FileSize
2.06 KB
4.54 KB
7.32 KB
9.45 KB

Removed the -moz- and -o- lines on gradients + converted px to em.

New patch in attach.

LewisNyman’s picture

Status: Needs review » Needs work
FileSize
8.37 KB
51.54 KB

I 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?

LewisNyman’s picture

Sorry that second screenshot should of been:

herom’s picture

Karmen’s picture

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

Well, 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 :)

Karmen’s picture

Sorry, forget the patch :P

rteijeiro’s picture

@Karmen: Could you provide an interdiff please?

Karmen’s picture

FileSize
1.12 KB

There is :)

G-raph’s picture

Assigned: Unassigned » G-raph
G-raph’s picture

I 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:


LewisNyman’s picture

Status: Needs review » Needs work
FileSize
18.6 KB

Thanks 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:

G-raph’s picture

This 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:



LewisNyman’s picture

Status: Needs review » Needs work

Thanks for providing the screenshots. I also manually tested the patch to check RTL and it looks good. Just one or two picky CSS comments.

  1. +++ b/core/themes/bartik/css/style.css
    @@ -1372,7 +1372,7 @@ div.messages,
    -  border-bottom-color: #b4b4b4;
    +  border-bottom: 1px solid #b4b4b4;
    

    I don't think we need to make this change because the other border properties are defined on the line above

  2. +++ b/core/themes/bartik/css/style.css
    @@ -1380,8 +1380,8 @@ div.messages,
    -  padding: 0.32em 1.063em;
    -  border-radius: 0.9em;
    +  padding: 4px 17px;
    

    I think we should stick to defining units in ems in all cases

G-raph’s picture

Thx 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):


Upchuk’s picture

These 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

LewisNyman’s picture

Status: Needs review » Reviewed & tested by the community

Great, I tested the patch to verify the changes. The code changes look good

Wim Leers’s picture

One less PNG more than offsets the extra CSS. Great work!

herom’s picture

Status: Reviewed & tested by the community » Needs work

Here's some RTL nitpick.

  1. +++ b/core/themes/bartik/css/style.css
    @@ -1892,18 +1894,115 @@ div.admin-panel .description {
    +.js .dropbutton-widget .button:hover {
    +  border-radius: 1em 0 0 1em;
    +}
    ...
    +[dir="rtl"].js  .dropbutton-widget .button:hover {
    +  border-radius: 0 1em 1em 0;
    +}
    

    Add the RTL rule immediately after its LTR equivalent.

  2. +++ b/core/themes/bartik/css/style.css
    @@ -1892,18 +1894,115 @@ div.admin-panel .description {
    +.js .dropbutton-multiple .dropbutton-widget .dropbutton-action:first-child a:hover {
    +  border-radius: 1em 0 0 1em;
    +}
    ...
    +.js .dropbutton-multiple.open .dropbutton-widget .dropbutton-action:first-child a:hover {
    +  border-radius: 1em 0 0 0;
    +}
    

    Add an /* LTR */ comment after the properties here.

Upchuk’s picture

Here we go.

lauriii’s picture

Status: Needs work » Needs review

Sending to testbot :)

LewisNyman’s picture

Status: Needs review » Reviewed & tested by the community

Great work. Thanks @herom for the RTL review.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

This 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!

  • alexpott committed f6fb464 on 8.0.x
    Issue #2278415 by G-raph, Upchuk, Karmen, JamesLefrère, herom | emma....

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.