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.

Sign up for GitHub

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

Jump to bottom

feat(calendar): add accessibility #73

Merged
merged 22 commits into from
Dec 13, 2021
Merged

feat(calendar): add accessibility #73

merged 22 commits into from
Dec 13, 2021

Conversation

goremikins
Copy link
Contributor

@goremikins goremikins commented Nov 9, 2021 •
edited
Loading

Description

Improves screen reader support

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have checked my code and corrected any misspellings

@goremikins
feat(calendar): add accessibility
5319f2c 
feat(phrasebook): add accessibility labels to calendar
@goremikins goremikins added the accessibility Accessibility improvement label Nov 9, 2021
@goremikins goremikins requested a review from wsuwt November 9, 2021 11:48
@wsuwt wsuwt requested a review from Jidapa-Pai November 12, 2021 11:21
@wsuwt wsuwt self-requested a review November 15, 2021 08:37
@wsuwt wsuwt marked this pull request as ready for review November 19, 2021 03:45
@Jidapa-Pai
Copy link
Contributor

I'm already checked issue about tap on year after merge this code #81 from develop, it works now.

@wsuwt wsuwt marked this pull request as draft November 19, 2021 10:35
@goremikins
test(calendar): keyboard navigation tests
57093bb 
feat(phrasebook): change wording for calendar
@goremikins goremikins marked this pull request as ready for review November 29, 2021 17:21
@goremikins
Merge remote-tracking branch 'origin/develop' into accessibility/cale…
cead7d2 
…ndar

# Conflicts:
#	packages/elements/src/calendar/__snapshots__/Defaults.md
#	packages/elements/src/calendar/__snapshots__/Multiple.md
#	packages/elements/src/calendar/__snapshots__/Navigation.md
#	packages/elements/src/calendar/__snapshots__/Range.md
#	packages/elements/src/calendar/__snapshots__/Value.md
@goremikins
Merge remote-tracking branch 'origin/develop' into accessibility/cale…
50f6f54 
…ndar
@goremikins
Merge remote-tracking branch 'origin/develop' into accessibility/cale…
46004cb 
…ndar
@goremikins
Merge remote-tracking branch 'origin/develop' into accessibility/cale…
b34ca6b 
…ndar
@goremikins
refactor(calendar): aria-selected should be always applied to selec…
006a0f6 
…table cells

chore(elements): update package-lock
@goremikins
Merge remote-tracking branch 'origin/develop' into accessibility/cale…
313b414 
…ndar
@goremikins
fix(calendar): do not fix the view on first render
7efd4e0
@goremikins
refactor(calendar): simplify the code
6406b49 
refactor(calendar): do not produce aria-label for non selectable cells
refactor(calendar): make sure IE11 renders in reasonable time
@goremikins
Merge branch 'develop' into accessibility/calendar
bb22be0
@goremikins
Copy link
Contributor Author

goremikins commented Dec 9, 2021 •
edited
Loading

@TremayneChrist , @wsuwt , I think this need another walkthrough. Most notably I have changed the following:

  1. IE11, navigation did not work
  2. IE11, it took absolutely ages to load the calendar. The reason is because polyfills are required to render formatted dates. So I removed aria-label for all elements that cause problem in IE11. screen-reader continues to work in IE11, but will not pronounce selection and full days. I think it is acceptable
  3. Simplified some types
  4. Changed the way how navigation maps are calculated to keep the code cleaner
  5. A button should be used to select dates, but not a cell

@goremikins goremikins marked this pull request as draft December 9, 2021 18:20
@goremikins
refactor(calendar): use abbr instead of aria-label for headers
949860f 
refactor(calendar): a clickable element should be a button, not cell
refactor(elemental-theme): support different selection model in calendar
refactor(solar-theme): support different selection model in calendar
refactor(halo-theme): support different selection model in calendar
@goremikins
fix(calendar): aria-selected and aria-multiselectable applied at the …
c28bff1 
…wrong level
@goremikins goremikins marked this pull request as ready for review December 13, 2021 11:37
@goremikins goremikins merged commit a2576cc into develop Dec 13, 2021
@goremikins goremikins deleted the accessibility/calendar branch December 13, 2021 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wsuwt wsuwt wsuwt approved these changes

@Jidapa-Pai Jidapa-Pai Awaiting requested review from Jidapa-Pai

Assignees

@wsuwt wsuwt

Labels
accessibility Accessibility improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@goremikins @Jidapa-Pai @wsuwt @phetw