Bug 92834 - [CSS] Add selectors for multiple fields time input UI.
Summary: [CSS] Add selectors for multiple fields time input UI.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: yosin
URL:
Keywords:
Depends on:
Blocks: 88970
  Show dependency treegraph
 
Reported: 2012-07-31 21:01 PDT by yosin
Modified: 2012-08-01 02:17 PDT (History)
5 users (show)

See Also:


Attachments
Patch 1 (3.54 KB, patch)
2012-07-31 21:17 PDT, yosin
no flags Details | Formatted Diff | Diff
Screenshot of multiple fields time input UI (62.51 KB, image/png)
2012-08-01 01:00 PDT, yosin
no flags Details
Patch 2 (4.19 KB, patch)
2012-08-01 01:12 PDT, yosin
no flags Details | Formatted Diff | Diff
Patch 3 (4.19 KB, patch)
2012-08-01 01:56 PDT, yosin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description yosin 2012-07-31 21:01:58 PDT
In the implementation of multiple fields time input UI uses CSS selectors for easy customization of looking of UI.

This bug is one of preparations of multiple fields time input UI, bug 88970.
Comment 1 yosin 2012-07-31 21:17:11 PDT
Created attachment 155716 [details]
Patch 1
Comment 2 yosin 2012-07-31 21:18:25 PDT
Comment on attachment 155716 [details]
Patch 1

Could you review this patch?
Thanks in advance.
Comment 3 Kent Tamura 2012-07-31 21:59:42 PDT
Comment on attachment 155716 [details]
Patch 1

View in context: https://bugs.webkit.org/attachment.cgi?id=155716&action=review

> Source/WebCore/ChangeLog:21
> +        (input::-webkit-datetime-edit-minute-field[disabled]): Added.
> +        (input::-webkit-datetime-edit-second-field[disabled]): Added.

Would you explain when they are used please? They aren't same as input[type=time]:disabled? Why they contain only minute and second?

> Source/WebCore/css/html.css:479
> +}

Do we need font-family:monospace?

> Source/WebCore/css/html.css:487
> +    color: GrayText;
> +}
> +
> +input::-webkit-datetime-edit-second-field[disabled] {
> +    color: GrayText;
> +}

Need a special care for Windows. like input[]:disabled in themeWin.css.

> Source/WebCore/css/html.css:538
> +    height: 1.5em;

Need a comment here in addition to ChangeLog.  It is confusing.
Comment 4 yosin 2012-08-01 01:00:21 PDT
Created attachment 155738 [details]
Screenshot of multiple fields time input UI
Comment 5 yosin 2012-08-01 01:02:30 PDT
(In reply to comment #3)
> (From update of attachment 155716 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=155716&action=review
> 
> > Source/WebCore/ChangeLog:21
> > +        (input::-webkit-datetime-edit-minute-field[disabled]): Added.
> > +        (input::-webkit-datetime-edit-second-field[disabled]): Added.
> 

These selectors used for 
 - minute[disabled]: step>=3600 but format contains minute field, e.g. h:m
 - second[disabled]: step>=60 but format contains second field, e.g. h:m:s

I attached a screenshot for sample of multiple fields time input UI.
Comment 6 yosin 2012-08-01 01:12:23 PDT
Created attachment 155743 [details]
Patch 2
Comment 7 yosin 2012-08-01 01:53:04 PDT
(In reply to comment #3)
> (From update of attachment 155716 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=155716&action=review
> > Source/WebCore/css/html.css:487
> > +    color: GrayText;
> > +}
> > +
> > +input::-webkit-datetime-edit-second-field[disabled] {
> > +    color: GrayText;
> > +}
> 

The intention of them is indicating users don't need to enter digits and can't be changed rather than disabling the field.

I'll use "readonly" instead of "disable" to avoid confusion.
Comment 8 yosin 2012-08-01 01:56:09 PDT
Created attachment 155756 [details]
Patch 3
Comment 9 yosin 2012-08-01 01:58:40 PDT
Comment on attachment 155756 [details]
Patch 3

Could you review this patch?
Thanks in advance.

= Changes since the last patch =
* Add font-family: monospace
* Use "readonly" attribute instead of "disabled" attribute for readonly fields.
* Add a comment about "height" property of spin button.
Comment 10 Kent Tamura 2012-08-01 02:03:18 PDT
Comment on attachment 155756 [details]
Patch 3

Looks ok
Comment 11 yosin 2012-08-01 02:17:51 PDT
Comment on attachment 155756 [details]
Patch 3

Clearing flags on attachment: 155756

Committed r124314: <http://trac.webkit.org/changeset/124314>
Comment 12 yosin 2012-08-01 02:17:57 PDT
All reviewed patches have been landed.  Closing bug.