-
Notifications
You must be signed in to change notification settings - Fork 832
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
LocalizationProvider #1537
LocalizationProvider #1537
Conversation
This pull request is being automatically deployed with ZEIT Now (learn more). 🔍 Inspect: https://zeit.co/mui-org/material-ui-pickers/pl8kib09x |
Test summaryRun details
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 |
Codecov Report
@@ Coverage Diff @@
## next #1537 +/- ##
=======================================
Coverage 91.83% 91.83%
=======================================
Files 63 63
Lines 1763 1763
Branches 320 304 -16
=======================================
Hits 1619 1619
Misses 109 109
Partials 35 35
Continue to review full report at Codecov.
|
README.md
Outdated
import { MuiPickersUtilsProvider } from '@material-ui/pickers'; | ||
import Luxon from '@date-io/luxon'; | ||
import Moment from '@date-io/moment'; | ||
import DateFns from '@date-io/date-fns'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the import come from?
import DateFns from '@date-io/date-fns'; | |
import dateFns from '@material-ui/pickers/adapter/date-fns'; |
But I wonder about keeping the file system flat?
import DateFns from '@date-io/date-fns'; | |
import dateFnsAdapter from '@material-ui/pickers/dateFnsAdapter'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the main point is to make library names the same (date-fns
) and not dateFnsAdapter
? What do you think about it, IMHO it will be more clear for users
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a clear idea of the best approach. There is a tradeoff between the consitency of the filenames and the npm name of the dependencies. Both seem OK.
@@ -0,0 +1,35 @@ | |||
import React, { useState } from 'react'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think of using React.useState
?
import React, { useState } from 'react'; | |
import React from 'react'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is even more invalid, somewhere I thought that this kind of import will be deprecated. We are primarily using import * as React
, but in examples using this notation.
I have no preference and can change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a related thread on this topic at mui/material-ui#19802.
What approach have you seen the more frequently used by TypeScript users on one side? And by JavaScript users on the other?
There are no clear guidelines by React on the matter, but I could find these two examples:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For typescript codebase, it is much easier to use import * as React
because a lot of internal types are also exported, like React.ReactNode
, React.HtmlProps
and so on.
My own notice: It is easier to visual recognize default react hooks like
useEffect
oruseState
when they are used likeReact.useEffect
, thus all custom hooks are easily distinguishable
Should we update Readme? Because it will affect new v3.x, because next is our default branch. |
Maybe the Readme should point to the documentation websit and be kept generic? |
Closes #1504
User facing changes
LocalizationProvider
Rename and changes in API for
MuiPickersUtilsProvider
. It is renamed toLocalizationProvider
. In future this provider may be moved to the@material-ui/core
Also,
LocalizationProvider
now accepts specialdateFormats
prop that allows globally override formats used for displaying datesAlso,
libInstance
from now renamed todateLibInstance
which is a more meaningful name in case of reusableLocalizationProvider
dateAdapter prop
There is a new prop
dateAdapter
in each component which allows passing configured date adapter object without context