Consolidate classes naming #1950

dmtrKovalenko · 2020-07-03T08:17:54Z

I have followed (at least) the PR section of the contributing guide.

Related to #1778 and #1631.

vercel · 2020-07-03T08:17:57Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/mui-org/material-ui-pickers/agvwk1473
✅ Preview: https://material-ui-pickers-git-feature-classes-naming.mui-org.vercel.app

cypress · 2020-07-03T08:44:14Z

Test summary

78 • 0 • 3 • 0

Run details


Project	material-ui-pickers
Status	Passed
Commit	`850a8da`
Started	Jul 14, 2020 9:34 AM
Ended	Jul 14, 2020 9:35 AM
Duration	01:41 💡
OS	Linux Debian - 10.0
Browser	Chrome 80

View run in Cypress Dashboard ➡️

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

oliviertassinari · 2020-07-03T13:35:02Z

docs/pages/guides/CssOverrides.example.tsx

 import { createMuiTheme, ThemeProvider } from '@material-ui/core';
 import { DatePicker, DatePickerProps } from '@material-ui/pickers';

 const materialTheme = createMuiTheme({
  overrides: {
-    MuiPickersToolbar: {
-      toolbar: {
+    MuiPickerToolbar: {


Could a version without an s feels inconsistent with the name of the package? @material-ui/pickers

I don't understand, it was original intent of the naming, isn't it? (#1778) but we decided to move out?

I don't understand either. What do you think of the last comment on #1778? If you agree with it, the intended name is

Suggested change

MuiPickerToolbar: {

MuiPickersToolbar: {

I still think that correct prefix is with a s to match the package name MuiPickersToolbar.

oliviertassinari · 2020-07-03T13:36:20Z

lib/src/typings/overrides.ts

@@ -2,24 +2,21 @@ import { useStyles as DayStyles } from '../views/Calendar/Day';
 import { useStyles as ClockStyles } from '../views/Clock/Clock';
 import { useStyles as MuiBasePickerStyles } from '../Picker/Picker';
 import { useStyles as CalendarStyles } from '../views/Calendar/Calendar';
-import { useStyles as MuiPickersYearStyles } from '../views/Calendar/Year';
+import { useStyles as MuiPickerYearStyles } from '../views/Calendar/Year';


I think that we will need to rename the components to so they can be imported and grep as they were in a single namespace.

Suggested change

import { useStyles as MuiPickerYearStyles } from '../views/Calendar/Year';

import { useStyles as MuiPickersYearStyles } from '../views/Calendar/PickerYear';

dmtrKovalenko · 2020-07-13T15:31:56Z

@oliviertassinari I am not sure that we need to name all the files with Picker it is the same as in the core repo you are not naming each file with Mui. Current solution works nice and keeps displayName/exportName in sync

oliviertassinari

Nice! Happy to see that coming :D

oliviertassinari · 2020-07-13T15:30:49Z

docs/pages/guides/CssOverrides.example.tsx

 import { createMuiTheme, ThemeProvider } from '@material-ui/core';
 import { DatePicker, DatePickerProps } from '@material-ui/pickers';

 const materialTheme = createMuiTheme({
  overrides: {
-    MuiPickersToolbar: {
-      toolbar: {
+    MuiPickerToolbar: {


I still think that correct prefix is with a s to match the package name MuiPickersToolbar.

oliviertassinari · 2020-07-13T15:30:58Z

docs/pages/regression/RegressionDay.tsx

@@ -1,11 +1,11 @@
 import React from 'react';
 import { IUtils } from '@date-io/core/IUtils';
-import { Day, DayProps } from '@material-ui/pickers';
+import { PickersDay, PickersDayProps } from '@material-ui/pickers';


lib/src/CalendarSkeleton.tsx

lib/src/DateTimePicker/DateTimePickerTabs.tsx

lib/src/Picker/Picker.tsx

oliviertassinari · 2020-07-13T15:37:43Z

lib/src/views/Calendar/Month.tsx

@@ -27,15 +27,15 @@ export const useStyles = makeStyles(
        color: theme.palette.primary.main,
        fontWeight: theme.typography.fontWeightMedium,
      },
+      '&:disabled': {
+        pointerEvents: 'none',
+        color: theme.palette.text.hint,


Thanks, bad merge :)

Seems to be still present

lib/src/views/Calendar/Year.tsx

lib/src/views/Clock/Clock.tsx

oliviertassinari · 2020-07-13T15:38:34Z

lib/src/views/Clock/ClockPointer.tsx

@@ -102,5 +102,5 @@ export const styles = (theme: Theme) =>
  });

 export default withStyles(styles, {
-  name: 'MuiPickersClockPointer',
+  name: 'MuiPickerClockPointer',


Suggested change

name: 'MuiPickerClockPointer',

name: 'MuiPickersClockPointer',

oliviertassinari · 2020-07-13T15:38:54Z

lib/src/views/MobileKeyboardInputView.tsx

+  {
+    padding: '16px 24px',
+  },
+  { name: 'MuiPickersMobileKeyboardInputView' }


Maybe? Not sure

Suggested change

{ name: 'MuiPickersMobileKeyboardInputView' }

{ name: 'MuiMobileKeyboardInputView' }

oliviertassinari · 2020-07-13T15:40:27Z

@oliviertassinari I am not sure that we need to name all the files with Picker it is the same as in the core repo you are not naming each file with Mui. Current solution works nice and keeps displayName/exportName in sync

Do you mean on the file system?

dmtrKovalenko · 2020-07-13T15:41:35Z

Yes I mean the file system

oliviertassinari · 2020-07-13T15:43:21Z

So, assuming we bring nested imports, we would have?

import { PickersDay, PickersDayProps } from '@material-ui/pickers';
import PickersDay, { PickersDayProps } from '@material-ui/pickers/Day';

No strong point of view, points that support your proposal:

People can figure that if Pickers is in the import name, it's already included in the package name, they can remove it
We might progressively drop support for nested import (not sure, but maybe) Support for nested imports? #1934

👍 from my end

dmtrKovalenko · 2020-07-13T15:45:02Z

My point of view: We must drop support of public nested imports and default exports at all, we could discuss it separately :)

oliviertassinari · 2020-07-13T15:46:12Z

My point of view: We must drop support of public nested imports and default exports at all

I'm pinning this subject, a potentially good one to discuss the next team meeting.

Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>

oliviertassinari · 2020-07-14T08:41:10Z

docs/pages/demo/datepicker/CustomDay.example.jsx

 // this guy required only on the docs site to work with dynamic date library
 import { makeJSDateObject } from '../../../utils/helpers';
+import { DatePicker, PickersDay } from '@material-ui/pickers';


Ohh, look like eslint-plugin-pretty-imports comes with a new cost I didn't see coming (first one is that it requires running eslint --fix): it creates harder review and git diff: it's moving code around.

oliviertassinari · 2020-07-14T08:41:48Z

docs/pages/guides/CssOverrides.example.tsx

+        '&$disabled': {
+          color: lightBlue['100'],
+        },
+        '&$selected': {


Regarding the change of pseudo classes, I think that we could have handled in a different pull request, keeping our focus on a single topic at the time :)

oliviertassinari · 2020-07-14T08:42:09Z

lib/src/__tests__/DatePickerTestingLib.test.tsx

@@ -4,7 +4,7 @@ import { getByMuiTest } from './test-utils';
 import { screen, waitFor } from '@testing-library/react';
 import { utilsToUse, FakeTransitionComponent } from './test-utils';
 import { createClientRender, fireEvent } from './createClientRender';
-import { DatePicker, MobileDatePicker, DesktopDatePicker } from '@material-ui/pickers';
+import { DatePicker, MobileDatePicker, DesktopDatePicker } from '../index';


Suggested change

import { DatePicker, MobileDatePicker, DesktopDatePicker } from '../index';

import { DatePicker, MobileDatePicker, DesktopDatePicker } from '@material-ui/pickers';

oliviertassinari · 2020-07-14T08:42:18Z

lib/src/__tests__/DateRangePicker.test.tsx

 import { screen, waitFor } from '@testing-library/react';
 import { utilsToUse, getAllByMuiTest } from './test-utils';
 import { TextField, TextFieldProps } from '@material-ui/core';
+import { DesktopDateRangePicker, StaticDateRangePicker } from '../';


Suggested change

import { DesktopDateRangePicker, StaticDateRangePicker } from '../';

import { DesktopDateRangePicker, StaticDateRangePicker } from '@material-ui/picker';

No it doesn’t work locally for some reason. Need investigation

oliviertassinari · 2020-07-14T08:43:07Z

lib/src/views/Calendar/Month.tsx

@@ -27,15 +27,15 @@ export const useStyles = makeStyles(
        color: theme.palette.primary.main,
        fontWeight: theme.typography.fontWeightMedium,
      },
+      '&:disabled': {
+        pointerEvents: 'none',
+        color: theme.palette.text.hint,


Seems to be still present

lib/src/views/Calendar/Year.tsx

* Change component name from MuiPickers to MuiPicker * Name root classnames "root" * Use pseudo-classes for disabled styling * Remove useless yearDisabled class * Update demo and typescript tests for overrides * Repalce MuiPicker with MuiPickers * Consolidate right component display name with mui component name * Update index.ts reexports to match component display names * Remove useless parameter * Fix docs example * Fix override example tsc * Use pseudo-classes for selected and disabled, closes mui#1845 * Address PR feedback * Update lib/src/Picker/Picker.tsx Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com> * Update lib/src/views/Clock/Clock.tsx Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com> * Make more convenient css classes names * Remove today border from selected day * Remove useless comment * Update tests * Fix global overrid of Mui-selected and Mui-disabled classes * Use theme.palette.secondary instead of hint * Fix linter * Fix incorrect package prefix Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>

dmtrKovalenko added 2 commits July 3, 2020 10:46

Change component name from MuiPickers to MuiPicker

85ae0c0

Name root classnames "root"

0568a89

Use pseudo-classes for disabled styling

554d34e

vercel bot deployed to Preview July 3, 2020 08:37 View deployment

Remove useless yearDisabled class

d75bf55

vercel bot deployed to Preview July 3, 2020 09:05 View deployment

Update demo and typescript tests for overrides

2b4274d

vercel bot deployed to Preview July 3, 2020 09:30 View deployment

oliviertassinari reviewed Jul 3, 2020

View reviewed changes

dmtrKovalenko added 4 commits July 13, 2020 17:41

Merge conflicts

1d2bf35

Repalce MuiPicker with MuiPickers

737f343

Consolidate right component display name with mui component name

73a144d

Update index.ts reexports to match component display names

33f4f12

vercel bot temporarily deployed to Preview July 13, 2020 15:04 Inactive

Remove useless parameter

e7b8b48

vercel bot deployed to Preview July 13, 2020 15:05 View deployment

dmtrKovalenko requested a review from oliviertassinari July 13, 2020 15:05

Fix docs example

1720a0b

vercel bot deployed to Preview July 13, 2020 15:18 View deployment

Fix override example tsc

3e2cd6c

vercel bot requested a deployment to Preview July 13, 2020 15:32 Abandoned

vercel bot requested a deployment to Preview July 13, 2020 15:36 Abandoned

oliviertassinari reviewed Jul 13, 2020

View reviewed changes

dmtrKovalenko added 2 commits July 13, 2020 22:36

Use pseudo-classes for selected and disabled, closes #1845

1e72edf

Address PR feedback

27c5fec

vercel bot temporarily deployed to Preview July 13, 2020 19:36 Inactive

Update lib/src/Picker/Picker.tsx

dc073e3

Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>

vercel bot temporarily deployed to Preview July 13, 2020 19:39 Inactive

Update lib/src/views/Clock/Clock.tsx

145bfa8

Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>

vercel bot deployed to Preview July 13, 2020 19:39 View deployment

Make more convenient css classes names

62bed3a

vercel bot deployed to Preview July 13, 2020 19:49 View deployment

oliviertassinari approved these changes Jul 13, 2020

View reviewed changes

dmtrKovalenko added 3 commits July 14, 2020 10:27

Remove today border from selected day

e7d7079

Remove useless comment

3ce0f32

Update tests

dc95144

vercel bot deployed to Preview July 14, 2020 08:37 View deployment

oliviertassinari reviewed Jul 14, 2020

View reviewed changes

oliviertassinari self-requested a review July 14, 2020 08:44

Fix global overrid of Mui-selected and Mui-disabled classes

82f6614

vercel bot deployed to Preview July 14, 2020 09:11 View deployment

dmtrKovalenko added 2 commits July 14, 2020 12:27

Use theme.palette.secondary instead of hint

3246564

Fix linter

634d638

vercel bot temporarily deployed to Preview July 14, 2020 09:29 Inactive

Fix incorrect package prefix

850a8da

vercel bot deployed to Preview July 14, 2020 09:31 View deployment

oliviertassinari approved these changes Jul 14, 2020

View reviewed changes

dmtrKovalenko merged commit 54faaad into next Jul 14, 2020

dmtrKovalenko deleted the feature/classes-naming branch July 14, 2020 10:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Consolidate classes naming #1950

Consolidate classes naming #1950

dmtrKovalenko commented Jul 3, 2020 •

edited by oliviertassinari

Loading

vercel bot commented Jul 3, 2020 •

edited

Loading

cypress bot commented Jul 3, 2020 •

edited

Loading

oliviertassinari Jul 3, 2020

dmtrKovalenko Jul 6, 2020

oliviertassinari Jul 6, 2020

oliviertassinari Jul 13, 2020

oliviertassinari Jul 3, 2020

dmtrKovalenko commented Jul 13, 2020

oliviertassinari left a comment

oliviertassinari Jul 13, 2020

oliviertassinari Jul 13, 2020

oliviertassinari Jul 13, 2020

dmtrKovalenko Jul 13, 2020

oliviertassinari Jul 14, 2020

oliviertassinari Jul 13, 2020

oliviertassinari Jul 13, 2020

oliviertassinari commented Jul 13, 2020 •

edited

Loading

dmtrKovalenko commented Jul 13, 2020

oliviertassinari commented Jul 13, 2020 •

edited

Loading

dmtrKovalenko commented Jul 13, 2020 •

edited

Loading

oliviertassinari commented Jul 13, 2020

oliviertassinari Jul 14, 2020 •

edited

Loading

oliviertassinari Jul 14, 2020

oliviertassinari Jul 14, 2020

oliviertassinari Jul 14, 2020

dmtrKovalenko Jul 14, 2020

oliviertassinari Jul 14, 2020

	import { useStyles as MuiPickerYearStyles } from '../views/Calendar/Year';
	import { useStyles as MuiPickersYearStyles } from '../views/Calendar/PickerYear';

	name: 'MuiPickerClockPointer',
	name: 'MuiPickersClockPointer',

	{ name: 'MuiPickersMobileKeyboardInputView' }
	{ name: 'MuiMobileKeyboardInputView' }

	import { DatePicker, MobileDatePicker, DesktopDatePicker } from '../index';
	import { DatePicker, MobileDatePicker, DesktopDatePicker } from '@material-ui/pickers';

	import { DesktopDateRangePicker, StaticDateRangePicker } from '../';
	import { DesktopDateRangePicker, StaticDateRangePicker } from '@material-ui/picker';

Consolidate classes naming #1950

Consolidate classes naming #1950

Conversation

dmtrKovalenko commented Jul 3, 2020 • edited by oliviertassinari Loading

vercel bot commented Jul 3, 2020 • edited Loading

cypress bot commented Jul 3, 2020 • edited Loading

Test summary

Run details

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmtrKovalenko commented Jul 13, 2020

oliviertassinari left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliviertassinari commented Jul 13, 2020 • edited Loading

dmtrKovalenko commented Jul 13, 2020

oliviertassinari commented Jul 13, 2020 • edited Loading

dmtrKovalenko commented Jul 13, 2020 • edited Loading

oliviertassinari commented Jul 13, 2020

oliviertassinari Jul 14, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmtrKovalenko commented Jul 3, 2020 •

edited by oliviertassinari

Loading

vercel bot commented Jul 3, 2020 •

edited

Loading

cypress bot commented Jul 3, 2020 •

edited

Loading

oliviertassinari commented Jul 13, 2020 •

edited

Loading

oliviertassinari commented Jul 13, 2020 •

edited

Loading

dmtrKovalenko commented Jul 13, 2020 •

edited

Loading

oliviertassinari Jul 14, 2020 •

edited

Loading