Skip to content
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] Improve English and fix 404 link #1815

Merged
merged 5 commits into from
May 27, 2020
Merged

[docs] Improve English and fix 404 link #1815

merged 5 commits into from
May 27, 2020

Conversation

dandv
Copy link
Contributor

@dandv dandv commented May 25, 2020

This PR hopefully closes #1814 partially.

@vercel
Copy link

vercel bot commented May 25, 2020

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/200r3km80
✅ Preview: https://material-ui-pickers-git-fork-dandv-patch-1.mui-org.now.sh

@cypress
Copy link

cypress bot commented May 25, 2020



Test summary

77 0 1 0


Run details

Project material-ui-pickers
Status Passed
Commit cdccff9
Started May 26, 2020 10:12 AM
Ended May 26, 2020 10:14 AM
Duration 01:23 💡
OS Linux Debian - 9.11
Browser Chrome 78

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

Co-authored-by: Dmitriy Kovalenko <dmtr.kovalenko@outlook.com>
@oliviertassinari oliviertassinari changed the title [docs] improve English and attempt to fix #1814 [docs] Improve English and attempt to fix #1814 May 25, 2020
@oliviertassinari oliviertassinari changed the title [docs] Improve English and attempt to fix #1814 [docs] Improve English and fix 404 link May 25, 2020
@dmtrKovalenko
Copy link
Member

I think we should mention not only ‘minTime’ and minDateTime but also max analogues.

Co-authored-by: Dmitriy Kovalenko <dmtr.kovalenko@outlook.com>
Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated the markdown to fix the 404 links as well as uniformize the generated markdown between the components. A couple of thoughts:

  • I wonder what should be our strategy to document the Mobile, Desktop, and Static variants of the components? I wouldn't be against a standalone component file, e.g. MobileDatePicker.js and API page.
  • The heading structure of the docs is wrong. The h1 > h2 > h3 structure should be continuous (for a11y). For instance, there is no h1, it starts (❌) from h2, then there is no h3, to jumps (❌) to h4. This gave me a new item idea to list into [DatePicker] Misc TODO material-ui#20960: the accessibility of the docs. A report by the Chrome Wave extension:

wave

  • There are a couple of UI jumps between the SSR rendered and hydrated versions of the components, I'm surprised nobody reported it yet (I guess people use it for client-side only dashboard, not e-commerce much). I think that it will come with more usage, happy to be lazy and wait.

@dmtrKovalenko
Copy link
Member

dmtrKovalenko commented May 26, 2020

Answering:

  1. No, we do not need to link API for each component, because they are mostly identical. In v3 KeyboardDatePicker and DatePicker had different props
  2. Totally right, shame on me 👎
  3. It is because an idea to prerender time on the server is wrong. Because of timezones conflict, it can be useful for some edge-cases like a google calendar app or some meeting app. But I think just nobody using SSR with picker content (like calendar or clock)

@oliviertassinari
Copy link
Member

But I think just nobody using SSR with picker content (like calendar or clock)

Yeah, I guess it's a much smaller segment of the market 😆.

@dmtrKovalenko dmtrKovalenko merged commit 9fa389f into mui:next May 27, 2020
@dandv dandv deleted the patch-1 branch February 21, 2021 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Broken link to KeyboardDateTimePicker on v4 page
3 participants