Although we can't style every part of a dropdown without having to write a lot of heavy javascript webkit browsers allow us to style just the initial select element.

We have a design for this here that is consistent with the text field styling in #1986418: Update textfield & textarea style

https://groups.drupal.org/node/283223#Basic_Form_Controls

Test Instructions

Check the select elements in a Webkit browser with...
1. LTR attribute in the tag.
2. RTL attribute in the tag.

Remaining tasks

  1. Remove no-svg support

Comments

lewisnyman’s picture

Issue tags: +drupalcampldn
emma.maria’s picture

Assigned: Unassigned » emma.maria
Issue tags: -drupalcampldn +DrupalCampLDN
lewisnyman’s picture

emma.maria’s picture

Assigned: emma.maria » Unassigned
Status: Active » Needs review
StatusFileSize
new3.46 KB

I have added webkit only styles for the select element. I have disabled the browser styles and applied the style guide styles plus the arrow using the Libricon icons.

Status: Needs review » Needs work

The last submitted patch, 4: select-style-2207391-4.patch, failed testing.

The last submitted patch, 4: select-style-2207391-4.patch, failed testing.

lewisnyman’s picture

+++ b/core/modules/comment/comment.module
@@ -226,7 +226,6 @@ function comment_count_unpublished() {
 function comment_field_instance_config_insert(FieldInstanceConfigInterface $instance) {
   if ($instance->getType() == 'comment' && !$instance->isSyncing()) {
     \Drupal::service('comment.manager')->addBodyField($instance->entity_type, $instance->getName());
-    \Drupal::cache()->delete('comment_entity_info');
   }
 }

Looks like this patch accidentally includes some changes from a recent commit

emma.maria’s picture

Status: Needs work » Needs review
StatusFileSize
new3.08 KB
new46.83 KB

Work carried out for this new patch:
Adjusted the arrow icon position and the right padding to accommodate the background icon.
Added hover, focus styles and RTL styles.

I have attached screenshots and this patch should pass :)

longwave’s picture

+    -webkit-font-smoothing: antialiased;
+    }
+    [dir="rtl"] select {
...
+    }
+}

Indentation looks off here?

lewisnyman’s picture

Status: Needs review » Needs work
+++ b/core/themes/seven/style.css
@@ -835,6 +834,63 @@ select.form-select:focus {
+        url("../../misc/icons/333333/caret-down.svg") no-repeat 1% 63%,
...
+    .no-svg select {
+      background: url("../../misc/icons/333333/caret-down.png"),
+      -webkit-linear-gradient(top, #f6f6f3, #e7e7df);
+    }

this property is missing “no-repeat 1% 63%”

There are also a few problems when the language is rtl and no SVG. We need to redeclare the multiple backgrounds again. It's a bit of a pain.

emma.maria’s picture

Assigned: Unassigned » emma.maria
emma.maria’s picture

Assigned: emma.maria » Unassigned
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new3.79 KB

I have fixed the issues pointed out in #9 and #10. I have also added the RTL no-svg styles that were missing.
I have listed all the different test scenarios in the issue summary.

rteijeiro’s picture

Status: Needs review » Needs work
StatusFileSize
new5.22 KB
new5.51 KB
new20.82 KB
new18.69 KB

Checked with different browsers and RTL languages too. In Chrome, Safari and Opera looks awesome!

Chrome

In Firefox looks a little bit weird when you open it. I'm not sure if it's possible to fix it.

Firefox Closed

Firefox Opened

RTL

lewisnyman’s picture

Aha there is still styling being applied to all browsers on focus:
select.form-select:focus

emma.maria’s picture

Status: Needs work » Needs review
StatusFileSize
new4.14 KB
new21.62 KB
new35.38 KB

I have removed all the non Webkit styles for Select elements. Firefox will only show the default browser styles.

rteijeiro’s picture

Status: Needs review » Needs work
StatusFileSize
new9.81 KB

Well done! Only one thing more. I noticed some misplacement when you open selectbox in Chrome.

I'm not sure but I think it was not here before. Could you check it, pleeeeeease?

lewisnyman’s picture

It looks like you can fix this by reducing the right padding of the select styling, which doesn't seem to do very much anyway.

emma.maria’s picture

Status: Needs work » Needs review
StatusFileSize
new48.33 KB
new31.27 KB
new4.11 KB

The font weight of the select boxes is causing the misplacement. We cannot target the styles of the options that pop up when clicked on so their text cannot be made bold to match. Therefore removing the bold font weight makes everything line up a lot better.

Status: Needs review » Needs work

The last submitted patch, 18: select-style-2207391-18.patch, failed testing.

The last submitted patch, 18: select-style-2207391-18.patch, failed testing.

emma.maria’s picture

Status: Needs work » Needs review
StatusFileSize
new4.11 KB
new49.54 KB
new33.04 KB

Updated patch - I changed the padding so you cannot see any of the right of the select box.

lewisnyman’s picture

Status: Needs review » Reviewed & tested by the community

Great, the select element looks good on Webkit and just looks normal in other browsers.

wim leers’s picture

Status: Reviewed & tested by the community » Needs review
Issue tags: +front-end performance

That's A LOT of additional CSS. And more HTTP requests to download the SVGs.

I'm sorry, but I really have to question the value here. Does the improved design really outweigh the decreased performance? Especially on mobile.

See https://speakerdeck.com/csswizardry/normalising-designs-for-better-quali....

lewisnyman’s picture

@Wim thanks for raising the concern, it's definitely something we have to consider. We have a problem now because it's not clear how we move forward and this issue is going to stagnate. We need an actual gate that we need to pass here. Any suggestions?

lewisnyman’s picture

Status: Needs review » Needs work

Ok I had a quick chat with Wim. There are a few things we can try to cut down on the weight.

  1. Can we remove the no-svg styling and just keep the default select button styling to cut down on CSS? Then we don't need to include the png and extra CSS at all.
  2. Maybe we can remove the SVG completely and use the browser default arrow?
Bojhan’s picture

How does it look with the browser default arrow? Their might be crisp that happens, but if not we can remove it.

lewisnyman’s picture

Yeah we can try and see if we can use the default arrow by removing -webkit-appearance: none;

tompagabor’s picture

StatusFileSize
new37.62 KB

I tested, what we can do, when removing -webkit-appearance: none;

cursor: pointer; -> working
padding: 1px 1.571em 2px 0.5em; /* LTR */ -> not working
border: 1px solid #a6a6a6; -> working
border-radius: 0.143em; -> not working
background-color: #f2f1eb; -> working, but make a gradient, with the color
color: #333333; -> working, also change default arrow color
text-decoration: none; -> not working
text-shadow: 0 1px hsla(0, 0%, 100%, 0.6); -> not working
font-size: 14px; -> working

test select without webkit appearance none

pakmanlh’s picture

StatusFileSize
new20.47 KB

Agree with the last test comments, and I think is necessary do the second point that @LewisNyman comment on #25, because if not the background icon appears over the defauls arrows. I attach a screenshot.
 none;

Bojhan’s picture

Issue tags: -front-end performance
lewisnyman’s picture

Issue summary: View changes
Issue tags: -DrupalCampLDN

It's worth exploring option 1 in #25 if #2286601: [policy] Drop support for browsers that don't support SVG is approved. That would half the CSS that we need for the arrow icon. Lets try it.

lewisnyman’s picture

Status: Needs work » Needs review
StatusFileSize
new2.65 KB
new2.42 KB

Here's a patch will all the no-svg fallbacks removed. I've also attempted to reduce up the multiple background declarations by using background-position and background-image where I can.

I wasn't able to test this because my local install is a bit borked.

lewisnyman’s picture

I got my local working, here are some screenshots. Focus and hover works great as well :)

Seven's style.css has gone from 31kb to 32kb, not too bad.

Status: Needs review » Needs work

The last submitted patch, 32: select-style-2207391-32.patch, failed testing.

lewisnyman’s picture

Issue tags: +Needs reroll, +svg
lokapujya’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new2.54 KB

Rerolled.

wim leers’s picture

Status: Needs review » Needs work
StatusFileSize
new2.63 KB
+++ b/core/misc/icons/333333/caret-down.svg
@@ -0,0 +1 @@
\ No newline at end of file

Can you add a newline here? :)

In any case, this patch looks much better already. Thanks :)

I still have some questions:

  1. It still scares me that we're adding fifteen properties for the select selector. AFAICT, the text-decoration and text-shadow and properties have zero effect (in Chrome and Safari). font-size: 14px as a fallback for font-size: 0.875rem also seems useless, because all Webkit browsers support rem in the first place! :)
  2. I don't know the proper terminology, but I see that the top of capital letters is much closer to the top of the <select> than the bottom of letters is to the bottom of the <select>. It feels off-balance. Or is that just me, because I'm used to the default styling?
    This is what it looks like for me (in Chrome):
  3. I didn't realize that this only is targeting WebKit. http://stackoverflow.com/questions/9620404/styling-a-select-element-in-f... and http://stackoverflow.com/a/21163651 suggest that this is possible in Firefox as well as IE, but perhaps it's too fragile? Please either make it work in all browsers, or update the issue title to reflect that this only targets Webkit.
  4. If we only target Webkit for such relatively prominent styling: do we have a precedent for that?

The last submitted patch, 37: select-style-2207391-34.patch, failed testing.

lewisnyman’s picture

Title: Style select elements » Style select elements in Webkit only
Status: Needs work » Needs review
StatusFileSize
new2.56 KB

This is an intense CSS review :P I like it.

I've fixed the newline and padding issues and removed 7 CSS properties that were for fallbacks or just not necessary.

I didn't realize that this only is targeting WebKit. http://stackoverflow.com/questions/9620404/styling-a-select-element-in-f... and http://stackoverflow.com/a/21163651 suggest that this is possible in Firefox as well as IE, but perhaps it's too fragile? Please either make it work in all browsers, or update the issue title to reflect that this only targets Webkit.

If you try the JS fiddle in the latest Firefox it no longer works so :( The bug to add proper support for this is https://bugzilla.mozilla.org/show_bug.cgi?id=649849. We can add support once this is fixed.

If we only target Webkit for such relatively prominent styling: do we have a precedent for that?

Considering all browsers style these differently anyway, I don't think this is introducing a problem. I'd love to enable support for more browsers as it becomes realistic though.

lewisnyman’s picture

StatusFileSize
new2.13 KB
wim leers’s picture

Status: Needs review » Reviewed & tested by the community

Hehe, glad you like it :)

I have no further complaints. Thanks for spending the extra time to not affect front-end performance as much :) The difference with #21 is immense.

Manually tested and still works great.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
  1. +++ b/core/themes/seven/css/style.css
    @@ -831,8 +831,7 @@ input.form-color,
    -select.form-select {
    
    @@ -862,7 +861,7 @@ select.form-select {
    -.form-select:focus {
    

    Why are we removing this? Won't this affect all browsers?

  2. +++ b/core/themes/seven/css/style.css
    @@ -862,7 +861,7 @@ select.form-select {
     .form-textarea:focus,
    ...
    +.form-textarea:focus {
    

    Duplicated.

lewisnyman’s picture

Status: Needs work » Needs review
StatusFileSize
new463 bytes
new2.15 KB

Thanks Alex, eagle eyed! I've removed that duplicated line, see interdiff.

Why are we removing this? Won't this affect all browsers?

Yes, this styling was added during #1986418: Update textfield & textarea style for consistencies sake but it doesn't feel very appropriate (compare other clickable elements like buttons). I'm happy to leave them styled by default in all browsers apart from Webkit for now and maybe open up another issue to see if we can sneak in little tweaks to other browsers here and there.

wim leers’s picture

Status: Needs review » Reviewed & tested by the community
alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 9e6dd21 and pushed to 8.x. Thanks!

diff --git a/core/misc/icons/333333/caret-down.svg b/core/misc/icons/333333/caret-down.svg
old mode 100755
new mode 100644

Fixed the file mode of caret-down.svg during commit.

  • alexpott committed 9e6dd21 on 8.0.x
    Issue #2207391 by LewisNyman, emma.maria, lokapujya: Style select...
nod_’s picture

No love for IE? it seems doable: http://sampsonblog.com/615/ie10s-pseudo-elements

lewisnyman’s picture

I think it is, looks like we'd need to add some javascript so we can add conditional styling for IE10, so it doesn't affect Firefox. I'll create a follow up issue so we can slowly investigate cross browser styling with careful testing.

Status: Fixed » Closed (fixed)

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