From 5d2f150688e04f8d483e30ff6e84c3c92eef7567 Mon Sep 17 00:00:00 2001 From: Piotr Bartkowiak Date: Thu, 22 Aug 2024 17:25:30 +0200 Subject: [PATCH 1/9] NgSelect double stop on mobile slternative fix --- .../ng-select-a11y.directive.ts | 33 +++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) 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..ccc9d159aee 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 @@ -11,10 +11,12 @@ import { HostListener, inject, Input, + Optional, Renderer2, } from '@angular/core'; import { FeatureConfigService, TranslationService } from '@spartacus/core'; -import { take } from 'rxjs'; +import { BREAKPOINT, BreakpointService } from '@spartacus/storefront'; +import { filter, take } from 'rxjs'; @Directive({ selector: '[cxNgSelectA11y]', @@ -41,6 +43,8 @@ export class NgSelectA11yDirective implements AfterViewInit { observer.observe(this.elementRef.nativeElement, { childList: true }); } + @Optional() breakpointService = inject(BreakpointService, { optional: true }); + constructor( private renderer: Renderer2, private elementRef: ElementRef @@ -67,7 +71,32 @@ export class NgSelectA11yDirective implements AfterViewInit { this.featureConfigService.isEnabled('a11yNgSelectMobileReadout') && inputElement.readOnly ) { - this.renderer.setAttribute(inputElement, 'aria-hidden', 'true'); + const observer = new MutationObserver((_changes, observer) => { + const comboboxAriaLabel = divCombobox.getAttribute('aria-label'); + const valueLabel = + this.elementRef.nativeElement.querySelector( + '.ng-value-label' + ).innerText; + 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(); + }); + + this.breakpointService + ?.isDown(BREAKPOINT.md) + .pipe(filter(Boolean), take(1)) + .subscribe(() => { + observer.observe(this.elementRef.nativeElement, { + subtree: true, + characterData: true, + }); + }); } } From 7274c8c71c2444170385f7b7882311031d0098cf Mon Sep 17 00:00:00 2001 From: Piotr Bartkowiak Date: Fri, 23 Aug 2024 09:27:51 +0200 Subject: [PATCH 2/9] circ dependency fix --- .../components/ng-select-a11y/ng-select-a11y.directive.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 ccc9d159aee..e7a51bd6ed7 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 @@ -15,8 +15,8 @@ import { Renderer2, } from '@angular/core'; import { FeatureConfigService, TranslationService } from '@spartacus/core'; -import { BREAKPOINT, BreakpointService } from '@spartacus/storefront'; import { filter, take } from 'rxjs'; +import { BREAKPOINT, BreakpointService } from '../../../layout'; @Directive({ selector: '[cxNgSelectA11y]', From 1256fd898ec2d69aab9db846ea3e82594dad8e82 Mon Sep 17 00:00:00 2001 From: Piotr Bartkowiak Date: Fri, 23 Aug 2024 18:53:12 +0200 Subject: [PATCH 3/9] removed old test + refactored --- .../ng-select-a11y.directive.spec.ts | 2 - .../ng-select-a11y.directive.ts | 54 ++++++++++++------- 2 files changed, 36 insertions(+), 20 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..1701cee113e 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 @@ -65,12 +65,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) => { 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 e7a51bd6ed7..c0b4b0eed4c 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 @@ -45,6 +45,8 @@ export class NgSelectA11yDirective implements AfterViewInit { @Optional() breakpointService = inject(BreakpointService, { optional: true }); + ariaLabelAttribute = 'aria-label'; + constructor( private renderer: Renderer2, private elementRef: ElementRef @@ -60,7 +62,11 @@ export class NgSelectA11yDirective implements AfterViewInit { const ariaControls = this.cxNgSelectA11y.ariaControls ?? elementId; if (ariaLabel) { - this.renderer.setAttribute(divCombobox, 'aria-label', ariaLabel); + this.renderer.setAttribute( + divCombobox, + this.ariaLabelAttribute, + ariaLabel + ); } if (ariaControls) { @@ -71,28 +77,15 @@ export class NgSelectA11yDirective implements AfterViewInit { this.featureConfigService.isEnabled('a11yNgSelectMobileReadout') && inputElement.readOnly ) { - const observer = new MutationObserver((_changes, observer) => { - const comboboxAriaLabel = divCombobox.getAttribute('aria-label'); - const valueLabel = - this.elementRef.nativeElement.querySelector( - '.ng-value-label' - ).innerText; - 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(); + const selectObserver = new MutationObserver((changes, observer) => { + this.appendValueToAriaLabel(changes, observer, divCombobox); }); this.breakpointService ?.isDown(BREAKPOINT.md) .pipe(filter(Boolean), take(1)) .subscribe(() => { - observer.observe(this.elementRef.nativeElement, { + selectObserver.observe(this.elementRef.nativeElement, { subtree: true, characterData: true, }); @@ -114,11 +107,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, + this.ariaLabelAttribute, + ariaLabel + ); } ); }); } observerInstance.disconnect(); } + + appendValueToAriaLabel( + _changes: any, + observer: MutationObserver, + divCombobox: HTMLElement + ) { + const valueLabel = + this.elementRef.nativeElement.querySelector('.ng-value-label')?.innerText; + if (!valueLabel) return; + const comboboxAriaLabel = + divCombobox?.getAttribute(this.ariaLabelAttribute) || ''; + const valueElement = + this.elementRef.nativeElement.querySelector('.ng-value'); + this.renderer.setAttribute(valueElement, 'aria-hidden', 'true'); + this.renderer.setAttribute( + divCombobox, + this.ariaLabelAttribute, + comboboxAriaLabel + ', ' + valueLabel + ); + observer.disconnect(); + } } From cee76de28fd917a6265ab2701721b23c515636c8 Mon Sep 17 00:00:00 2001 From: Piotr Bartkowiak Date: Mon, 26 Aug 2024 12:11:01 +0200 Subject: [PATCH 4/9] fixed sonarcube complaint --- .../ng-select-a11y.directive.ts | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) 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 c0b4b0eed4c..570d0d4ceee 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 @@ -126,17 +126,18 @@ export class NgSelectA11yDirective implements AfterViewInit { ) { const valueLabel = this.elementRef.nativeElement.querySelector('.ng-value-label')?.innerText; - if (!valueLabel) return; - const comboboxAriaLabel = - divCombobox?.getAttribute(this.ariaLabelAttribute) || ''; - const valueElement = - this.elementRef.nativeElement.querySelector('.ng-value'); - this.renderer.setAttribute(valueElement, 'aria-hidden', 'true'); - this.renderer.setAttribute( - divCombobox, - this.ariaLabelAttribute, - comboboxAriaLabel + ', ' + valueLabel - ); + if (valueLabel) { + const comboboxAriaLabel = + divCombobox?.getAttribute(this.ariaLabelAttribute) || ''; + const valueElement = + this.elementRef.nativeElement.querySelector('.ng-value'); + this.renderer.setAttribute(valueElement, 'aria-hidden', 'true'); + this.renderer.setAttribute( + divCombobox, + this.ariaLabelAttribute, + comboboxAriaLabel + ', ' + valueLabel + ); + } observer.disconnect(); } } From a006b6be59835af1b10160841a8945a300e8e8bb Mon Sep 17 00:00:00 2001 From: "Bartkowiak, Piotr (External)" Date: Fri, 6 Sep 2024 15:00:55 +0200 Subject: [PATCH 5/9] Added JSDoc + unit test --- .../ng-select-a11y.directive.spec.ts | 34 ++++++++++++++++--- .../ng-select-a11y.directive.ts | 4 +++ 2 files changed, 34 insertions(+), 4 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 1701cee113e..7326e85acdb 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 @@ -6,22 +6,23 @@ import { FeatureConfigService, TranslationService } from '@spartacus/core'; import { of } from 'rxjs'; import { NgSelectA11yDirective } from './ng-select-a11y.directive'; import { NgSelectA11yModule } from './ng-select-a11y.module'; +import { BreakpointService } from '@spartacus/storefront'; @Component({ template: ` - {{ - val - }}
`, }) class MockComponent { isSearchable: boolean = false; + selected = 1; } class MockFeatureConfigService { @@ -39,6 +40,7 @@ class MockTranslationService { describe('NgSelectA11yDirective', () => { let component: MockComponent; let fixture: ComponentFixture; + let breakpointService: BreakpointService; beforeEach(() => { TestBed.configureTestingModule({ @@ -51,8 +53,8 @@ describe('NgSelectA11yDirective', () => { }).compileComponents(); fixture = TestBed.createComponent(MockComponent); - component = fixture.componentInstance; + breakpointService = TestBed.inject(BreakpointService); }); function getNgSelect(): DebugElement { @@ -89,4 +91,28 @@ 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) + ); + 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 570d0d4ceee..4c25b9a7a13 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 @@ -119,6 +119,10 @@ export class NgSelectA11yDirective implements AfterViewInit { 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, From 871761dfd4717d878a10defd0234f0b179c62f04 Mon Sep 17 00:00:00 2001 From: "Bartkowiak, Piotr (External)" Date: Tue, 10 Sep 2024 16:19:24 +0200 Subject: [PATCH 6/9] CR changes --- .../ng-select-a11y.directive.ts | 21 ++++++------------- 1 file changed, 6 insertions(+), 15 deletions(-) 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 4c25b9a7a13..4c5bf68c6ae 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 @@ -18,6 +18,8 @@ import { FeatureConfigService, TranslationService } from '@spartacus/core'; import { filter, take } from 'rxjs'; import { BREAKPOINT, BreakpointService } from '../../../layout'; +const ARIA_LABEL = 'aria-label'; + @Directive({ selector: '[cxNgSelectA11y]', }) @@ -45,8 +47,6 @@ export class NgSelectA11yDirective implements AfterViewInit { @Optional() breakpointService = inject(BreakpointService, { optional: true }); - ariaLabelAttribute = 'aria-label'; - constructor( private renderer: Renderer2, private elementRef: ElementRef @@ -62,11 +62,7 @@ export class NgSelectA11yDirective implements AfterViewInit { const ariaControls = this.cxNgSelectA11y.ariaControls ?? elementId; if (ariaLabel) { - this.renderer.setAttribute( - divCombobox, - this.ariaLabelAttribute, - ariaLabel - ); + this.renderer.setAttribute(divCombobox, ARIA_LABEL, ariaLabel); } if (ariaControls) { @@ -107,11 +103,7 @@ 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, - this.ariaLabelAttribute, - ariaLabel - ); + this.renderer.setAttribute(option, ARIA_LABEL, ariaLabel); } ); }); @@ -131,14 +123,13 @@ export class NgSelectA11yDirective implements AfterViewInit { const valueLabel = this.elementRef.nativeElement.querySelector('.ng-value-label')?.innerText; if (valueLabel) { - const comboboxAriaLabel = - divCombobox?.getAttribute(this.ariaLabelAttribute) || ''; + 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, - this.ariaLabelAttribute, + ARIA_LABEL, comboboxAriaLabel + ', ' + valueLabel ); } From 5ebcede2a3b853547d1d53bb3be8f1c2dc4e767d Mon Sep 17 00:00:00 2001 From: "Bartkowiak, Piotr (External)" Date: Fri, 20 Sep 2024 12:26:33 +0200 Subject: [PATCH 7/9] CR changes --- .../components/ng-select-a11y/ng-select-a11y.directive.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) 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 4c5bf68c6ae..2e75396d095 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 @@ -73,14 +73,13 @@ export class NgSelectA11yDirective implements AfterViewInit { this.featureConfigService.isEnabled('a11yNgSelectMobileReadout') && inputElement.readOnly ) { - const selectObserver = new MutationObserver((changes, observer) => { - this.appendValueToAriaLabel(changes, observer, divCombobox); - }); - 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, From 5a72223903ea0e59b378c249d6648448afcb771d Mon Sep 17 00:00:00 2001 From: "Bartkowiak, Piotr (External)" Date: Thu, 10 Oct 2024 12:22:02 +0200 Subject: [PATCH 8/9] stop execution serverside --- .../components/ng-select-a11y/ng-select-a11y.directive.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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 2e75396d095..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,14 +4,17 @@ * 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'; @@ -47,6 +50,8 @@ export class NgSelectA11yDirective implements AfterViewInit { @Optional() breakpointService = inject(BreakpointService, { optional: true }); + @Inject(PLATFORM_ID) protected platformId: Object; + constructor( private renderer: Renderer2, private elementRef: ElementRef @@ -71,7 +76,8 @@ export class NgSelectA11yDirective implements AfterViewInit { if ( this.featureConfigService.isEnabled('a11yNgSelectMobileReadout') && - inputElement.readOnly + inputElement.readOnly && + isPlatformBrowser(this.platformId) ) { this.breakpointService ?.isDown(BREAKPOINT.md) From 0d87475a347d6a6a020985dc951e41126a379358 Mon Sep 17 00:00:00 2001 From: "Bartkowiak, Piotr (External)" Date: Thu, 10 Oct 2024 13:51:37 +0200 Subject: [PATCH 9/9] uni test fix --- .../ng-select-a11y/ng-select-a11y.directive.spec.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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 7326e85acdb..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,10 +3,10 @@ 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'; -import { BreakpointService } from '@spartacus/storefront'; @Component({ template: ` @@ -41,6 +41,7 @@ describe('NgSelectA11yDirective', () => { let component: MockComponent; let fixture: ComponentFixture; let breakpointService: BreakpointService; + let directive: NgSelectA11yDirective; beforeEach(() => { TestBed.configureTestingModule({ @@ -55,6 +56,10 @@ describe('NgSelectA11yDirective', () => { 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 { @@ -96,6 +101,7 @@ describe('NgSelectA11yDirective', () => { const isDownSpy = spyOn(breakpointService, 'isDown').and.returnValue( of(true) ); + directive['platformId'] = 'browser'; fixture.detectChanges(); const ngSelectInstance = getNgSelect().componentInstance; ngSelectInstance.writeValue(component.selected);