fix: (core) Calendar A11y - change ul/li to table with grid #1649
Conversation
JKMarkowski
requested review from
droshev,
jacobdevera,
InnaAtanasova and
stefanoScalzo
|
Deploy preview for fundamental-ngx ready! Built with commit 5fffc75 |
JKMarkowski
force-pushed
the
fix/1536-calendar-a11y
branch
from
2949d9c
to
2df38df
Compare
There was a problem hiding this comment.
Nice! Just a couple things needed for a11y.
Also, not related to these changes, I noticed that the aria-label for the dates is set at the <td> level. These should be set at the <span> level. Because it is at the <td> level, both the aria-label and the <span>'s content will be read at once. This can be confusing in certain cases e.g. "1 January 2020 1" will sound like the year 2021 to screen reader users.
| @@ -20,6 +20,7 @@ | |||
| [attr.aria-label]="calendarI18nLabels.monthSelectionLabel" | |||
There was a problem hiding this comment.
The aria-label on the month and year buttons should either be removed or dynamically change depending on the month (e.g. "month selection, January") because it overrides the reading of the button's textContent. Because of this, nothing is currently read out when the month/year changes.
| @@ -20,6 +20,7 @@ | |||
| [attr.aria-label]="calendarI18nLabels.monthSelectionLabel" | |||
| [attr.aria-selected]="isOnMonthView()" | |||
| (click)="processViewChange('month')" | |||
| aria-live="polite" | |||
There was a problem hiding this comment.
Having multiple aria-live can cause conflicts. There should just be one aria-live on a parent element (e.g. the <header> would work).
|
Awesome! Something I didn't catch: Disabled dates should have |
|
Hello @jacobdevera, thanks for pointing it out :) It helped a lot during implementation. Attribute |
Please provide a link to the associated issue.
fixes: #1536
Please provide a brief summary of this pull request.
There is changed the way how we build the calendar month and year components. Also there is changed the way how the models are generated. There is also added one missing
aria-livePlease check whether the PR fulfills the following requirements
https://github.com/SAP/fundamental-ngx/blob/master/CONTRIBUTING.md
Documentation checklist:
README.md