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] Create Pickers accessibility page #13274

Conversation

arthurbalduini
Copy link
Member

This PR introduces the accessibility content for Date Pickers and its components.
It's part of the issue #6451

Another PR with content about Time Pickers will be opened afterwards.

@arthurbalduini arthurbalduini added docs Improvements or additions to the documentation accessibility a11y component: date picker This is the name of the generic UI component, not the React module! component: date range picker This is the name of the generic UI component, not the React module! labels May 28, 2024
@arthurbalduini arthurbalduini requested a review from a team May 28, 2024 06:54
Copy link
Member

@michelengelen michelengelen left a comment

Choose a reason for hiding this comment

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

A few suggestions! 👍🏻

docs/data/date-pickers/accessibility/accessibility.md Outdated Show resolved Hide resolved
docs/data/date-pickers/accessibility/accessibility.md Outdated Show resolved Hide resolved
docs/data/date-pickers/accessibility/accessibility.md Outdated Show resolved Hide resolved
docs/data/date-pickers/accessibility/accessibility.md Outdated Show resolved Hide resolved
@LukasTy LukasTy changed the title [docs] Creates accessibility page with content for date pickers [docs] Create Pickers accessibility page May 29, 2024
Copy link
Member

@LukasTy LukasTy left a comment

Choose a reason for hiding this comment

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

Great work!
Leaving some suggestions and discussions points. 😉

docs/data/date-pickers/accessibility/accessibility.md Outdated Show resolved Hide resolved
docs/data/date-pickers/accessibility/accessibility.md Outdated Show resolved Hide resolved
docs/data/date-pickers/accessibility/accessibility.md Outdated Show resolved Hide resolved
docs/data/date-pickers/accessibility/accessibility.md Outdated Show resolved Hide resolved
docs/data/date-pickers/accessibility/accessibility.md Outdated Show resolved Hide resolved
docs/data/date-pickers/accessibility/accessibility.md Outdated Show resolved Hide resolved
docs/data/date-pickers/accessibility/accessibility.md Outdated Show resolved Hide resolved
docs/data/date-pickers/accessibility/accessibility.md Outdated Show resolved Hide resolved
docs/data/date-pickers/accessibility/accessibility.md Outdated Show resolved Hide resolved
docs/data/date-pickers/accessibility/accessibility.md Outdated Show resolved Hide resolved
arthurbalduini and others added 4 commits May 30, 2024 14:48
Co-authored-by: Lukas <llukas.tyla@gmail.com>
Signed-off-by: Arthur Suh Balduini <34691066+arthurbalduini@users.noreply.github.com>
Co-authored-by: Lukas <llukas.tyla@gmail.com>
Signed-off-by: Arthur Suh Balduini <34691066+arthurbalduini@users.noreply.github.com>
@arthurbalduini arthurbalduini linked an issue May 31, 2024 that may be closed by this pull request
Copy link
Member

@samuelsycamore samuelsycamore left a comment

Choose a reason for hiding this comment

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

great addition to the docs @arthurbalduini ! I really appreciate that you've applied the style guide consistently. 🙌😁 This review is mostly copyediting.


## Guidelines

The most commonly encountered conformance guidelines for accessibility are:
Copy link
Member

Choose a reason for hiding this comment

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

copyediting

Suggested change
The most commonly encountered conformance guidelines for accessibility are:
Common conformance guidelines for accessibility include:

Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: This section is a copy and paste from Data Grid and Tree View a11y pages.
If we apply this change, it would be nice to replicate it on those pages as well. 👍

Copy link
Member

Choose a reason for hiding this comment

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

@arthurbalduini do you plan on applying these changes?
Or are you leaving that for a separate PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

After discussing with @samuelsycamore, I will apply these changes and also replicate them to the other pages where they appear on this PR

Comment on lines 23 to 24
Level AA meets the most commonly encountered conformance guidelines.
This is the most common target for organizations so it is what we aim to support.
Copy link
Member

@samuelsycamore samuelsycamore May 31, 2024

Choose a reason for hiding this comment

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

I like describing this as "exceeding the basic criteria" because it sounds more reassuring if you're not deeply informed about this stuff.

Suggested change
Level AA meets the most commonly encountered conformance guidelines.
This is the most common target for organizations so it is what we aim to support.
Level AA exceeds the basic criteria for accessibility and is a common target for most organizations, so this is what we aim to support.

docs/data/date-pickers/accessibility/accessibility.md Outdated Show resolved Hide resolved
docs/data/date-pickers/accessibility/accessibility.md Outdated Show resolved Hide resolved
docs/data/date-pickers/accessibility/accessibility.md Outdated Show resolved Hide resolved

### Date Range Picker

The [Date Range Picker](/x/react-date-pickers/date-range-picker/) keeps the focus on the field all the time and thus has the same keyboard navigation as Date Range [Field](/x/react-date-pickers/accessibility/#fields).
Copy link
Member

Choose a reason for hiding this comment

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

I would use more than one word for link text whenever possible—three is the ideal minimum. I don't fully understand what's meant by "all the time"—I would reword this as "at all times" if I understood the meaning correctly. Can you elaborate on what this means or implies for the UX?

Suggested change
The [Date Range Picker](/x/react-date-pickers/date-range-picker/) keeps the focus on the field all the time and thus has the same keyboard navigation as Date Range [Field](/x/react-date-pickers/accessibility/#fields).
The [Date Range Picker](/x/react-date-pickers/date-range-picker/) keeps the focus on the field at all times and thus has the same keyboard navigation as the [Date Range Field](/x/react-date-pickers/accessibility/#fields).

Copy link
Member

Choose a reason for hiding this comment

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

This references the current Date Range Picker UX pattern, where the focus is returned to the input after selection in an open Picker view as compared to Date Picker, where the focus is kept in the Popper until the complete selection is made.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have rewritten this part on #c10ee9f. Would appreciate your opinion ! :)

arthurbalduini and others added 5 commits June 5, 2024 14:54
Co-authored-by: Sycamore <71297412+samuelsycamore@users.noreply.github.com>
Co-authored-by: Lukas <llukas.tyla@gmail.com>
Signed-off-by: Arthur Suh Balduini <34691066+arthurbalduini@users.noreply.github.com>
@LukasTy LukasTy removed component: date picker This is the name of the generic UI component, not the React module! component: date range picker This is the name of the generic UI component, not the React module! labels Jun 6, 2024
Copy link
Member

@LukasTy LukasTy left a comment

Choose a reason for hiding this comment

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

Great work! 💯
A few final nitpicks. 😉


## Guidelines

The most commonly encountered conformance guidelines for accessibility are:
Copy link
Member

Choose a reason for hiding this comment

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

@arthurbalduini do you plan on applying these changes?
Or are you leaving that for a separate PR?


### Date Range Picker

When interacting with the keyboard, the [Date Range Picker](/x/react-date-pickers/date-range-picker/) keep the focus on the Field component, thereby offering the same keyboard navigation support as the [Date Range Field](/x/react-date-pickers/accessibility/#fields).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
When interacting with the keyboard, the [Date Range Picker](/x/react-date-pickers/date-range-picker/) keep the focus on the Field component, thereby offering the same keyboard navigation support as the [Date Range Field](/x/react-date-pickers/accessibility/#fields).
When interacting with the keyboard, the [Date Range Picker](/x/react-date-pickers/date-range-picker/) keeps the focus on the Field component, thereby offering the same keyboard navigation support as the [Date Range Field](/x/react-date-pickers/accessibility/#fields).

P.S. WDYT about removing the Date Range Field or the Field link below on L84? 🤔
They point to the same section and are too close to one another IMHO. 🙈

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, and actually I think L82 and L84 are redundant

@arthurbalduini arthurbalduini merged commit 3247869 into mui:master Jun 6, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility a11y component: pickers This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[docs][pickers] Add a11y/keyboard navigation documentation
6 participants