mui / material-ui-pickers

Date & Time pickers for Material UI (support from v1 to v4)
https://github.com/mui/material-ui-pickers/issues/2157
MIT License
2.32k stars 832 forks source link

[DatePicker] Update to match the specification (desktop) #1648

Closed oliviertassinari closed 3 years ago

oliviertassinari commented 4 years ago

Environment

Tech Version
@material-ui/pickers v4.0.0-alpha.5

Steps to reproduce

https://next.material-ui-pickers.dev/demo/datepicker#basic-usage

Actual behavior

Capture d’écran 2020-04-13 à 00 11 25

Expected behavior

Capture d’écran 2020-04-13 à 00 07 00
  1. The component should have a higher density on desktop. It should be 258x281 instead of 320x358.
  2. The popper should be positioned to start on the right side, not at the center.
  3. The day chip should be 28x28 (maybe less, not sure) instead of 36x36 pixels.
  4. The year chip should be 54x28 instead of 72x36 pixels.
  5. The colors of the range don't match. Try to set the same primary color as in material.io #7001f3 to get an idea of the issue. I propose this diff:
diff --git a/lib/src/DateRangePicker/DateRangePickerDay.tsx b/lib/src/DateRangePicker/DateRangePickerDay.tsx
index 9f54cb31..bff27496 100644
--- a/lib/src/DateRangePicker/DateRangePickerDay.tsx
+++ b/lib/src/DateRangePicker/DateRangePickerDay.tsx
@@ -2,7 +2,7 @@ import * as React from 'react';
 import clsx from 'clsx';
 import { DAY_MARGIN } from '../constants/dimensions';
 import { useUtils } from '../_shared/hooks/useUtils';
-import { makeStyles, fade } from '@material-ui/core/styles';
+import { makeStyles, fade, lighten } from '@material-ui/core/styles';
 import { Day, DayProps, areDayPropsEqual } from '../views/Calendar/Day';

 interface DateRangeDayProps extends DayProps {
@@ -38,8 +38,8 @@ const useStyles = makeStyles(
     },
     rangeIntervalDayHighlight: {
       borderRadius: 0,
-      color: theme.palette.primary.contrastText,
-      backgroundColor: fade(theme.palette.primary.light, 0.6),
+      color: theme.palette.getContrastText(lighten(theme.palette.primary.main, 0.8)),
+      backgroundColor: lighten(theme.palette.primary.main, 0.8),
       '&:first-child': startBorderStyle,
       '&:last-child': endBorderStyle,
     },
@@ -66,7 +66,7 @@ const useStyles = makeStyles(
       },
     },
     dayInsideRangeInterval: {
-      color: theme.palette.getContrastText(fade(theme.palette.primary.light, 0.6)),
+      color: theme.palette.getContrastText(lighten(theme.palette.primary.main, 0.8)),
     },
     notSelectedDate: {
       backgroundColor: 'transparent',

before

Capture d’écran 2020-04-13 à 00 34 11

after

Capture d’écran 2020-04-13 à 00 33 18

baseline

Capture d’écran 2020-04-13 à 00 34 33

Looking again, 0.8 isn't enough. Maybe 0.85 or 0.9.

dmtrKovalenko commented 4 years ago

Related to sizes. I am not sure – do core components precisely following sizes from guidelines? Because IMHO 32x32 is too small for mobile, isn't it?

image

dmtrKovalenko commented 4 years ago

oh goodness, really components have different sizes for desktop. Looks like all of our sizes are for mobile. Thanks for catching it!

oliviertassinari commented 4 years ago

Sorry, I should have mentioned that mobile and desktop should have different dimensions. Thanks for inlining the screeshot from the specification with the actual dimensions.

oliviertassinari commented 4 years ago

Regarding the media query logic. I would be eager to try a logic based on touch vs click support as the primary pointer device (instead of the screen width). It's definitely related to #1653.

oliviertassinari commented 4 years ago

Related to sizes. I am not sure – do core components precisely following sizes from guidelines? Because IMHO 32x32 is too small for mobile, isn't it?

@dmtrKovalenko Regarding your concern about the size.

The core components try to follow the size of the specification, as much as possible, but we have divergences, especially when the specification isn't consistent or dropping 1px simplifies the CSS.

I think that we can benchmark with a couple of frequently used date pickers, to get a sense of where we are on the spectrum (sorted from too small to too large):


Site note, we add vertical spacing on the rows and horizontal spacing on the columns. I think that we should aim for a single place (single element/selector) to handle CSS spacing, to make overrides simpler.

Capture d’écran 2020-04-25 à 13 12 24 Capture d’écran 2020-04-25 à 13 12 31