Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: (CXSPA-1128) - NgSelect double stop on mobile alternative fix #19152

Merged
merged 25 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
5d2f150
NgSelect double stop on mobile slternative fix
Pio-Bar Aug 22, 2024
7274c8c
circ dependency fix
Pio-Bar Aug 23, 2024
1256fd8
removed old test + refactored
Pio-Bar Aug 23, 2024
cee76de
fixed sonarcube complaint
Pio-Bar Aug 26, 2024
a006b6b
Added JSDoc + unit test
Pio-Bar Sep 6, 2024
8f03930
Merge remote-tracking branch 'origin/develop' into bugfix/CXSPA-1128
Pio-Bar Sep 6, 2024
871761d
CR changes
Pio-Bar Sep 10, 2024
f884ca2
Merge remote-tracking branch 'origin/develop' into bugfix/CXSPA-1128
Pio-Bar Sep 10, 2024
5ebcede
CR changes
Pio-Bar Sep 20, 2024
4ed00e2
Merge remote-tracking branch 'origin/develop' into bugfix/CXSPA-1128
Pio-Bar Sep 20, 2024
568f740
Merge remote-tracking branch 'origin/develop' into bugfix/CXSPA-1128
Pio-Bar Sep 20, 2024
4b4ee21
Merge branch 'develop' into bugfix/CXSPA-1128
Pio-Bar Sep 24, 2024
1e76ddf
Merge branch 'develop' into bugfix/CXSPA-1128
Pio-Bar Sep 24, 2024
dec5d55
Merge branch 'develop' into bugfix/CXSPA-1128
Pio-Bar Sep 24, 2024
d1e2f66
Merge branch 'develop' into bugfix/CXSPA-1128
Pio-Bar Sep 24, 2024
55c55e7
Merge branch 'develop' into bugfix/CXSPA-1128
Pio-Bar Sep 25, 2024
f50aff5
Merge branch 'develop' into bugfix/CXSPA-1128
Pio-Bar Sep 26, 2024
b20be28
Merge branch 'develop' into bugfix/CXSPA-1128
Pio-Bar Sep 30, 2024
0ee36ff
Merge branch 'develop' into bugfix/CXSPA-1128
Pio-Bar Oct 2, 2024
12dead4
Merge branch 'develop' into bugfix/CXSPA-1128
Zeyber Oct 9, 2024
5a72223
stop execution serverside
Pio-Bar Oct 10, 2024
abc2b8d
Merge branch 'develop' into bugfix/CXSPA-1128
Pio-Bar Oct 10, 2024
0d87475
uni test fix
Pio-Bar Oct 10, 2024
938dba3
Merge remote-tracking branch 'origin/develop' into bugfix/CXSPA-1128
Pio-Bar Oct 10, 2024
d214125
Merge branch 'develop' into bugfix/CXSPA-1128
Pio-Bar Oct 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -12,16 +13,16 @@ import { NgSelectA11yModule } from './ng-select-a11y.module';
<ng-select
[searchable]="isSearchable"
[cxNgSelectA11y]="{ ariaLabel: 'Size', ariaControls: 'size-results' }"
[items]="[1, 2, 3]"
[(ngModel)]="selected"
>
<ng-option *ngFor="let val of [1, 2, 3]" [value]="val">{{
val
}}</ng-option>
</ng-select>
<div id="size-results"></div>
`,
})
class MockComponent {
isSearchable: boolean = false;
selected = 1;
}

class MockFeatureConfigService {
Expand All @@ -39,6 +40,8 @@ class MockTranslationService {
describe('NgSelectA11yDirective', () => {
let component: MockComponent;
let fixture: ComponentFixture<MockComponent>;
let breakpointService: BreakpointService;
let directive: NgSelectA11yDirective;

beforeEach(() => {
TestBed.configureTestingModule({
Expand All @@ -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 {
Expand All @@ -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) => {
Expand All @@ -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();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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]',
Expand All @@ -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
Expand All @@ -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) {
Expand All @@ -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,
});
});
}
}

Expand All @@ -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(
Zeyber marked this conversation as resolved.
Show resolved Hide resolved
_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();
}
}
Loading