Skip to content

Commit b953aa0

Browse files
committed
refactor: Drop focus ring controller from carousel
1 parent 769ab41 commit b953aa0

File tree

2 files changed

+31
-8
lines changed

2 files changed

+31
-8
lines changed

src/components/carousel/carousel.ts

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import { styleMap } from 'lit/directives/style-map.js';
1212
import { themes } from '../../theming/theming-decorator.js';
1313
import IgcButtonComponent from '../button/button.js';
1414
import { carouselContext } from '../common/context.js';
15-
import { addKeyboardFocusRing } from '../common/controllers/focus-ring.js';
1615
import {
1716
type SwipeEvent,
1817
addGesturesController,
@@ -105,12 +104,12 @@ export default class IgcCarouselComponent extends EventEmitterMixin<
105104

106105
private static readonly increment = createCounter();
107106
private readonly _carouselId = `igc-carousel-${IgcCarouselComponent.increment()}`;
108-
private readonly _focusRingManager = addKeyboardFocusRing(this);
109107

110108
private readonly _internals: ElementInternals;
111109
private _lastInterval!: ReturnType<typeof setInterval> | null;
112110
private _hasKeyboardInteractionOnIndicators = false;
113111
private _hasMouseStop = false;
112+
private _hasInnerFocus = false;
114113

115114
private _context = new ContextProvider(this, {
116115
context: carouselContext,
@@ -328,6 +327,13 @@ export default class IgcCarouselComponent extends EventEmitterMixin<
328327

329328
this.addEventListener('pointerenter', this.handlePointerEnter);
330329
this.addEventListener('pointerleave', this.handlePointerLeave);
330+
this.addEventListener('pointerdown', () => {
331+
this._hasInnerFocus = false;
332+
});
333+
334+
this.addEventListener('keyup', () => {
335+
this._hasInnerFocus = true;
336+
});
331337

332338
addGesturesController(this, {
333339
ref: this._carouselSlidesContainerRef,
@@ -382,22 +388,22 @@ export default class IgcCarouselComponent extends EventEmitterMixin<
382388

383389
private handlePointerEnter(): void {
384390
this._hasMouseStop = true;
385-
if (this._focusRingManager.focused) {
391+
if (this._hasInnerFocus) {
386392
return;
387393
}
388394
this.handlePauseOnInteraction();
389395
}
390396

391397
private handlePointerLeave(): void {
392398
this._hasMouseStop = false;
393-
if (this._focusRingManager.focused) {
399+
if (this._hasInnerFocus) {
394400
return;
395401
}
396402
this.handlePauseOnInteraction();
397403
}
398404

399405
private handleFocusIn(): void {
400-
if (this._focusRingManager.focused || this._hasMouseStop) {
406+
if (this._hasInnerFocus || this._hasMouseStop) {
401407
return;
402408
}
403409
this.handlePauseOnInteraction();
@@ -410,8 +416,12 @@ export default class IgcCarouselComponent extends EventEmitterMixin<
410416
return;
411417
}
412418

413-
if (this._focusRingManager.focused && !this._hasMouseStop) {
414-
this.handlePauseOnInteraction();
419+
if (this._hasInnerFocus) {
420+
this._hasInnerFocus = false;
421+
422+
if (!this._hasMouseStop) {
423+
this.handlePauseOnInteraction();
424+
}
415425
}
416426
}
417427

src/components/common/controllers/focus-ring.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,16 @@ import type { ReactiveController, ReactiveControllerHost } from 'lit';
66
*
77
* By default the class attaches a keyup event handler on the component host and will update its keyboard focus
88
* state based on it.
9+
*
10+
* **Important Note:** This controller is designed for use with **atomic web components** that represent single,
11+
* interactive elements (e.g., buttons, form fields, interactive icons). It helps these components correctly
12+
* display a focus indicator *only* when keyboard navigation is occurring, improving accessibility without
13+
* visual clutter during mouse or touch interactions.
14+
*
15+
* **Do not use this controller as a general-purpose shortcut for managing focus state in complex components or layouts.**
16+
* Misusing it in this way can lead to incorrect focus ring behavior, accessibility issues, and make your
17+
* application harder to maintain. For managing focus within larger, composite components, consider alternative
18+
* strategies like ARIA attributes, managing `tabindex`, or a bespoke implementation if needed.
919
*/
1020
class KeyboardFocusRingController implements ReactiveController {
1121
private static readonly _events = [
@@ -24,7 +34,7 @@ class KeyboardFocusRingController implements ReactiveController {
2434
return this._isKeyboardFocused;
2535
}
2636

27-
constructor(readonly host: ReactiveControllerHost & HTMLElement) {
37+
constructor(host: ReactiveControllerHost & HTMLElement) {
2838
this._host = host;
2939
host.addController(this);
3040
}
@@ -54,6 +64,9 @@ export type { KeyboardFocusRingController };
5464

5565
/**
5666
* Adds a {@link KeyboardFocusRingController} responsible for managing keyboard focus state.
67+
*
68+
* This utility function is intended for use with **atomic web components** that require
69+
* dynamic focus ring visibility based on keyboard interaction.
5770
*/
5871
export function addKeyboardFocusRing(
5972
host: ReactiveControllerHost & HTMLElement

0 commit comments

Comments
 (0)