From ec946e913298491b43312c165626299a31f27eec Mon Sep 17 00:00:00 2001 From: PioBar <72926984+Pio-Bar@users.noreply.github.com> Date: Thu, 10 Oct 2024 14:44:57 +0200 Subject: [PATCH] fix: (CXSPA-1128) - NgSelect double stop on mobile alternative fix (#19152) Co-authored-by: Caine Rotherham --- .../ng-select-a11y.directive.spec.ts | 42 ++++++++++++-- .../ng-select-a11y.directive.ts | 58 +++++++++++++++++-- 2 files changed, 89 insertions(+), 11 deletions(-) diff --git a/projects/storefrontlib/shared/components/ng-select-a11y/ng-select-a11y.directive.spec.ts b/projects/storefrontlib/shared/components/ng-select-a11y/ng-select-a11y.directive.spec.ts index 5a883f00d2d..27ceae10eea 100644 --- a/projects/storefrontlib/shared/components/ng-select-a11y/ng-select-a11y.directive.spec.ts +++ b/projects/storefrontlib/shared/components/ng-select-a11y/ng-select-a11y.directive.spec.ts @@ -3,6 +3,7 @@ import { ComponentFixture, TestBed } from '@angular/core/testing'; import { By } from '@angular/platform-browser'; import { NgSelectModule } from '@ng-select/ng-select'; import { FeatureConfigService, TranslationService } from '@spartacus/core'; +import { BreakpointService } from '@spartacus/storefront'; import { of } from 'rxjs'; import { NgSelectA11yDirective } from './ng-select-a11y.directive'; import { NgSelectA11yModule } from './ng-select-a11y.module'; @@ -12,16 +13,16 @@ import { NgSelectA11yModule } from './ng-select-a11y.module'; - {{ - val - }}
`, }) class MockComponent { isSearchable: boolean = false; + selected = 1; } class MockFeatureConfigService { @@ -39,6 +40,8 @@ class MockTranslationService { describe('NgSelectA11yDirective', () => { let component: MockComponent; let fixture: ComponentFixture; + let breakpointService: BreakpointService; + let directive: NgSelectA11yDirective; beforeEach(() => { TestBed.configureTestingModule({ @@ -51,8 +54,12 @@ describe('NgSelectA11yDirective', () => { }).compileComponents(); fixture = TestBed.createComponent(MockComponent); - component = fixture.componentInstance; + breakpointService = TestBed.inject(BreakpointService); + const directiveEl = fixture.debugElement.query( + By.directive(NgSelectA11yDirective) + ); + directive = directiveEl.injector.get(NgSelectA11yDirective); }); function getNgSelect(): DebugElement { @@ -65,12 +72,10 @@ describe('NgSelectA11yDirective', () => { const select = getNgSelect().nativeElement; const innerDiv = select.querySelector("[role='combobox']"); - const inputElement = select.querySelector('input'); expect(innerDiv).toBeTruthy(); expect(innerDiv.getAttribute('aria-controls')).toEqual('size-results'); expect(innerDiv.getAttribute('aria-label')).toEqual('Size'); - expect(inputElement.getAttribute('aria-hidden')).toEqual('true'); }); it('should append aria-label to options', (done) => { @@ -91,4 +96,29 @@ describe('NgSelectA11yDirective', () => { done(); }); }); + + it('should append value to aria-label and hide the value element from screen reader on mobile', (done) => { + const isDownSpy = spyOn(breakpointService, 'isDown').and.returnValue( + of(true) + ); + directive['platformId'] = 'browser'; + fixture.detectChanges(); + const ngSelectInstance = getNgSelect().componentInstance; + ngSelectInstance.writeValue(component.selected); + ngSelectInstance.detectChanges(); + + // Wait for the mutation observer to update the aria-label + setTimeout(() => { + const select = getNgSelect().nativeElement; + const valueElement = select.querySelector('.ng-value'); + const divCombobox = select.querySelector("[role='combobox']"); + + expect(valueElement.getAttribute('aria-hidden')).toEqual('true'); + expect(divCombobox.getAttribute('aria-label')).toContain( + `, ${component.selected}` + ); + isDownSpy.and.callThrough(); + done(); + }); + }); }); diff --git a/projects/storefrontlib/shared/components/ng-select-a11y/ng-select-a11y.directive.ts b/projects/storefrontlib/shared/components/ng-select-a11y/ng-select-a11y.directive.ts index 8b63f49f98a..97410186f5c 100644 --- a/projects/storefrontlib/shared/components/ng-select-a11y/ng-select-a11y.directive.ts +++ b/projects/storefrontlib/shared/components/ng-select-a11y/ng-select-a11y.directive.ts @@ -4,17 +4,24 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { isPlatformBrowser } from '@angular/common'; import { AfterViewInit, Directive, ElementRef, HostListener, + Inject, inject, Input, + Optional, + PLATFORM_ID, Renderer2, } from '@angular/core'; import { FeatureConfigService, TranslationService } from '@spartacus/core'; -import { take } from 'rxjs'; +import { filter, take } from 'rxjs'; +import { BREAKPOINT, BreakpointService } from '../../../layout'; + +const ARIA_LABEL = 'aria-label'; @Directive({ selector: '[cxNgSelectA11y]', @@ -41,6 +48,10 @@ export class NgSelectA11yDirective implements AfterViewInit { observer.observe(this.elementRef.nativeElement, { childList: true }); } + @Optional() breakpointService = inject(BreakpointService, { optional: true }); + + @Inject(PLATFORM_ID) protected platformId: Object; + constructor( private renderer: Renderer2, private elementRef: ElementRef @@ -56,7 +67,7 @@ export class NgSelectA11yDirective implements AfterViewInit { const ariaControls = this.cxNgSelectA11y.ariaControls ?? elementId; if (ariaLabel) { - this.renderer.setAttribute(divCombobox, 'aria-label', ariaLabel); + this.renderer.setAttribute(divCombobox, ARIA_LABEL, ariaLabel); } if (ariaControls) { @@ -65,9 +76,21 @@ export class NgSelectA11yDirective implements AfterViewInit { if ( this.featureConfigService.isEnabled('a11yNgSelectMobileReadout') && - inputElement.readOnly + inputElement.readOnly && + isPlatformBrowser(this.platformId) ) { - this.renderer.setAttribute(inputElement, 'aria-hidden', 'true'); + this.breakpointService + ?.isDown(BREAKPOINT.md) + .pipe(filter(Boolean), take(1)) + .subscribe(() => { + const selectObserver = new MutationObserver((changes, observer) => { + this.appendValueToAriaLabel(changes, observer, divCombobox); + }); + selectObserver.observe(this.elementRef.nativeElement, { + subtree: true, + characterData: true, + }); + }); } } @@ -85,11 +108,36 @@ export class NgSelectA11yDirective implements AfterViewInit { options.forEach( (option: HTMLOptionElement, index: string | number) => { const ariaLabel = `${option.innerText}, ${+index + 1} ${translation} ${options.length}`; - this.renderer.setAttribute(option, 'aria-label', ariaLabel); + this.renderer.setAttribute(option, ARIA_LABEL, ariaLabel); } ); }); } observerInstance.disconnect(); } + + /** + * Hides the input value from the screen reader and provides it as part of the aria-label instead. + * This improves the screen reader output on mobile devices. + */ + appendValueToAriaLabel( + _changes: any, + observer: MutationObserver, + divCombobox: HTMLElement + ) { + const valueLabel = + this.elementRef.nativeElement.querySelector('.ng-value-label')?.innerText; + if (valueLabel) { + const comboboxAriaLabel = divCombobox?.getAttribute(ARIA_LABEL) || ''; + const valueElement = + this.elementRef.nativeElement.querySelector('.ng-value'); + this.renderer.setAttribute(valueElement, 'aria-hidden', 'true'); + this.renderer.setAttribute( + divCombobox, + ARIA_LABEL, + comboboxAriaLabel + ', ' + valueLabel + ); + } + observer.disconnect(); + } }