To provide a unified and 'app-like' experience we should override default browser styles for select boxes in the offcanvas tray as we have done in seven theme.
Style the selects to match the styling of similar elements in the tray like dropbuttons.
Selects will be grey with white arrow and 2p corner radius in all browsers.
Before
After
None
None
| Comment | File | Size | Author |
|---|---|---|---|
| #49 | patched_chrome.png | 47.38 KB | tedbow |
| #49 | unpatched_chrome.png | 45.83 KB | tedbow |
| #48 | interdiff-2802457-46-48.txt | 3.16 KB | starshaped |
| #48 | 2802457-48.patch | 4.21 KB | starshaped |
| #48 | 2802457-48-safari.png | 91.32 KB | starshaped |
Comments
Comment #2
tkoleary commentedComment #3
tkoleary commentedComment #4
tkoleary commentedThere is no patch yet until the patch at #2781577: Properly style outside-in off canvas tray lands but anyone who wants to can look test this by applying the patch from #2781577, adding this CSS file, then adding this: @import url("/core/modules/outside_in/css/outside_in.select.css"); to the top of outside-in.theme.css.
Doing it that way ftm instead of adding to libraries.yml because it needs to load last and a function has been added to the module to load the outside-in.theme.css last.
Comment #5
tkoleary commentedInitial patch
Comment #7
corbacho commentedTested the patch:
* Bug: The test seems to fail because
it expects a constant, not a number ? (In this case 200)* Bug: IE Edge shows the default browser arrow, resulting in 2 arrows (the default and custom ones). See screenshot
* Note: In Chrome now the item shows focused styles (without the patch, there is no visual styles for focused select form). So the patch improves the a11y of the select form.
I created a new patch, that:
Wrong fix:
* Adds the CSS_AGGREGATE_AFTER_THEME constant in the interface ( I guess it's the right place? #2807785: Move global constants from *.module files into interfaces* Adds a new rule to override the default appareance of the selct form for IE11. "appareance:none" doesn't work. The solution is based on ::-ms-expand See http://stackoverflow.com/questions/20163079/remove-select-arrow-on-ie
Comment #9
corbacho commentedForget about the comments of test failing. The yml file doesn't need to be touched
Comment #10
yoroy commentedI'm seeing this with the latest patch in simplytest:
- The proposed design seems to use a darker gray
- Font size of the values in the select list seems (too) small
Comment #11
tkoleary commentedWe need this for inputs where the default option text is very long so that they dont overflow the edge of the tray. It's the same solution we use for mobile.
I missed that. Nice catch.
Comment #12
corbacho commentedMhh.. I'm confused. Actually I didn't change any CSS in your patch. Just the fact that I re-rolled the patch on latest code might have "adapted" the patch?. Next time I will upload a interdiff to see better
Comment #13
tkoleary commented@yoroy
I have another patch that creates both a dark and a light theme for the tray so I pulled the dark styles out of this patch. This one just sets a generic coler that will be overridden by the dark or light CSS file.
On the font size, not sure what's happening there but there's no explicit styling of options in this patch. Maybe it's inheriting from something else?
Comment #14
tkoleary commented@yoroy @corbacho
Ok, from what I can gather, in Firefox 'option' inherits font size from the select which is not the case in chrome. This appears to solve it.
Comment #15
yoroy commentedI was using Firefox indeed.
Comment #16
tkoleary commentedFirefox problem fixed.
Comment #17
tkoleary commentedFirefox problem fixed. Ignore interdiff, it's incorrect. The only changes are in lines 120-123.
Comment #18
tkoleary commented@corbacho
Thanks. I just realized that there was some stuff in the patch introduced accidentally by me. I removed it. Rerolling.
Comment #19
tkoleary commentedComment #20
corbacho commentedThe "font-size:initial" rule didn't help. After searching around, and finding other people with the same problem, I arrived to the conclusion that is impossible to alter the font-size of the select options in Firefox and Chrome. For example, try to make "font-size:30px", it won't be applied.
IE is a different case. You can specify "font-size:30px" to the select options in IE and it will work. But IE doesn't support the CSS3 "font-size:initial" property, any version of IE.
So.. I think the best we can do, it's to remove the
font-size:initialrule, and let the browser apply his default font-size, without trying to manipulate it. What do you think?Tested in Firefox/Chrome in OSx. IE 11 in Win 8.1 virtual machine
Comment #21
tkoleary commented@corbacho
If we remove the specific font size attribute introduced here and just let the browser set the size the problem Yoroy pointed out *should* go away.
Comment #22
corbacho commentedI think there was no problem at all, Yoroy said the font "seems" small. But actually, I think the screenshot he shows, it's the default Firefox size for select options and you can't change it with CSS. We can't fix it.
If Yoroy, or someone else can confirm my findings, we can move forward, and remove the "font-size" attribute from the patch.
Comment #23
Vidushi Mehta commented@corbacho I've tested applying bigger font size for select options and the font-size is changing in Firefox and Chrome hence working fine for me. Just to make it more clear i've also attached screenshots for the same.
Also, I've tested your #9 patch and it is working fine for me i couldn't find the font size small for select option. For the reference I've attached the screenshots for this as well.
Also, attached screenshots for before applying any patch just to make a check with font-size. Removed the "font-size" attribute and attaching a patch for this as well.
Comment #24
Vidushi Mehta commentedComment #26
tkoleary commented@Vidushi Mehta
This looks good to me. Not sure why it's not passing tests. @corbacho, any ideas?
Comment #27
tedbowThe test failures from #25 are from an unrelated problem. The test are now passing for the issue.
Comment #28
yoroy commentedComment #29
tkoleary commentedlooks like a regression where we are now losing the down arrow icon in chrome (tested mac)
Comment #30
tedbowI see what happened.
#23 did not create the new file core/misc/icons/787878/chevron-down.svg that was in #19
Could easily just been "git add core/modules/outside" which would not have included "core/misc".
Just be clear this new file was intentional in #19, correct? none the existing *-down.svg files worked.
This patch combines #23 with chevron-down.svg from #19.
Comment #32
tedbowRetesting. Weird error
Comment #34
tkoleary commentedI was not able to get the previous patch to apply cleanly in 8.4 which is why no interdiff, but the patch is relatively small in any case.
Comment #35
Vidushi Mehta commentedI applied the patch #34 but getting errors. Please see the screenshot attached.
Comment #36
tedbow@Vidushi Mehta thanks for checking on this issue.
The patch applies for me. It looks like from your error message that you have some of the new files that this patch creates already in your working directory.
Have you applied this patch before? Make sure your git status is clean. If your git status is clean and you are still running into this problem it may mean that you are ignoring untrack files.
Comment #37
tedbowWhoops actually does not need re-roll
Comment #38
Vidushi Mehta commented@tedbow thank you for making me correct. I've applied the patch #34 again and tested on mac for missing down arrow icon and it's working fine. Find the attached screenshots for ff, chrome and safari.
Comment #39
skaughtapplied #34 on simplytest.me.
certainly the images from 38 are accurate -- there is no styling to select at this time.
i appreciate the original style inspiration:
but is a heavy attempt to alter a form element that most browsers (expect IE) don't like being styled with using a JS replacement [ie: http://selectric.js.org]
i think in the least the patch should make the select element {width:100%} to visually level the flow of elements as seen with (editing menus) with the 'initial visibility level' and 'number to level to display' -- treat all selects to fill the line width.
theming selects (radios and checkboxes) is always full of challenges.
Comment #40
skaughtComment #42
tedbowChanging to new settings_tray.module component. @drpal thanks for script help! :)
Comment #43
tedbowNeeds reroll at least for changes in #2803375: Rename Outside-in module to "Settings Tray" for real
Also we should figure out if this is still needed because #2826722: Add a 'fence' around settings tray with aggressive CSS reset. may have fixed this
Comment #44
jofitzRe-rolled.
Comment #46
jofitzCorrect filename from "off_canvas.selects.css" to "off-canvas.selects.css".
Comment #47
tedbow@Jo Fitzgerald thanks for the re-roll!
This is problem with existing patch not the re-roll, there are a bunch of CSS changes that are unrelated to select boxes. These either need to be removed or it is no be clearer what this issue is about.
Comment #48
starshapedRemoved CSS changes unrelated to select boxes. Attached are screenshots from Firefox, Chrome, and Safari showing the select boxes.
Comment #49
tedbow@starshaped thanks for removing the unrelated CSS!
I noticed in the Menu block. The "Initial Visibility level" select in the unpatched version it is always on the next line from the label
while in the patched version it is always on the same line
I tried to resize the tray and it always stayed this way.
Is this intentional?
But from #13 that look like it was taken out of the patch intentionally.
I don't think we are going to be doing the light and dark theme any more.
So we either need to update the IS or if we want add the grey back in.
Comment #51
wim leersDo we still want this?
Comment #52
Vidushi Mehta commented@Wim Leers Can you elaborate your question. Not getting the point.
Comment #64
cosmicdreams commentedToday, it is certainly possible to create your own styled select component. It has taken the web platform a great deal of time and effort to catch up with the spirit of the original request. I wonder, however, if there is actually work to do here.
Doesn't the off-canvas css already provide styles to suit this purpose. I'll investigate.
Comment #65
quietone commentedBased on the lack of activity here and no response to the query by @cosmicdreams, it is time to close this issue. If this is something people want to do the next step would be to open an issue in the contributed Settings Tray project.
The Settings Tray Module was approved for removal in #3576315: [policy, no patch] Move Settings Tray to a contributed project.
The deprecation work is in #3576667: [meta] Tasks to deprecate Settings Tray and the removal work in #3581818: [meta] Tasks to remove Settings Tray module.
Settings Tray will be moved to a contributed project before Drupal 12.0.0 is released.