Closed Bug 1309457 Opened 8 years ago Closed 8 years ago

Style the hover state for <input type=time>

Categories

(Core :: Layout: Form Controls, defect)

defect
Not set
normal

Tracking

()

RESOLVED WONTFIX
Tracking Status
firefox52 --- affected

People

(Reporter: jessica, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

According to this style guide [1], when the input box is hovered, there is a slight gray outline around the input box.

Note that this bug is only for input type=time, for input types in general, it's tracked in Bug 1307384 (may need more discussion).

[1] https://firefoxux.github.io/StyleGuide/#/inputs
Attached patch patch, v1.Splinter Review
Comment on attachment 8800069 [details] [diff] [review]
patch, v1.

Hi Daniel,

Do you think this patch is acceptable? Sorry that I am not that familiar with styling.

While I was testing for other input types (e.g., text), I found that the outline shows for anonymous content too, but it doesn't happen for focus style, I mean, you don't see the blue shadow on anonymous text control inside input type=number when clicking on it. Do you know how it is achieved?

Thanks.
Attachment #8800069 - Flags: review?(dholbert)
Comment on attachment 8800069 [details] [diff] [review]
patch, v1.

Hi Jessica! Sorry for the delay.

(In reply to Jessica Jong [:jessica] from comment #0)
> According to this style guide [1], when the input box is hovered, there is a
> slight gray outline around the input box.
[...]
> [1] https://firefoxux.github.io/StyleGuide/#/inputs

I'm not immediately seeing that gray-outline requirement in the style guide.  Can you clarify?  (Perhaps it's in a datetime-specific section? I don't see anything about datetime controls there, so maybe that's why I'm not seeing this.)

I do see a section on "text input states", but there's nothing about a hover-triggered outline around the control.  Though the mockup does show a subtly-darker *border* on the hovered text-control.

So: marking this r- for now, since I'm not clear enough on where/why this change is required in the style guide.

(In reply to Jessica Jong [:jessica] from comment #2)
> Do you think this patch is acceptable?

That largely depends on the answer to the above. :)

> While I was testing for other input types (e.g., text), I found that the
> outline shows for anonymous content too, but it doesn't happen for focus
> style, I mean, you don't see the blue shadow on anonymous text control
> inside input type=number when clicking on it. Do you know how it is achieved?

I'm not 100% sure what blue shadow you're talking about, or what question you're asking here.

I suspect you're on Mac, so perhaps the blue coloring is coming from your Mac native theme?  (I'm on linux, with GTK theming.)  I think that "native" hover/focus-specific theming would happen via something calling into https://dxr.mozilla.org/mozilla-central/source/widget/cocoa/nsNativeThemeCocoa.mm to update the state of the  widget and/or specifically draw stuff around it.  Perhaps e.g. nsNativeThemeCocoa::DrawFocusOutline, or some other mentions of *_FOCUS  and *_HOVER in that file.  So -- you might be able to discover more by placing breakpoints in that code...
Attachment #8800069 - Flags: review?(dholbert) → review-
(In reply to Daniel Holbert [:dholbert] from comment #3)
> Comment on attachment 8800069 [details] [diff] [review]
> patch, v1.
> 
> Hi Jessica! Sorry for the delay.

No problem, thanks for your feedback.

> I'm not immediately seeing that gray-outline requirement in the style guide.
> Can you clarify?  (Perhaps it's in a datetime-specific section? I don't see
> anything about datetime controls there, so maybe that's why I'm not seeing
> this.)
> 
> I do see a section on "text input states", but there's nothing about a
> hover-triggered outline around the control.  Though the mockup does show a
> subtly-darker *border* on the hovered text-control.
> 
> So: marking this r- for now, since I'm not clear enough on where/why this
> change is required in the style guide.

Sorry for not being clearer. You're right, the style guide only shows for text input, there is no specific date/time section. But as far as I understand, all input controls with a input box will share the same style. Let me ni UX/Visual to confirm this.

Actually, what I wanted to achieve is the subtly-darker *border* on the hovered text-control on the date/time input box. However, setting 'border' does not work and 'outline' is the closest I can get. I can try to dig in more...

> 
> (In reply to Jessica Jong [:jessica] from comment #2)
> > Do you think this patch is acceptable?
> 
> That largely depends on the answer to the above. :)

I see. :)

> 
> I'm not 100% sure what blue shadow you're talking about, or what question
> you're asking here.
> 
> I suspect you're on Mac, so perhaps the blue coloring is coming from your
> Mac native theme?  (I'm on linux, with GTK theming.)  I think that "native"
> hover/focus-specific theming would happen via something calling into
> https://dxr.mozilla.org/mozilla-central/source/widget/cocoa/
> nsNativeThemeCocoa.mm to update the state of the  widget and/or specifically
> draw stuff around it.  Perhaps e.g. nsNativeThemeCocoa::DrawFocusOutline, or
> some other mentions of *_FOCUS  and *_HOVER in that file.  So -- you might
> be able to discover more by placing breakpoints in that code...

Thanks for your guide, will take a look at it.
Flags: needinfo?(hhuang)
As I know, currently we don’t have any component for datetime controls. Therefore, the visual style of input box should follow text input states for consistency.
Flags: needinfo?(hhuang)
(In reply to Jessica Jong [:jessica] from comment #4)
> Actually, what I wanted to achieve is the subtly-darker *border* on the
> hovered text-control on the date/time input box. However, setting 'border'
> does not work

Are you sure it doesn't work? It seems to work if I just set it directly, e.g.:
  https://jsfiddle.net/se5vtxu5/
(I'm using a profile with dom.forms.datetime = true)

Of course, my "border" tweak also disables native styling.  But that's true for all controls -- and I don't think there's any way to adjust the border color *without* disabling native styling.

(Are you sure that style guide is supposed to be a reference for our internal default widget behavior? https://firefoxux.github.io/StyleGuide/#/introduction seems to suggest that it's documenting best-practices for styling your own widgets, when you're writing content that uses widgets. It doesn't seem to be about *default* native widget behavior...)
Note in particular that our default <input> elements (with no extra styling) don't look/behave anything like the <input> elements on that page.

Though: if I copypaste all of the input[type=text] CSS from that page's source into my jsfiddle (with s/type=text/type=time/), it pretty much matches the style guidelines: https://jsfiddle.net/se5vtxu5/1/

(There are a few rendering quirks -- e.g. for me, the "x" button is no longer centered, and the AM/PM selector gets wrapped to a second line.  But the shadow/border/hover behavior is all pretty much what that style guide shows in its mockup.)

Anyway: at this point, I'm pretty suspicious that this style guide is purely suggesting how Firefox-frontend/addon designers should style their <input type="time"> widgets in order to optimally fit in with other Firefox frontend UI -- and it has no bearing on how the default <input type="time"> widget should behave.  This seems to be what its introduction page says, at least: https://firefoxux.github.io/StyleGuide/#/introduction

So, I suspect this bug is INVALID (though we should perhaps spin off a separate bug to prevent the rendering quirks around the "x" and "AM/PM" selector that I mentioned above).
(In reply to Daniel Holbert [:dholbert] (PTO Oct 21-25) from comment #7)
> though we should perhaps spin off a
> separate bug to prevent the rendering quirks around the "x" and "AM/PM"
> selector that I mentioned above

I filed bug 1311857 on these rendering quirks, BTW.
NI=:jessica to confirm whether there's anything we actually need to do here (and close this, if appropriate) per comment 6 / comment 7.
Flags: needinfo?(jjong)
Thank you Daniel for looking into this.

Hmm, setting the border in forms.css is not working for me, maybe it is overriden or something. The jsfiddle (https://jsfiddle.net/se5vtxu5/) does work, but it seems that it is drawn inside the input box, instead of outside of it? However, applying the css from the style guidelines (https://jsfiddle.net/se5vtxu5/1/) works as expected.

About the style guidelines, our UX/Visual team is communicating with the Firefox UX team. I think their plan is to have a unified style for all input controls in the future, and since this is a new feature, we are suggested to follow the style guidelines. But let's wait for their confirmation. I'll keep the NI for tracking.

Thanks for filing bug 1311857, based on our schedule plan, we are starting to work on input type=date picker, and will get back to input type=time bugs soon.
Need info Stephen to get more feedback.

Hi Stephen, here are some questions would like to have your feedback.

Based on the style guide, we can only find text input states for references. https://firefoxux.github.io/StyleGuide/#/inputs
For me, it's reasonable that all the input controls could share the same style. Therefore, the current input box for the datetime pickers is following the style guide for text input. Do you have any concerns about this?

The other question is here we define the focused states of input box will show a blue border. However, as we know the theme color in Ubuntu is orange, in that case, should we keep using blue or just follow the OS theme color? Seems there is no guideline about the controls in different OS, so I really need your advice. Thank you!
Flags: needinfo?(shorlander)
(In reply to Jessica Jong [:jessica] from comment #10)
> I think their plan is to have a unified style for all input
> controls in the future, and since this is a new feature, we are suggested to
> follow the style guidelines.

Two thoughts (& semi-pushback) on this:
(1) If we're matching the unified-style-guidelines (the look and feel) for this new widget, then I'd expect that we should *avoid* shipping this new time-input until we've changed all other widgets to the new guidelines as well.  We don't want pages to end up with a weird mix of widget theming - we've tried hard to make sure that widgets have a consistent look and feel on a given platform.  Is that the plan here?  If we want to ship the time-input-widget sooner than we ship the unified-widget-style, then IMO it's more important to match the *current* widget look-and-feel.

(2) Once we have the new look-and-feel implemented for text widgets, I suspect it might mostly/entirely "just work" for time-inputs, too (since I believe they use a text-input-widget, under the hood).  If we need any additional hacks/changes *specifically* to get the right look for time-inputs, that's fine -- but we should add those as targeted fixes, building on top of the new text-input look-and-feel.

SO: based on each of those thoughts, I think we shouldn't take any specific action here, until we're actively fixing other widgets' default look-and-feel as well.
Thank you Daniel for your feedback.
After considering the points you mentioned in comment 12, we think you are right, if we want to unify the style for all input controls, we should do it all at once (even though we are not shipping the new time-input yet, it's pref-off now). So, let's close this bug and track this in bug 1307384, which is for all input controls.

Thanks again!
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: needinfo?(jjong)
Resolution: --- → WONTFIX
Flags: needinfo?(shorlander)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: