feat(material/select): allow user-defined aria-describedby #24644
Conversation
RobertAKARobin
force-pushed
the
dev/select-described
branch
from
7440c6d to
7c350b1
Compare
|
I suspect |
| @@ -611,7 +614,7 @@ export abstract class _MatSelectBase<C> | |||
| ngOnChanges(changes: SimpleChanges) { | |||
| // Updating the disabled state is handled by `mixinDisabled`, but we need to additionally let | |||
| // the parent form field know to run change detection when the disabled state changes. | |||
| if (changes['disabled']) { | |||
| if (changes['disabled'] || changes['userAriaDescribedBy']) { | |||
There was a problem hiding this comment.
Why is this change necessary? Since the aria-describedby is on the select it shouldn't be required.
There was a problem hiding this comment.
Without it, this test fails:
it('should support user binding to `aria-describedby`', fakeAsync(() => {
fixture.componentInstance.ariaDescribedBy = 'test';
fixture.detectChanges();
expect(select.getAttribute('aria-describedby')).toBe('test');
}));
As a result of this PR (and #24292 for MatChipList and #19587 for MatInput), the aria-describedby is no longer set in @Component({ host }). Rather, it is set using nativeElement.setAttribute in MatSelect#setDescribedByIds, which is called by MatFormField#_syncDescribedByIds, which in turn is called when stateChanges updates.
So, userAriaDescribedBy has to trigger a stateChanges.next() in order for the aria-describedby to actually be set.
There was a problem hiding this comment.
...and for that matter, MatChipList needs unit tests for this too! I've submitted #24657. In that one I used a setter for userAriaDescribedBy to call stateChanges.next(). Let me know if you'd prefer that here too.
| fixture.componentInstance.hint = 'test'; | ||
| fixture.detectChanges(); | ||
| const hint = fixture.debugElement.query(By.css('mat-hint')).nativeElement; | ||
| expect(select.getAttribute('aria-describedby')).toBe(hint.getAttribute('id')); |
There was a problem hiding this comment.
I think that you can drop this assertion since it's covered by the one below.
There was a problem hiding this comment.
This asserts that it's exactly the same as the hint's ID, whereas the next assertion asserts only that it matches a certain format: expect(select.getAttribute('aria-describedby')).toMatch(/^mat-mdc-hint-\d+$/);
crisbeto
added
P3
zarend
added
target: minor
|
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. |
Based on #24292.
Further fixes #16209.