Skip to content

Adding ARIA for calendar component#8526

Merged
zdrawku merged 42 commits intomasterfrom
mpopov/calendar-accessibility-master
Nov 10, 2020
Merged

Adding ARIA for calendar component#8526
zdrawku merged 42 commits intomasterfrom
mpopov/calendar-accessibility-master

Conversation

@desig9stein
Copy link
Contributor

@desig9stein desig9stein commented Nov 5, 2020

Closes #6272

Additional information (check all that apply):

  • Bug fix
  • New functionality
  • Documentation
  • Demos
  • CI/CD

Checklist:

  • All relevant tags have been applied to this PR
  • This PR includes unit tests covering all the new code (test guidelines)
  • This PR includes API docs for newly added methods/properties (api docs guidelines)
  • This PR includes feature/README.MD updates for the feature docs
  • This PR includes general feature table updates in the root README.MD
  • This PR includes CHANGELOG.MD updates for newly added functionality
  • This PR contains breaking changes
  • This PR includes ng update migrations for the breaking changes (migrations guidelines)
  • This PR includes behavioral changes and the feature specification has been updated with them

MPopov and others added 26 commits September 30, 2020 15:12
…niteui-angular into mpopov/calendar-accessibility

� Conflicts:
�	projects/igniteui-angular/src/lib/calendar/calendar.component.html
�	projects/igniteui-angular/src/lib/calendar/months-view/months-view.component.html
…niteui-angular into mpopov/calendar-accessibility

� Conflicts:
�	projects/igniteui-angular/src/lib/calendar/calendar.component.html
�	projects/igniteui-angular/src/lib/calendar/calendar.component.ts
…into mpopov/calendar-accessibility

� Conflicts:
�	projects/igniteui-angular/src/lib/calendar/calendar.component.html
@simeonoff simeonoff requested a review from ddincheva November 5, 2020 09:48
@desig9stein desig9stein requested review from damyanpetev and zdrawku and removed request for ddincheva November 5, 2020 09:51
@ddincheva ddincheva added the 💥 status: in-test PRs currently being tested label Nov 5, 2020
@ddincheva ddincheva added ✅ status: verified Applies to PRs that have passed manual verification and removed 💥 status: in-test PRs currently being tested labels Nov 5, 2020
zdrawku
zdrawku previously approved these changes Nov 9, 2020
Copy link
Contributor

@zdrawku zdrawku left a comment

Choose a reason for hiding this comment

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

As we discussed with Damyan, his comment will be addressed with a future release

Copy link
Contributor

@zdrawku zdrawku left a comment

Choose a reason for hiding this comment

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

@desig9stein could you please update the readme.md file and changelog if necessary

Copy link
Member

@damyanpetev damyanpetev left a comment

Choose a reason for hiding this comment

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

Requesting changes mainly for the localization strings comments, the rest might be logged as separate issue.

<div class="igx-calendar__body-row">
<div *ngIf="showWeekNumbers" class="igx-calendar__label igx-calendar__label--week-number">
<div role="row" class="igx-calendar__body-row">
<div role="columnheader" *ngIf="showWeekNumbers" class="igx-calendar__label igx-calendar__label--week-number">
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for adding this 👍, NVDA reads rows and column headers with without the week number being specified a header, you end with 8 columns and 7 weekday headers and the reading is skewed when numbers are active.

<div class="igx-calendar__body">
<div class="igx-calendar__body-column" (wheel)="scroll($event)" (pan)="pan($event)">
<span [igxCalendarYear]="year" [date]="date" (onYearSelection)="selectYear($event)" *ngFor="let year of decade; trackBy: yearTracker">
<ul class="igx-calendar__body-column" (wheel)="scroll($event)" (pan)="pan($event)">
Copy link
Member

Choose a reason for hiding this comment

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

Okay, something bothers me about this one. It's not quite right describing it as a list. Some AT do read the list when entering and "List with 7 items" or "4 out of 7" are not really descriptive of how this component behaves. It's and endless scroll.. or spin ☺️
So quite a lot like this: https://www.w3.org/TR/wai-aria-practices/examples/spinbutton/datepicker-spinbuttons.html
I'd love to say you can get away with reverting these back to div/span and binding the role in the igxCalendarYear directive in a similar way as it's done for the tabindex:

    @HostBinding('attr.role')
    public get role(): string {
        return this.isCurrentYear ? 'spinbutton' : null;
    }

    @HostBinding('attr.aria-valuenow')
    public get valuenow(): number {
        return this.isCurrentYear ? this.date.getFullYear() : null;
    }

This works for the most part, except when navigating the year list items actually shift positions in the DOM for whatever reason - that has the unfortunate effect of announcing the spin button every time since you focus a new element each time :/
PS: Also NVDA insists on navigating lists on its own (haven't found the setting to stop that) and thus swallows the key up/down events and the list never scrolls for me, so that'll fix that 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.

If this proves too complicated for now I'm down with logging an additional issue to improve the years picker/view.

@ddincheva ddincheva requested a review from zdrawku November 10, 2020 11:28
@zdrawku zdrawku merged commit 7e93477 into master Nov 10, 2020
@zdrawku zdrawku deleted the mpopov/calendar-accessibility-master branch November 10, 2020 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aria-support 📆 calendar version: 11.0.x ✅ status: verified Applies to PRs that have passed manual verification

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Calendar ARIA

4 participants