Closed
Bug 1091592
Opened 10 years ago
Closed 8 years ago
Implement improved style for <select> dropdowns
Categories
(Core :: Layout: Form Controls, defect, P3)
Tracking
()
RESOLVED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox52 | --- | fixed |
People
(Reporter: phlsa, Assigned: markgolbeck08, Mentored)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
(Whiteboard: [qx:spec])
Attachments
(3 files, 2 obsolete files)
Our current <select> dropdowns look pretty dated. With a few styling changes we can improve the legibility quite a bit.
Spec: http://phlsa.github.io/ff-unstyled-form-widgets
Flags: firefox-backlog+
Reporter | ||
Updated•10 years ago
|
Whiteboard: [qx]
Updated•9 years ago
|
Whiteboard: [qx] → [qx:spec]
Updated•9 years ago
|
Flags: needinfo?(gijskruitbosch+bugs)
Comment 1•9 years ago
|
||
(In reply to Philipp Sackl [:phlsa] (Firefox UX) please use needinfo from comment #0)
> Our current <select> dropdowns look pretty dated. With a few styling changes
> we can improve the legibility quite a bit.
>
> Spec: http://phlsa.github.io/ff-unstyled-form-widgets
This still changes the height and font-sizing of the options, which is likely to cause website compat problems. Can we avoid doing that and/or is it worth doing something here if those need to remain fixed?
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(philipp)
Reporter | ||
Comment 2•9 years ago
|
||
Could you elaborate how that can cause compatibility problems?
The popup is entirely self contained and can't change anything about the layout of the page around it, as far as I can tell.
Fonts and sizing within that popup will look different, but that's the point of the change.
Flags: needinfo?(philipp) → needinfo?(gijskruitbosch+bugs)
Comment 3•9 years ago
|
||
(In reply to Philipp Sackl [:phlsa] (Firefox UX) please use needinfo from comment #2)
> Could you elaborate how that can cause compatibility problems?
> The popup is entirely self contained and can't change anything about the
> layout of the page around it, as far as I can tell.
If you change the font-size (or font-family, for that matter) of an <option> that affects the size of the collapsed select box that contains the <option>. Likewise for height.
> Fonts and sizing within that popup will look different, but that's the point
> of the change.
This too can (likely will) cause more minor issues in terms of how much overlap there is/isn't with other parts of the page.
Flags: needinfo?(gijskruitbosch+bugs)
Updated•9 years ago
|
Flags: needinfo?(philipp)
Comment 4•9 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #3)
> If you change the font-size (or font-family, for that matter) of an <option>
> that affects the size of the collapsed select box that contains the
> <option>. Likewise for height.
On at least some platforms, the borders of default-styled collapsed select boxes depend on the toolkit theme, but this is only if the content does not style the select boxes. Similarly with font-size, I expect.
Reporter | ||
Comment 5•9 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #3)
> (In reply to Philipp Sackl [:phlsa] (Firefox UX) please use needinfo from
> comment #2)
> > Could you elaborate how that can cause compatibility problems?
> > The popup is entirely self contained and can't change anything about the
> > layout of the page around it, as far as I can tell.
>
> If you change the font-size (or font-family, for that matter) of an <option>
> that affects the size of the collapsed select box that contains the
> <option>. Likewise for height.
I just checked the spec again: It uses 11px as the font size, which is the same as the current font size in collapsed select elements. The padding of the entries in the opened box shouldn't affect the size of the collapsed element.
Also, the main point of this bug is to improve the appearance of the opened select element. So even if the collapsed element doesn't change at all, there's a lot to improve.
> > Fonts and sizing within that popup will look different, but that's the point
> > of the change.
>
> This too can (likely will) cause more minor issues in terms of how much
> overlap there is/isn't with other parts of the page.
That is not something that pages should expect to be consistent. The sizes of opened select boxes vary widely between browsers and platforms (for example, Edge uses a very touch friendly style that makes the popup huge).
Flags: needinfo?(philipp)
Comment 6•9 years ago
|
||
(In reply to Philipp Sackl [:phlsa] (Firefox UX) please use needinfo from comment #5)
> (In reply to :Gijs Kruitbosch from comment #3)
> > (In reply to Philipp Sackl [:phlsa] (Firefox UX) please use needinfo from
> > comment #2)
> > > Could you elaborate how that can cause compatibility problems?
> > > The popup is entirely self contained and can't change anything about the
> > > layout of the page around it, as far as I can tell.
> >
> > If you change the font-size (or font-family, for that matter) of an <option>
> > that affects the size of the collapsed select box that contains the
> > <option>. Likewise for height.
>
> I just checked the spec again: It uses 11px as the font size, which is the
> same as the current font size in collapsed select elements. The padding of
> the entries in the opened box shouldn't affect the size of the collapsed
> element.
It's nice to say "shouldn't", but that is not how CSS works right now, as far as I can tell. I don't think it's possible to style the options separately as they appear in the popup, and not affect the <select> element's size (at least, no incantation of "option" or ":-moz-display-comboboxcontrol-frame" or ":-moz-dropdown-list" seemed to work for me when I tried). We could try to counteract that select element's size change by fixing its size, but that seems likely to break e.g. how we deal with OS default font sizes. Also keep in mind that the same HTML elements are also used by non-dropdown selects (ie with size > 1).
Maybe Daniel knows if I'm missing something here in terms of the CSS possibilities.
Flags: needinfo?(dholbert)
Comment 7•9 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #6)
> Maybe Daniel knows if I'm missing something here in terms of the CSS
> possibilities.
I do not. (I don't know much about <select> internals/rendering.) karlt may be of more use to you here than I am, since he's worked more directly on widget implementation stuff.
(While I'm commenting, though -- somewhat offtopic, but for reference RE mentions of web-compat here -- bug 1230065 is one example where we've broken a website by changing the internal sizes/paddings inside of a collapsed <select>.)
Flags: needinfo?(dholbert)
Comment 8•9 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #6)
> It's nice to say "shouldn't", but that is not how CSS works right now, as
> far as I can tell. I don't think it's possible to style the options
> separately as they appear in the popup, and not affect the <select>
> element's size (at least, no incantation of "option" or
> ":-moz-display-comboboxcontrol-frame" or ":-moz-dropdown-list" seemed to
> work for me when I tried). We could try to counteract that select element's
> size change by fixing its size, but that seems likely to break e.g. how we
> deal with OS default font sizes. Also keep in mind that the same HTML
> elements are also used by non-dropdown selects (ie with size > 1).
>
> Maybe Daniel knows if I'm missing something here in terms of the CSS
> possibilities.
Per Daniel's reply, redirecting to :karlt :-)
Flags: needinfo?(karlt)
Comment 9•9 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #6)
> It's nice to say "shouldn't", but that is not how CSS works right now, as
> far as I can tell. I don't think it's possible to style the options
> separately as they appear in the popup, and not affect the <select>
> element's size (at least, no incantation of "option" or
> ":-moz-display-comboboxcontrol-frame" or ":-moz-dropdown-list" seemed to
> work for me when I tried). We could try to counteract that select element's
> size change by fixing its size, but that seems likely to break e.g. how we
> deal with OS default font sizes. Also keep in mind that the same HTML
> elements are also used by non-dropdown selects (ie with size > 1).
I don't know much about how things are implemented, so I'm only guessing, and providing random thoughts.
I would expect most form control sizes to be dependent on the toolkit theme, so while changing default option sizes may have some webcompat issues, I expect the effect will most likely be that some sites look better and some not so good. I'm surprised at the implication that option font sizes don't come from the toolkit theme.
What is probably more important is controls that are styled by content and so not natively themed. Where content currently has control of sizing, it should keep control.
Definitely worth testing default size = 1 and size > 1 separately.
While I'd expect the intrinsic height of the select border to depend on option padding when size > 1, that need not be the case with size = 1. It would be nice to keep consistent font-sizes even in the size = 1 case, but padding could differ.
I don't know whether or not padding differs in the current implementation though.
Perhaps consider changing the option styling only for size = 1 and checking the current implementation if changing the height is a concern.
For security reasons, I would expect styling of options in a popup from content to be somewhat more restricted than styling of options in a size > 1 select.
I think my approach to this would be to use styles from the native toolkit theme when the controls are not styled by content (and maybe that includes the size = 1 popup), and keep things simple when styled by content, so that we don't surprise the content. I don't know how that compares to the current implementation.
Flags: needinfo?(karlt)
Comment 10•9 years ago
|
||
(In reply to Karl Tomlinson (ni?:karlt) from comment #9)
> (In reply to :Gijs Kruitbosch from comment #6)
> > It's nice to say "shouldn't", but that is not how CSS works right now, as
> > far as I can tell. I don't think it's possible to style the options
> > separately as they appear in the popup, and not affect the <select>
> > element's size (at least, no incantation of "option" or
> > ":-moz-display-comboboxcontrol-frame" or ":-moz-dropdown-list" seemed to
> > work for me when I tried). We could try to counteract that select element's
> > size change by fixing its size, but that seems likely to break e.g. how we
> > deal with OS default font sizes. Also keep in mind that the same HTML
> > elements are also used by non-dropdown selects (ie with size > 1).
>
> I don't know much about how things are implemented, so I'm only guessing,
> and providing random thoughts.
It would be useful to hear from someone who does know what our CSS options in forms.css are for making this look as specced, and what the likely consequences would be in terms of web compat. Could you suggest someone else to ask?
> I would expect most form control sizes to be dependent on the toolkit theme,
> so while changing default option sizes may have some webcompat issues, I
> expect the effect will most likely be that some sites look better and some
> not so good. I'm surprised at the implication that option font sizes don't
> come from the toolkit theme.
I didn't mean to suggest we shouldn't use the OS size, but that if we use a particular size for the <option> text in the popup (either hardcoded or based on the default * some multiplier), and we want to ensure that the option text inside the collapsed size=1 select element doesn't change, we will need to modify its font-size specifically, which might or might not break web pages that e.g. only style <option> and expect it to affect the size of the text in the collapsed <select>.
> What is probably more important is controls that are styled by content and
> so not natively themed. Where content currently has control of sizing, it
> should keep control.
Right.
> Definitely worth testing default size = 1 and size > 1 separately.
> While I'd expect the intrinsic height of the select border to depend on
> option padding when size > 1, that need not be the case with size = 1. It
> would be nice to keep consistent font-sizes even in the size = 1 case, but
> padding could differ.
The suggested UI here does not do this, ie the font-size is not consistent between the collapsed and uncollapsed renderings of the <option>(s).
> I think my approach to this would be to use styles from the native toolkit
> theme when the controls are not styled by content (and maybe that includes
> the size = 1 popup),
I'm not clear what you mean here. How do you propose checking for "not styled by content"? AIUI all we can do is provide a default style that content has to override. I don't think it's possible to add extra styling only when no page styling affects the style of the form control.
Flags: needinfo?(karlt)
Comment 11•9 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #10)
> It would be useful to hear from someone who does know what our CSS options
> in forms.css are for making this look as specced, and what the likely
> consequences would be in terms of web compat. Could you suggest someone else
> to ask?
I don't know. Perhaps bz or dbaron, but I would look at the blame of the relevant part of forms.css, after using DOM Inspector to find the relevant part.
> > I think my approach to this would be to use styles from the native toolkit
> > theme when the controls are not styled by content (and maybe that includes
> > the size = 1 popup),
>
> I'm not clear what you mean here. How do you propose checking for "not
> styled by content"? AIUI all we can do is provide a default style that
> content has to override. I don't think it's possible to add extra styling
> only when no page styling affects the style of the form control.
I don't know whether we have an existing pseudoclass to do this with CSS only,
but we do already style widgets differently according to whether or how
content has styled the widget. Compare:
data:text/html,<select><option>Hello<option>Option2
data:text/html,<select style="background:yellow"><option>Hello<option>Option2
Flags: needinfo?(karlt)
Comment 12•9 years ago
|
||
(In reply to Karl Tomlinson (ni?:karlt) from comment #11)
> (In reply to :Gijs Kruitbosch from comment #10)
> > It would be useful to hear from someone who does know what our CSS options
> > in forms.css are for making this look as specced, and what the likely
> > consequences would be in terms of web compat. Could you suggest someone else
> > to ask?
>
> I don't know. Perhaps bz or dbaron, but I would look at the blame of the
> relevant part of forms.css, after using DOM Inspector to find the relevant
> part.
Hm, a lot is hg@1, but at least e.g. bug 610733 and bug 363858 involve both of those people. :-)
> I don't know whether we have an existing pseudoclass to do this with CSS
> only,
> but we do already style widgets differently according to whether or how
> content has styled the widget. Compare:
>
> data:text/html,<select><option>Hello<option>Option2
> data:text/html,<select style="background:yellow"><option>Hello<option>Option2
Interesting! Seems to happen for background: but not for color/font-size... I wonder why that is...
Flags: needinfo?(dbaron)
Comment 13•9 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #12)
> (In reply to Karl Tomlinson (ni?:karlt) from comment #11)
> > I don't know whether we have an existing pseudoclass to do this with CSS
> > only,
> > but we do already style widgets differently according to whether or how
> > content has styled the widget. Compare:
> >
> > data:text/html,<select><option>Hello<option>Option2
> > data:text/html,<select style="background:yellow"><option>Hello<option>Option2
>
> Interesting! Seems to happen for background: but not for color/font-size...
> I wonder why that is...
This is common behavior for a number of form controls: for Web compatibility, we treat background and border styles as equivalent to -moz-appearance:none. See the codepath:
nsIFrame::IsThemed
nsNativeTheme*::ThemeSupportsWidget (platform-specific)
nsNativeTheme::IsWidgetStyled
nsPresContext::HasAuthorSpecifiedRules
nsRuleNode::HasAuthorSpecifiedRules
Comment 14•9 years ago
|
||
So I was hoping to provide some more information here as to the first part of your question, but it's a decent amount of work, and I honestly don't think it's a priority. I think we currently do a half-decent job of matching native styling, which seems good enough to me, and we have too many other things to work on.
I'll say this:
* select popups right now work in totally different ways in e10s and non-e10s, and have substantially different appearance.
* in e10s, the data for the popup is sent to the parent process and displayed using (I think) XUL
* in non-e10s, the options are essentially block frames whose styling is artificially restricted, but I think some of those restrictions could be relaxed if needed -- but doing so might break the way we size the non-popup part of the select -- and in fact changes here might affect the non-popup part of the select in e10s as well
Flags: needinfo?(dbaron)
Comment 15•9 years ago
|
||
(I think it would be far more useful to make progress on making it possible for authors to style form controls in useful and standardized ways. Before Mounir left we had a bit of a plan for this written down, but not much has come of it since.)
Updated•9 years ago
|
Priority: -- → P3
Comment 16•8 years ago
|
||
Updated•8 years ago
|
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Comment 17•8 years ago
|
||
Attachment #8774438 -
Attachment is obsolete: true
Comment 18•8 years ago
|
||
Attachment #8775754 -
Attachment is obsolete: true
Comment 19•8 years ago
|
||
Is this bug strictly only about non-e10s <select>? I thought non-e10s content is going away anyway?
Comment 20•8 years ago
|
||
(I was trying to figure out the relation of this bug with bug 946571, and the state of non-e10s select, see bug 946571 comment 23)
Comment 21•8 years ago
|
||
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #19)
> Is this bug strictly only about non-e10s <select>? I thought non-e10s
> content is going away anyway?
This bug is focused on non-e10s and e10s. The work will implement updated styling for the e10s XUL popup and port the non-e10s implementation to use the e10s XUL popup.
Updated•8 years ago
|
Updated•8 years ago
|
Updated•8 years ago
|
Updated•8 years ago
|
Comment hidden (mozreview-request) |
Comment 23•8 years ago
|
||
mozreview-review |
Comment on attachment 8799576 [details]
Bug 1091592 - Improve style of select dropdowns.
https://reviewboard.mozilla.org/r/84708/#review83522
The comments on top of each change should be removed as they didn't provide any extra information that wasn't already specified in the actual rules.
The padding defined here assumes that the font-size will always be 11px. But the font-size can change if the website has a different font-size set on the select. After you fix up the issues mentioned in this review, we'll want to change the SelectParentHelper.jsm to adjust the padding size if the font-size needed adjustment.
The patch looks good, just some minor fixes which are detailed below. Looking forward to the next version.
::: browser/themes/windows/browser.css:2668
(Diff revision 1)
> +/**
> + * indents each option in the select dropdown group by 20 px from the beginning.
> + */
> +.contentSelectDropdown-ingroup .menu-iconic-text{
> + padding-inline-start: 20px;
> +}
This comment doesn't add much value so we can remove it. It is better to have a comment explain "why" instead of "how" or "what".
This should use the child selector ">" between .contentSelectDropdown-ingroup and .menu-iconic-text.
Also, a couple code-styling remarks to make:
- There should always be a space between the opening curly braket and the last character of the selector.
- The opening curly bracket should be on the same line as the last selector (this one is fine but some others below need to be changed)
- There should only be two spaces of indentation (use spaces, not tabs)
- There should be no whitespace characters at the end of a line
- When in doubt, look at how the code is written in this file and try to be consistent with that
::: browser/themes/windows/browser.css:2696
(Diff revision 1)
> + * sets the padding to 6 px for the left and right
> + * sets the font family to message-box
> + * sets the font size to 11px
> + */
> + #ContentSelectDropdown > menupopup > menuitem{
> + padding: 4px 6px 4px 6px;
The padding property has a short-hand syntax that allows you to specify the top and bottom lengths together and the left and right lengths together.
This syntax would look like:
padding: 4px 6px;
When only specifying two values, the first value is used for the top and the bottom (4px each) and the second value is used for the left and right (6px each).
Please use the two-value syntax here.
::: browser/themes/windows/browser.css:2706
(Diff revision 1)
> +/**
> + * For the select dropdown group menu label
> + * sets the font size to 11px
> + * sets the padding to 0px on the top and bottom
> + */
> + #ContentSelectDropdown > menupopup > menuitem > label{
It is best to have the right-most selector be as specific as possible to improve the speed of the CSS.
Please change this selector to:
#ContentSelectDropdown > menupopup > menuitem > .menu-iconic-text
::: browser/themes/windows/browser.css:2708
(Diff revision 1)
> + padding-top: 0px;
> + padding-bottom: 0px;
Can you please include a comment on top of the padding-top rule that says:
/* Remove the extra vertical padding set by menu.css since the menuitem itself will include enough padding. */
::: browser/themes/windows/browser.css:2713
(Diff revision 1)
> +/*
> + * For non-selected options in the select dropdown menu
> + * Set the color of the text to moz-fieldtext
> + */
> +#ContentSelectDropdown > menupopup > menuitem[_moz-menuactive="false"]{
> + color: -moz-fieldtext;
> +}
I don't think we ever set _moz-menuactive="false", instead the attribute gets removed.
I think you should just remove these lines.
::: browser/themes/windows/browser.css:2740
(Diff revision 1)
> + * sets the padding to 4px on the top and bottom
> + * sets the padding to 6 px for the left and right
> + */
> +#ContentSelectDropdown > menupopup > menucaption{
> + background-color: buttonface;
> + padding: 4px 6px 4px 6px;
You should create a separate rule that specifies the padding for both the menucaptions and the menuitems, this way the value isn't duplicated in the CSS. When values are duplicated one of them might get changed without someone thinking of changing the other.
The new rule should look like:
#ContentSelectDropdown > menupopup > menucaption,
#ContentSelectDropdown > menupopup > menuitem {
padding: 4px 6px;
}
::: browser/themes/windows/browser.css:2754
(Diff revision 1)
> + * Sets the padding to 6 px for the left and right
> + */
> +#ContentSelectDropdown > menupopup > menucaption[disabled="true"]
> +{
> + color: Graytext;
> + padding: 4px 6px 4px 6px;
The padding doesn't need to be set here because it is already defined by the rule above for all menucaptions.
Attachment #8799576 -
Flags: review?(jaws) → review-
Comment hidden (mozreview-request) |
Comment 25•8 years ago
|
||
mozreview-review |
Comment on attachment 8799576 [details]
Bug 1091592 - Improve style of select dropdowns.
https://reviewboard.mozilla.org/r/84708/#review83862
Looking better, should be all good after you fix the issues below.
::: browser/themes/windows/browser.css:2673
(Diff revisions 1 - 2)
> - * For the full popup for select dropdown
> - * sets the background color to -moz-field
> - * sets the outside border of the popup to light gray
> - */
> - #ContentSelectDropdown > menupopup{
> - background-color: -moz-field;
> + background-color: -moz-field;
Please make sure to remove trailing whitespace.
::: browser/themes/windows/browser.css:2693
(Diff revisions 1 - 2)
> }
>
> +#ContentSelectDropdown > menupopup > menuitem > .menu-iconic-text {
> + font-size: 11px;
> -/**
> + /**
> - * For the select dropdown group menu label
> + * Remove the extra vertical padding set by menu.css since
Trailing whitespace here too.
::: browser/themes/windows/browser.css:2711
(Diff revisions 1 - 2)
> - * Sets the padding to 6 px for the left and right
> - */
> -#ContentSelectDropdown > menupopup > menucaption[disabled="true"]
> -{
> color: Graytext;
> - padding: 4px 6px 4px 6px;
> + padding: 4px 6px;
We can remove the padding here since it is already set above for all menucaptions.
::: browser/themes/windows/browser.css
(Diff revisions 1 - 2)
> -/**
> - * For the select dropdown menu options when Windows Touch is enabled
> - * Sets the padding to 11px on the top and bottom
> - */
> -@media (-moz-touch-enabled)
> -{
> - #ContentSelectDropdown > menupopup > menuitem
> - {
> - padding-top: 11px;
> - padding-bottom: 11px;
> - }
> }
Why did you remove the -moz-touch-enabled part? Please add this back in, but place the opening curly brackets on the same line as the selector.
@media (-moz-touch-enabled) {
#ContentSelectDropdown > menupopup > menuitem {
padding-top: 11px;
padding-bottom: 11px;
}
}
Attachment #8799576 -
Flags: review?(jaws) → review-
Comment hidden (mozreview-request) |
Comment 27•8 years ago
|
||
mozreview-review |
Comment on attachment 8799576 [details]
Bug 1091592 - Improve style of select dropdowns.
https://reviewboard.mozilla.org/r/84708/#review84180
Looks good. I've made some minor changes to have apply the increased vertical padding for menucaptions on touch-enabled devices, as well as removing a duplicate rule for font-size. I'll upload a folded patch that we can land.
Attachment #8799576 -
Flags: review?(jaws) → review+
Updated•8 years ago
|
Attachment #8799576 -
Flags: review?(mconley)
Comment 28•8 years ago
|
||
Comment 29•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/83461556cda59e8bcd54f09b9263e3202ce01dc2
Bug 1091592 - Improve style of select dropdowns, including adding more padding for touch-enabled devices. r=jaws
Updated•8 years ago
|
Assignee: jaws → markgolbeck08
Mentor: jaws
Updated•8 years ago
|
Component: Layout: Form Controls → Theme
Product: Core → Firefox
Updated•8 years ago
|
Component: Theme → Layout: Form Controls
Product: Firefox → Core
Comment 30•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Updated•8 years ago
|
Comment hidden (obsolete) |
Reporter | ||
Comment 32•8 years ago
|
||
Hey Jared, is there a build I should review here?
Flags: needinfo?(jaws)
Comment 33•8 years ago
|
||
(In reply to Philipp Sackl [:phlsa] (Firefox UX) please use needinfo from comment #32)
> Hey Jared, is there a build I should review here?
Hey Philipp, this is now in Nightly builds as of 15 October. You can test it at http://webdev.cse.msu.edu/~beachjar/capstone/test.html
Flags: needinfo?(jaws)
Comment 34•8 years ago
|
||
(In reply to Philipp Sackl [:phlsa] (Firefox UX) please use needinfo from comment #32)
> Hey Jared, is there a build I should review here?
To note, the patch that landed here only changed the styling primarily on Windows. On Linux it made disabled optgroups have a gray text, and there were no changes on OSX.
Phlipp, can you please review the changes on Nightly and let us know if they look good to you? Thanks!
Flags: needinfo?(philipp)
Reporter | ||
Comment 35•8 years ago
|
||
Thanks! Looks very good overall!
Two issues I've found (both on Windows):
1. On a touch-enabled device, the dropdowns should only have the additional touch-friendly padding if they've been enabled through a touch event. When activated using the mouse, they should use the normal element size with less padding.
2. The search-enabled select elements didn't show a search field on my build (Nightly 2016-10-17). Should that work already?
Flags: needinfo?(philipp)
Comment 36•8 years ago
|
||
(In reply to Philipp Sackl [:phlsa] (Firefox UX) please use needinfo from comment #35)
> Thanks! Looks very good overall!
>
> Two issues I've found (both on Windows):
> 1. On a touch-enabled device, the dropdowns should only have the additional
> touch-friendly padding if they've been enabled through a touch event. When
> activated using the mouse, they should use the normal element size with less
> padding.
I filed bug 1311450 for this issue.
> 2. The search-enabled select elements didn't show a search field on my build
> (Nightly 2016-10-17). Should that work already?
This is being tracked in bug 1309935.
Comment hidden (mozreview-request) |
Comment 38•8 years ago
|
||
mozreview-review |
Comment on attachment 8799576 [details]
Bug 1091592 - Improve style of select dropdowns.
https://reviewboard.mozilla.org/r/84708/#review93928
This last version includes a bunch of changes that are unrelated. Please create a new patch that removes font:message-box and font-size:11px and attach that to one of the existing bugs that are regarding the font family looking unfamiliar.
Attachment #8799576 -
Flags: review+ → review-
Updated•8 years ago
|
Attachment #8799576 -
Flags: review?(mconley)
You need to log in
before you can comment on or make changes to this bug.
Description
•