1320647 - [DateTimePicker] Add min/max support to date picker

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect, P3)

Comment 1

[Scott Wu [:scottwu]]

Attachment: Bug 1320647 - Implement min and max for date picker.

Review commit: https://reviewboard.mozilla.org/r/96180/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/96180/

Comment 2

[Scott Wu [:scottwu]]

WIP update
- Improved styling for month-year button
- Added new svg for arrows
- Added min/max support
- Working on step support

Comment 3

[Scott Wu [:scottwu]]

Comment on attachment 8815153 [details]
Bug 1320647 - Implement min and max for date picker.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/96180/diff/1-2/

Comment 4

[Scott Wu [:scottwu]]

WIP update
- Rebased to central
- Implemented step for days on calendar
- Will work on step support for months and years

Comment 5

[Scott Wu [:scottwu]]

Attachment: Bug 1320647 - Part 2: Add disabled styles and remove over-specific css rules

Review commit: https://reviewboard.mozilla.org/r/101078/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/101078/

Comment 6

[Scott Wu [:scottwu]]

Attachment: Bug 1320647 - Part 3: Implement steps for days

Review commit: https://reviewboard.mozilla.org/r/101080/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/101080/

Comment 7

[Scott Wu [:scottwu]]

Attachment: Bug 1320647 - Part 4: Implement steps for months and years

Review commit: https://reviewboard.mozilla.org/r/101082/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/101082/

Comment 8

[Scott Wu [:scottwu]]

Comment on attachment 8815153 [details]
Bug 1320647 - Implement min and max for date picker.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/96180/diff/2-3/

Comment 9

[Scott Wu [:scottwu]]

Due to changes in spec, I'm breaking this bug up to smaller ones. This one will be for min/max specifically.

Comment 10

[Scott Wu [:scottwu]]

Comment on attachment 8815153 [details]
Bug 1320647 - Implement min and max for date picker.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/96180/diff/3-4/

Comment 11

[Scott Wu [:scottwu]]

This patch implements support for the min and max attributes on <input type="date"/>. Dates outside of the min and max range are greyed-out and do not respond to clicks or hover.

Also, the DateKeeper now uses a date object (instead of 3 separate attributes: year, month, day) to track of the date it is displaying.

Mike, I wonder if you have time to review this bug? (and a few more to come)
Thanks in advance!

Comment 12

[Mike Conley (:mconley) (:⚙️)]

(In reply to Scott Wu [:scottwu] from comment #11)
> Mike, I wonder if you have time to review this bug? (and a few more to come)
> Thanks in advance!

I think I'll be able to review this in the short term, yes. In future, btw, no need to needinfo? as well as r? me - the r? is sufficient on its own. :)

Comment 13

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8815153 [details]
Bug 1320647 - Implement min and max for date picker.

https://reviewboard.mozilla.org/r/96180/#review140254

Sorry for the delay - this looks good to me. Thanks!

::: toolkit/content/widgets/datekeeper.js:19
(Diff revision 4)
> +        MIN_DATE = -8640000000000000,
> +        MAX_DATE = 8640000000000000;

I suspect you're deriving this from http://ecma-international.org/ecma-262/5.1/#sec-15.9.1.1, right? If so, can you please mention that?

::: toolkit/content/widgets/datekeeper.js:75
(Diff revision 4)
>       *          {Number} month [optional]
>       *          {Number} date [optional]
>       *        }
>       */
> -    set({ year = this.state.year, month = this.state.month, day = this.state.day }) {
> -      this._update(year, month, day);
> +    set({ year = this.year, month = this.month, day = this.day }) {
> +      // Use setUTCFullYear so that year 99 doesn't get parsed as 1999

Maybe we should mention this in the function header, that we're not taking the short-form for years?

Comment 14

[Scott Wu [:scottwu]]

Comment on attachment 8815153 [details]
Bug 1320647 - Implement min and max for date picker.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/96180/diff/4-5/

Comment 15

[Scott Wu [:scottwu]]

Comment on attachment 8815153 [details]
Bug 1320647 - Implement min and max for date picker.

https://reviewboard.mozilla.org/r/96180/#review140254

No worries. Thanks a lot Mike!

> I suspect you're deriving this from http://ecma-international.org/ecma-262/5.1/#sec-15.9.1.1, right? If so, can you please mention that?

Yeah I'll add a comment for that.

> Maybe we should mention this in the function header, that we're not taking the short-form for years?

Ok!

Comment 16

[Scott Wu [:scottwu]]

Comment on attachment 8815153 [details]
Bug 1320647 - Implement min and max for date picker.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/96180/diff/5-6/

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ece97e873769
Implement min and max for date picker. r=mconley

Comment 18

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/mozilla-central/rev/ece97e873769