[Datepicker] Redesign datepicker as per material spec #3739

tintin1343 · 2016-03-18T20:09:24Z

PR has tests / docs demo, and is linted.
Commit and PR titles begin with [ComponentName], and are in imperative form: "[Component] Fix leaky abstraction".
Description explains the issue / use-case resolved, and auto-closes the related issue(s) (http://tr.im/vFqem).

This PR deals with the following:

Closes #3603.
Closes #3740.
Closes mui/mui-x#7515.
Closes #1489.
Closes #3046.

mbrookes · 2016-03-18T23:39:54Z

Hi Nitin, nice work! The landscape picker looks much better already!

Just a few style tweaks as promised in gitter 😄 :

(1) The date selector overlaps the current selection:

In general the vertical spacing looks a bit tight, so increasing that slightly will probably fix this overlap issue too.

(2) The days should be vertically aligned with the dates:

This left margin may be the issue:

(3) There needs to be some margin around the calendar, at least on the sides and bottom, and 8px between the buttons:

(4) If we're aiming to follow the spec, it looks like the buttons have a smaller min-width. (Note how the OK button is narrower, even with the extra margin in the MD spec):

That may be a fault with our button component, but for now you could override it for the calendar.

(5) In 0.15.0-alpha.2, the calendar used to have an animated transition when changing month, similar to:

https://material-design.storage.googleapis.com/publish/material_v_4/material_ext_publish/0B6Okdz75tqQsSm5KN1FNQXRFRDA/components_pickers_date_navigation_xhdpi_004.webm

Also in that video, you'll notice the calendar maintains it's height when the number of weeks changes. Currently ours jumps p and down in height. This also affects the year selector.

(6) The hover state for the year selector in the redesigned picker should be different to the selected year:

Google don't give any hints here, but it should probably be an intermediate state, not the same as the selected year - smaller size than selected, and the lighter green color. IT should probably transition between states.

(7) The font size for the unselected years is too small.

Redesigned picker:

Spec:

There's a min-height missing somewhere in the portrait picker:

Expect more to follow, but that should keep you busy for a little while 😄

mbrookes · 2016-03-21T10:07:05Z

Need to check this for #1489.

tintin1343 · 2016-03-21T14:03:51Z

@mbrookes : Alright. Will do.

tintin1343 · 2016-03-21T16:33:54Z

@mbrookes : Suggestions have been implemented. Looking forward to your suggestions. As far as #1489 is concerned there is an issue with months with 6 rows in them (in Safari) let me see what I can do about it.

tintin1343 · 2016-03-21T16:37:13Z

The date selector overlaps the current selection:
The days should be vertically aligned with the dates:
There needs to be some margin around the calendar, at least on the sides and bottom, and 8px between the buttons:
If we're aiming to follow the spec, it looks like the buttons have a smaller min-width. (Note how the OK button is narrower, even with the extra margin in the MD spec):
In 0.15.0-alpha.2, the calendar used to have an animated transition when changing month.
The hover state for the year selector in the redesigned picker should be different to the selected
The font size for the unselected years is too small.
There's a min-height missing somewhere in the portrait picker
The year list be laid out with fixed spacing so it doesn't reposition later years on font size change of earlier ones

@mbrookes: Just added these for you to uncheck if you feel it needs more work. 😀

tintin1343 · 2016-03-21T20:40:04Z

@mbrookes : This should resolve #1489

mbrookes · 2016-03-23T00:47:16Z

@tintin1343 - getting there! 👍

I've unchecked the ones that don't appear to have been addressed yet.

Also:

The hover state for the year selector in the redesigned picker should be different to the selected

It's better, I don't think the currently selected year should shrink when hovered, also could the list be laid out with fixed spacing so it doesn't reposition later years on font size change of earlier ones?

tintin1343 · 2016-03-24T19:54:07Z

mbrookes · 2016-03-24T21:14:34Z

Hi Nitin, getting there. A few more tweaks as mentioned:

The days should be vertically aligned with the dates:

(Also, they appear to be using floats rather than flex, could you fix that?)
Button padding should follow MD guidlines:

(8px above, below and to the right). Same for the Year selector.

Also from the spec above, 24px from the bottom of the last row of numbers to the top of the buttons (it's too large at the moment).
Dialog button minWidth should be 64px (including text padding):

Year selector font sizes for both selected and unselected are still too small, and the vertical spacing too large:

Dialog changes size between month and year selectors:

(might be related to the lack of padding below the buttons.)

The years in the year selector are off center again (see image above).
This is still happening:

(But maybe you just hadn't pushed that change.)

All that said, I'm assuming the internal structure is reasonable, but I'll let one of the others comment.

tintin1343 · 2016-03-24T21:17:05Z

Thanks @mbrookes! Those are some amazing observations.

I will work on em right away.

tintin1343 · 2016-03-28T21:25:19Z

@mbrookes: Updated the changes:

The days should be vertically aligned with the dates:
Button padding should follow MD guidlines:
Also from the spec above, 24px from the bottom of the last row of numbers to the top of the buttons (it's too large at the moment).
Dialog button minWidth should be 64px (including text padding):
Year selector font sizes for both selected and unselected are still too small, and the vertical spacing too large:
Dialog changes size between month and year selectors:
The years in the year selector are off center again (see image above).
Height Issue.

I am not able to reproduce the last one. Can you help me with it?

mbrookes · 2016-03-28T22:16:14Z

Last few issues outstanding.

Month animation direction is reversed.

Still no button margin:

And the buttons look too tall. Check the spec again (layout and visual).

Also from the spec above, 24px from the bottom of the last row of numbers to the top of the button margin (it's too large at the moment).

Still appears to be about 78px:

Only if you wanted to try, it would be pretty cool if when you click in a year, it animates to adopt the selected year position, and size.

nathanmarks · 2016-05-03T23:03:29Z

~~Letters are cut off here due to the overflow being hidden:~~

nathanmarks · 2016-05-03T23:07:25Z

~~These buttons need to be minWidth: 64~~

tintin1343 · 2016-05-03T23:17:31Z

Letters are cut off here due to the overflow being hidden:

@nathanmarks : which browser? Looks fine in chrome, Safari and firefox

These buttons need to be minWidth: 64

They do have.

  },
      flatButtons: {
        fontsize: 14,
        margin: '4px 8px 8px 0px',
        maxHeight: 36,
-->    minWidth: 64,
        padding: 0,
      },

The CalendarActionButtons.js file has this.

nathanmarks · 2016-05-03T23:40:17Z

@tintin1343 I was on the wrong PR 😄

tintin1343 · 2016-05-03T23:43:01Z

@nathanmarks U scared me man. :p

nathanmarks · 2016-05-03T23:57:25Z

@tintin1343 hahaah

I think we should force the month onto the 2nd line here, under all circumstances:

usually it is like

tintin1343 · 2016-05-04T00:05:11Z

@nathanmarks : I agree. Let me fix that.

nathanmarks · 2016-05-04T02:20:20Z

src/DatePicker/DateDisplay.js

@@ -128,12 +129,14 @@ class DateDisplay extends Component {
    const {prepareStyles} = this.context.muiTheme;
    const styles = getStyles(this.props, this.context, this.state);
    const year = selectedDate.getFullYear();
+    const regex = /[.,]/;
+    const delimiter = this.props.locale === 'fr' ? '.' : ',';


hey dude, this technique isn't going to be scalable... I think we may just have to leave it as it was 😞 there's no way we can account for all the possible locales that a user might pass if they are providing the formatter.

I know and I agree. This is what I was thinking when I was considering locales. However I don't think this would do any harm as most of the locales will use either a comma or a dot. Only crazy thing would be if there is a hypen or slash.

I can do one thing and it's a crude way/ hack. Leave it as it was append after weekday so that it pushes the day and month to the bottom. What say?

I meant append....&nbsp..

mbrookes · 2016-05-04T21:36:09Z

src/DatePicker/Calendar.js

@@ -47,7 +46,7 @@ class Calendar extends React.Component {
    open: PropTypes.bool,
    shouldDisableDate: PropTypes.func,
    showActionButtons: PropTypes.bool,
-    wordings: PropTypes.object,
+    wordings: deprecated(PropTypes.object, 'Instead, use `cancelLabel` and `okLabel`.'),


You should only need the warning in DatePicker, otherwise the warning will fire multiple times as the value propagates down the component tree.

nathanmarks · 2016-05-04T22:24:52Z

src/DatePicker/Calendar.js

@@ -46,7 +45,7 @@ class Calendar extends Component {
    open: PropTypes.bool,
    shouldDisableDate: PropTypes.func,
    showActionButtons: PropTypes.bool,
-    wordings: deprecated(PropTypes.object, 'Instead, use `cancelLabel` and `okLabel`.'),
+    wordings: PropTypes.object,


Why are we getting rid of a deprecated function without removing the prop?

@mbrookes asked me to just keep it in the Datepicker and remove em from the child components.

@tintin1343 ahhhh ok!

tintin1343 · 2016-05-05T02:03:29Z

@mbrookes : Fixed the issues that we discussed.

tintin1343 force-pushed the datepicker-redesign branch 2 times, most recently from 045d5bb to f437272 Compare March 18, 2016 20:59

mbrookes added PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI design: material This is about Material Design, please involve a visual or UX designer in the process labels Mar 18, 2016

tintin1343 force-pushed the datepicker-redesign branch from 16c316c to e3ba816 Compare March 21, 2016 16:32

tintin1343 force-pushed the datepicker-redesign branch from e3ba816 to 9e7a71e Compare March 21, 2016 20:39

tintin1343 force-pushed the datepicker-redesign branch from 9e7a71e to d3dbbec Compare March 21, 2016 20:45

izziaraffaele mentioned this pull request Mar 24, 2016

[DatePicker] Add possibility to customize particular days #3794

Closed

tintin1343 force-pushed the datepicker-redesign branch from a8e176b to 9d0d307 Compare March 24, 2016 19:50

tintin1343 force-pushed the datepicker-redesign branch from bd1c392 to b026992 Compare March 28, 2016 21:21

tintin1343 force-pushed the datepicker-redesign branch from b026992 to bd1c392 Compare March 28, 2016 21:41

This was referenced Apr 6, 2016

[DatePicker] Calculated wrong top value on selected date DIV tag #3886

Closed

[DatePicker] DatePicker not working on mobile #3891

Closed

oliviertassinari mentioned this pull request Apr 8, 2016

[valueLink] React is deprecating the usage #2880

Closed

9 tasks

tintin1343 force-pushed the datepicker-redesign branch from 83320fa to 0ab730f Compare April 13, 2016 19:46

This was referenced Apr 14, 2016

[DatePicker] support on Safari, IE, Edge #2146

Closed

datepicker: days of month do not fit in boundaries. #2462

Closed

There is no option for changing years in Datepicker dialog #1555

Closed

nathanmarks removed the PR: Needs Review label May 3, 2016

nathanmarks reviewed May 4, 2016
View reviewed changes

tintin1343 force-pushed the datepicker-redesign branch 5 times, most recently from ebc661d to 8211ddf Compare May 4, 2016 14:13

[DatePicker] Remove prop from CalendarActionButtons

3b13929

tintin1343 force-pushed the datepicker-redesign branch from 8211ddf to 3b13929 Compare May 4, 2016 14:14

mbrookes reviewed May 4, 2016
View reviewed changes

[DatePicker] Remove deprecaed warning from inner components

4bc7aa2

nathanmarks reviewed May 4, 2016
View reviewed changes

[DatePicker] Fix autoOk functionality & onTouchTapDay nomenclature

7797867

tintin1343 force-pushed the datepicker-redesign branch from dcb9ded to 7797867 Compare May 4, 2016 23:10

tintin1343 added 2 commits May 4, 2016 19:09

[DatePicker] Remove showActionButtons

b6903fe

[DatePicker] Add width to fix Calendar Component

e44d5e1

mbrookes added PR: Review Accepted and removed PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI labels May 5, 2016

nathanmarks merged commit 4378b84 into mui:master May 6, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Datepicker] Redesign datepicker as per material spec #3739

[Datepicker] Redesign datepicker as per material spec #3739

tintin1343 commented Mar 18, 2016 •

edited

Loading

mbrookes commented Mar 18, 2016

mbrookes commented Mar 21, 2016

tintin1343 commented Mar 21, 2016

tintin1343 commented Mar 21, 2016

tintin1343 commented Mar 21, 2016 •

edited

Loading

tintin1343 commented Mar 21, 2016

mbrookes commented Mar 23, 2016

tintin1343 commented Mar 24, 2016

mbrookes commented Mar 24, 2016

tintin1343 commented Mar 24, 2016

tintin1343 commented Mar 28, 2016

mbrookes commented Mar 28, 2016

nathanmarks commented May 3, 2016 •

edited

Loading

nathanmarks commented May 3, 2016 •

edited

Loading

tintin1343 commented May 3, 2016 •

edited

Loading

nathanmarks commented May 3, 2016

tintin1343 commented May 3, 2016

nathanmarks commented May 3, 2016 •

edited

Loading

tintin1343 commented May 4, 2016

nathanmarks May 4, 2016

tintin1343 May 4, 2016

tintin1343 May 4, 2016 •

edited

Loading

mbrookes May 4, 2016

tintin1343 May 4, 2016

nathanmarks May 4, 2016

tintin1343 May 4, 2016

nathanmarks May 4, 2016

tintin1343 commented May 5, 2016

[Datepicker] Redesign datepicker as per material spec #3739

[Datepicker] Redesign datepicker as per material spec #3739

Conversation

tintin1343 commented Mar 18, 2016 • edited Loading

mbrookes commented Mar 18, 2016

mbrookes commented Mar 21, 2016

tintin1343 commented Mar 21, 2016

tintin1343 commented Mar 21, 2016

tintin1343 commented Mar 21, 2016 • edited Loading

tintin1343 commented Mar 21, 2016

mbrookes commented Mar 23, 2016

tintin1343 commented Mar 24, 2016

mbrookes commented Mar 24, 2016

tintin1343 commented Mar 24, 2016

tintin1343 commented Mar 28, 2016

mbrookes commented Mar 28, 2016

nathanmarks commented May 3, 2016 • edited Loading

nathanmarks commented May 3, 2016 • edited Loading

tintin1343 commented May 3, 2016 • edited Loading

nathanmarks commented May 3, 2016

tintin1343 commented May 3, 2016

nathanmarks commented May 3, 2016 • edited Loading

tintin1343 commented May 4, 2016

nathanmarks May 4, 2016

Choose a reason for hiding this comment

tintin1343 May 4, 2016

Choose a reason for hiding this comment

tintin1343 May 4, 2016 • edited Loading

Choose a reason for hiding this comment

mbrookes May 4, 2016

Choose a reason for hiding this comment

tintin1343 May 4, 2016

Choose a reason for hiding this comment

nathanmarks May 4, 2016

Choose a reason for hiding this comment

tintin1343 May 4, 2016

Choose a reason for hiding this comment

nathanmarks May 4, 2016

Choose a reason for hiding this comment

tintin1343 commented May 5, 2016

tintin1343 commented Mar 18, 2016 •

edited

Loading

tintin1343 commented Mar 21, 2016 •

edited

Loading

nathanmarks commented May 3, 2016 •

edited

Loading

nathanmarks commented May 3, 2016 •

edited

Loading

tintin1343 commented May 3, 2016 •

edited

Loading

nathanmarks commented May 3, 2016 •

edited

Loading

tintin1343 May 4, 2016 •

edited

Loading