Skip to content
This repository was archived by the owner on Jan 13, 2025. It is now read-only.

Commit 901c83e

Browse files
bbrittocopybara-github
authored andcommitted
feat(select): Select foundation preserves describedby elements on validate
PiperOrigin-RevId: 512656605
1 parent 311ab4d commit 901c83e

File tree

2 files changed

+7
-78
lines changed

2 files changed

+7
-78
lines changed

packages/mdc-select/foundation.ts

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,6 @@ export class MDCSelectFoundation extends MDCFoundation<MDCSelectAdapter> {
9393

9494
private readonly leadingIcon: MDCSelectIconFoundation|undefined;
9595
private readonly helperText: MDCSelectHelperTextFoundation|undefined;
96-
private readonly ariaDescribedbyIds: string[];
9796

9897
// Disabled state
9998
private disabled = false;
@@ -122,11 +121,6 @@ export class MDCSelectFoundation extends MDCFoundation<MDCSelectAdapter> {
122121

123122
this.leadingIcon = foundationMap.leadingIcon;
124123
this.helperText = foundationMap.helperText;
125-
this.ariaDescribedbyIds =
126-
this.adapter.getSelectAnchorAttr(strings.ARIA_DESCRIBEDBY)
127-
?.trim()
128-
?.split(' ') ||
129-
[];
130124
}
131125

132126
/** Returns the index of the currently selected menu item, or -1 if none. */
@@ -486,20 +480,11 @@ export class MDCSelectFoundation extends MDCFoundation<MDCSelectAdapter> {
486480
const helperTextId = this.helperText.getId();
487481

488482
if (helperTextVisible && helperTextId) {
489-
this.adapter.setSelectAnchorAttr(
490-
strings.ARIA_DESCRIBEDBY, this.ariaDescribedbyIds.join(' '));
483+
this.adapter.setSelectAnchorAttr(strings.ARIA_DESCRIBEDBY, helperTextId);
491484
} else {
492-
// Remove helptext from list of describedby ids. Needed because
493-
// screenreaders will read labels pointed to by `aria-describedby` even if
494-
// they are `aria-hidden`.
495-
if (this.ariaDescribedbyIds.length > 1) {
496-
this.adapter.setSelectAnchorAttr(
497-
strings.ARIA_DESCRIBEDBY,
498-
this.ariaDescribedbyIds.filter(id => id !== helperTextId)
499-
.join(' '));
500-
} else { // helper text is the only describedby element
501-
this.adapter.removeSelectAnchorAttr(strings.ARIA_DESCRIBEDBY);
502-
}
485+
// Needed because screenreaders will read labels pointed to by
486+
// `aria-describedby` even if they are `aria-hidden`.
487+
this.adapter.removeSelectAnchorAttr(strings.ARIA_DESCRIBEDBY);
503488
}
504489
}
505490

packages/mdc-select/test/foundation.test.ts

Lines changed: 3 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,7 @@ describe('MDCSelectFoundation', () => {
8383
]);
8484
});
8585

86-
function setupTest(
87-
hasLeadingIcon = true, hasHelperText = false, describedbyElements = '') {
86+
function setupTest(hasLeadingIcon = true, hasHelperText = false) {
8887
const mockAdapter = createMockAdapter(MDCSelectFoundation);
8988
const leadingIcon = jasmine.createSpyObj('leadingIcon', [
9089
'setDisabled', 'setAriaLabel', 'setContent', 'registerInteractionHandler',
@@ -108,8 +107,6 @@ describe('MDCSelectFoundation', () => {
108107
mockAdapter.getMenuItemTextAtIndex.withArgs(0).and.returnValue('foo');
109108
mockAdapter.getMenuItemTextAtIndex.withArgs(1).and.returnValue('bar');
110109
mockAdapter.getMenuItemCount.and.returnValue(2);
111-
mockAdapter.getSelectAnchorAttr.withArgs('aria-describedby')
112-
.and.returnValue(describedbyElements);
113110

114111
const foundation = new MDCSelectFoundation(mockAdapter, foundationMap);
115112
return {foundation, mockAdapter, leadingIcon, helperText};
@@ -771,10 +768,10 @@ describe('MDCSelectFoundation', () => {
771768
() => {
772769
const hasIcon = false;
773770
const hasHelperText = true;
774-
const mockId = 'foobarbazcool';
775771
const {foundation, mockAdapter, helperText} =
776-
setupTest(hasIcon, hasHelperText, mockId);
772+
setupTest(hasIcon, hasHelperText);
777773

774+
const mockId = 'foobarbazcool';
778775
helperText.getId.and.returnValue(mockId);
779776
helperText.isVisible.and.returnValue(true);
780777

@@ -783,59 +780,6 @@ describe('MDCSelectFoundation', () => {
783780
.toHaveBeenCalledWith(strings.ARIA_DESCRIBEDBY, mockId);
784781
});
785782

786-
it('#setValid, with client ids, sets aria-describedby', () => {
787-
const hasIcon = false;
788-
const hasHelperText = true;
789-
const mockId = 'foobarbazcool';
790-
const clientDescribedbyIds = 'id1 id2 id3';
791-
792-
const {foundation, mockAdapter, helperText} =
793-
setupTest(hasIcon, hasHelperText, clientDescribedbyIds + ' ' + mockId);
794-
795-
helperText.getId.and.returnValue(mockId);
796-
helperText.isVisible.and.returnValue(true);
797-
798-
foundation.setValid(false);
799-
expect(mockAdapter.setSelectAnchorAttr)
800-
.toHaveBeenCalledWith(
801-
strings.ARIA_DESCRIBEDBY, clientDescribedbyIds + ' ' + mockId);
802-
});
803-
804-
it('#setValid, w/ client ids, remove helpertextId from aria-describedby',
805-
() => {
806-
const hasIcon = false;
807-
const hasHelperText = true;
808-
const mockId = 'foobarbazcool';
809-
const clientDescribedbyIds = `id1 id2 id3`;
810-
811-
const {foundation, mockAdapter, helperText} = setupTest(
812-
hasIcon, hasHelperText, clientDescribedbyIds + ' ' + mockId);
813-
814-
helperText.getId.and.returnValue(mockId);
815-
helperText.isVisible.and.returnValue(false);
816-
817-
foundation.setValid(false);
818-
expect(mockAdapter.setSelectAnchorAttr)
819-
.toHaveBeenCalledWith(
820-
strings.ARIA_DESCRIBEDBY, clientDescribedbyIds);
821-
});
822-
823-
it('#setValid, no client describedby ids, remove aria-describedby', () => {
824-
const hasIcon = false;
825-
const hasHelperText = true;
826-
const mockId = 'foobarbazcool';
827-
828-
const {foundation, mockAdapter, helperText} =
829-
setupTest(hasIcon, hasHelperText, mockId);
830-
831-
helperText.getId.and.returnValue(mockId);
832-
helperText.isVisible.and.returnValue(false);
833-
834-
foundation.setValid(false);
835-
expect(mockAdapter.removeSelectAnchorAttr)
836-
.toHaveBeenCalledWith(strings.ARIA_DESCRIBEDBY);
837-
});
838-
839783
it('#setValid true sets aria-invalid to false and removes invalid classes',
840784
() => {
841785
const {foundation, mockAdapter} = setupTest();

0 commit comments

Comments
 (0)