Skip to content

Commit

Permalink
fix: Use proper keybaord navigation keys for facets (#19651)
Browse files Browse the repository at this point in the history
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
sdrozdsap and github-actions[bot] authored Nov 29, 2024
1 parent 8297d98 commit b32ecac
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ import {
import { Facet, FacetValue, FeatureConfigService } from '@spartacus/core';
import { Observable } from 'rxjs';
import { ICON_TYPE } from '../../../../../cms-components/misc/icon/icon.model';
import { FocusDirective } from '../../../../../layout/a11y/keyboard-focus/focus.directive';
import {
FocusDirective,
disableTabbingForTick,
} from '../../../../../layout/a11y';
import { FacetCollapseState } from '../facet.model';
import { FacetService } from '../services/facet.service';

Expand Down Expand Up @@ -153,7 +156,21 @@ export class FacetComponent implements AfterViewInit {
case 'ArrowUp':
this.onArrowUp(event, targetIndex);
break;
case 'Tab':
this.onTabNavigation();
break;
}
}

/**
* If a11yTabComponent is enabled, we temporarily disable tabbing for the facet values.
* This is to use proper keyboard navigation keys(ArrowUp/ArrowDown) for navigating through the facet values.
*/
protected onTabNavigation(): void {
if (!this.featureConfigService?.isEnabled('a11yTabComponent')) {
return;
}
disableTabbingForTick(this.values.map((el) => el.nativeElement));
}

/**
Expand Down
1 change: 1 addition & 0 deletions projects/storefrontlib/layout/a11y/keyboard-focus/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export { FocusConfig, TrapFocus, TrapFocusType } from './keyboard-focus.model';
export * from './keyboard-focus.module';
export * from './focus-testing.module';
export * from './services/index';
export * from './keyboard-focus.utils';

// export * from './autofocus/index';
// export * from './base/index';
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { disableTabbingForTick } from './keyboard-focus.utils';
import { fakeAsync, tick } from '@angular/core/testing';

describe('disableTabbingForTick', () => {
let elements: HTMLElement[];

beforeEach(() => {
elements = [document.createElement('div'), document.createElement('div')];
elements.forEach((el) => document.body.appendChild(el));
});

afterEach(() => {
elements.forEach((el) => document.body.removeChild(el));
});

it('should set tabIndex to -1 for each element', () => {
disableTabbingForTick(elements);
elements.forEach((el) => {
expect(el.tabIndex).toBe(-1);
});
});

it('should reset tabIndex to 0 after a tick', fakeAsync(() => {
disableTabbingForTick(elements);
tick(100);
elements.forEach((el) => {
expect(el.tabIndex).toBe(0);
});
}));
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* SPDX-FileCopyrightText: 2024 SAP Spartacus team <[email protected]>
*
* SPDX-License-Identifier: Apache-2.0
*/

/**
* Temporarily removes elements from the tabbing flow and restores them after a tick.
*
* This method sets the `tabIndex` of each element in the provided iterable to `-1`
* and resets it back to `0` using `requestAnimationFrame`. While using `requestAnimationFrame`
* may seem like a bad code smell, it is justified here as it ensures a natural tabbing flow
* in cases where determining the next focusable element is complex, such as when directives
* like `TrapFocusDirective` modify the DOM's focus behavior.
*
* This utility is especially useful for scenarios like menus, lists, or carousels where
* `Tab` navigation is intentionally disabled, but other keyboard keys (e.g., `Arrow` keys)
* are used for navigation. It helps prevent these elements from disrupting the tab order
* while allowing other key-based interactions.
*
* @param elements - An iterable of `HTMLElement` objects to temporarily remove from tab navigation.
*/
export const disableTabbingForTick = (elements: Iterable<HTMLElement>) => {
for (const element of elements) {
element.tabIndex = -1;
}
requestAnimationFrame(() => {
for (const element of elements) {
element.tabIndex = 0;
}
});
};
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { BehaviorSubject, Observable } from 'rxjs';
import { tap } from 'rxjs/operators';
import { ICON_TYPE } from '../../../cms-components/misc/icon/icon.model';
import { CarouselService } from './carousel.service';
import { disableTabbingForTick } from '../../../layout/a11y';

/**
* Generic carousel component that can be used to render any carousel items,
Expand Down Expand Up @@ -128,13 +129,8 @@ export class CarouselComponent implements OnInit, OnChanges {
}

/**
* Handles "Tab" navigation within the carousel.
*
* Temporarily removes all `cxFocusableCarouselItem` elements from the tab flow
* and restores them after a short delay. While using `requestAnimationFrame` may seem like
* a bad code smell, it is justified here as it ensures natural tabbing flow in
* cases where determining the next focusable element is complex(e.g. if `TrapFocusDirective` is used).
*
* Handles Tab key on carousel items. If the carousel items have `ArrowRight`/`ArrowLeft`
* navigation enabled, it temporarily disables tab navigation for these items.
* The `cxFocusableCarouselItem` selector is used because it identifies carousel
* items that have `ArrowRight`/`ArrowLeft` navigation enabled. These items should not
* use tab navigation according to a11y requirements.
Expand All @@ -146,14 +142,7 @@ export class CarouselComponent implements OnInit, OnChanges {
if (!carouselElements.length) {
return;
}
carouselElements.forEach((element) => {
element.tabIndex = -1;
});
requestAnimationFrame(() => {
carouselElements.forEach((element) => {
element.tabIndex = 0;
});
});
disableTabbingForTick(carouselElements);
}

/**
Expand Down

0 comments on commit b32ecac

Please sign in to comment.