-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[docs] Edit the Pickers Getting started doc #14555
[docs] Edit the Pickers Getting started doc #14555
Conversation
samuelsycamore
commented
Sep 9, 2024
- fix typos and grammar mistakes
- copyedits for clarity and brevity throughout
- apply style guide
- move some content here from Base concepts page - will follow up with another PR for that doc
@@ -1,34 +1,38 @@ | |||
--- | |||
productId: x-date-pickers | |||
title: Date and Time Picker - Getting started | |||
title: Date and Time Pickers - Getting started |
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.
noticed this throughout these docs - Pickers should always be plural in this context
|
||
## Installation | ||
|
||
Using your favorite package manager, install: | ||
Install the base package (which can either be the free Community version or the paid Pro version) along with a required third-party date library. | ||
The Pickers currently support [Day.js](https://day.js.org/), [date-fns](https://date-fns.org/), [Luxon](https://moment.github.io/luxon/#/), and [Moment.js](https://momentjs.com/). |
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.
feels much better to have this info right here rather than needing to visit the base concepts page
:::info | ||
If you need more information about the date library supported by the Date and Time Pickers, | ||
take a look at the [dedicated section](/x/react-date-pickers/base-concepts/#date-library) | ||
:::success |
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.
another one that feels more useful here than in base concepts
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.
@LukasTy related to our topic from yesterday. With all the problems dayJs has regarding timezones and its relatively unmaintained state are we sure we want to recommend that?
@@ -57,70 +63,40 @@ Please note that [react](https://www.npmjs.com/package/react) and [react-dom](ht | |||
}, | |||
``` | |||
|
|||
### Style engine |
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.
is this info really necessary? Looks like copypasta from the Material UI docs.
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.
Yes, it is actually a copypasta, given that most users probably stick to default—Emotion, there is little need for this section here. 👍
|
||
{{"demo": "FirstComponent.js"}} | ||
|
||
## What's next? |
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.
this feels unnecessary when "what's next" is already baked into the design of the docs at the bottom of the page.
@@ -10,7 +10,6 @@ const packages = { | |||
|
|||
const peerDependency = { | |||
label: 'Date library', | |||
installationComment: '// Install date library (if not already installed)', |
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.
is this really necessary? feels like it just clutters the space
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 should be clear enough without it. 👍
Besides, comments are not rendered properly (no different styling). 🙈 🤷
Deploy preview: https://deploy-preview-14555--material-ui-x.netlify.app/ Updated pages: |
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.
Yay! Love seeing movement here. 😄
Co-authored-by: Ale <93217218+alelthomas@users.noreply.github.com> Signed-off-by: Sycamore <71297412+samuelsycamore@users.noreply.github.com>
@@ -57,70 +63,40 @@ Please note that [react](https://www.npmjs.com/package/react) and [react-dom](ht | |||
}, | |||
``` | |||
|
|||
### Style engine |
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.
Yes, it is actually a copypasta, given that most users probably stick to default—Emotion, there is little need for this section here. 👍
@@ -10,7 +10,6 @@ const packages = { | |||
|
|||
const peerDependency = { | |||
label: 'Date library', | |||
installationComment: '// Install date library (if not already installed)', |
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 should be clear enough without it. 👍
Besides, comments are not rendered properly (no different styling). 🙈 🤷
Co-authored-by: Lukas Tyla <llukas.tyla@gmail.com> Signed-off-by: Michel Engelen <32863416+michelengelen@users.noreply.github.com>
:::info | ||
If you need more information about the date library supported by the Date and Time Pickers, | ||
take a look at the [dedicated section](/x/react-date-pickers/base-concepts/#date-library) | ||
:::success |
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.
@LukasTy related to our topic from yesterday. With all the problems dayJs has regarding timezones and its relatively unmaintained state are we sure we want to recommend that?
Co-authored-by: Michel Engelen <32863416+michelengelen@users.noreply.github.com> Signed-off-by: Sycamore <71297412+samuelsycamore@users.noreply.github.com>
Signed-off-by: Sycamore <71297412+samuelsycamore@users.noreply.github.com> Signed-off-by: Michel Engelen <32863416+michelengelen@users.noreply.github.com> Co-authored-by: Ale <93217218+alelthomas@users.noreply.github.com> Co-authored-by: Michel Engelen <32863416+michelengelen@users.noreply.github.com> Co-authored-by: Lukas Tyla <llukas.tyla@gmail.com>