fix(datepicker): add footer with close button for screen readers #14429
Conversation
googlebot
added
the
cla: yes
|
issue #14379 |
| @@ -0,0 +1,9 @@ | |||
| <div class="mat-calendar-footer cdk-visually-hidden"> | |||
There was a problem hiding this comment.
@jelbourn shouldn't we show the button if it receives focus? Otherwise people that are tabbing through the dialog without screen readers will land on it with no feedback. See how GitHub does it with their "Skip to content" button.
|
Hi @crisbeto , I appreciate your feedback. I have added the appropriate unit tests and removed the useless cdkAriaLive attribute from the button. Please keep me posted on whether the button should be shown when focused. Thanks! |
|
@crisbeto you make a good point. We could make the button visible while focused by positioning it at the bottom-right corner as |
|
@mmalerba can you take a look? |
src/lib/datepicker/datepicker.ts
Outdated
| @@ -145,6 +145,9 @@ export class MatDatepicker<D> implements OnDestroy, CanColor { | |||
| /** An input indicating the type of the custom header component for the calendar, if set. */ | |||
| @Input() calendarHeaderComponent: ComponentType<any>; | |||
|
|
|||
| /** An input indicating the type of the custom footer component for the calendar, if set. */ | |||
| @Input() calendarFooterComponent: ComponentType<any>; | |||
There was a problem hiding this comment.
I don't know about this, I don't think we want to let users drop content in here. At least not the way it's currently set up in this PR, the content will just wind up floating below the calendar.
For now how about just putting a button in mat-datepicker-content and we can consider opening it up via an API at some point in the future
Users with screen readers have difficulty closing the datepicker popup obviously because they would have a hard time tapping the backdrop. Also, on mobile device screen readers they do not have an escape key.
|
Is there any help needed to finish up this PR ? From what I can see the main content is ok and approved and the last work is about making the changes pass the CI right ? |
| </mat-calendar> | ||
|
|
||
| <!-- An invisible close button so users with screen readers can easily close the datepicker --> | ||
| <div cdkMonitorSubtreeFocus |
There was a problem hiding this comment.
This seems like overkill, you can just use (focus) and (blur) events on the button
| position: absolute; | ||
| margin-top: 10px; | ||
| &.cdk-keyboard-focused { | ||
| background: #2468cf; |
There was a problem hiding this comment.
Where are these colors coming from? Would a mat-raised-button with color="primary" work instead?
| &.cdk-keyboard-focused { | ||
| background: #2468cf; | ||
| color: #ffffff; | ||
| z-index: 999; |
There was a problem hiding this comment.
This seems unnecessarily high, will z-index: 1 work?
| super(elementRef); | ||
| } | ||
|
|
||
| ngAfterViewInit() { | ||
| this._calendar.focusActiveCell(); | ||
| } | ||
|
|
||
| formatOrigin(origin: FocusOrigin): string { |
There was a problem hiding this comment.
use a boolean variable (e.g. isCloseButtonFocused)
| return origin ? origin + ' focused' : 'blurred'; | ||
| } | ||
|
|
||
| markForCheck() { |
There was a problem hiding this comment.
this shouldn't be necessary if you switch to just using the (focus) and (blur) events
| /** For the focus monitor */ | ||
| closeButtonElementOrigin: string = this.formatOrigin(null); | ||
|
|
||
| get closeButtonLabel(): string { |
There was a problem hiding this comment.
Getters compile down to a lot of code (relative to just a plain method). Can you change this to a method (e.g. getCloseButtonLabel), or just make _intl public and access it directly in the template
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Users with screen readers have difficulty closing the datepicker popup
obviously because they would have a hard time tapping the backdrop.
Also, on mobile device screen readers they do not have an escape key.