refactor: Keyboard focus ring controller

src/components/button-group/toggle-button.ts

@@ -30,14 +30,14 @@ export default class IgcToggleButtonComponent extends LitElement {
   };
 
   /* blazorSuppress */
-  public static register() {
+  public static register(): void {
     registerComponent(IgcToggleButtonComponent);
   }
 
-  private _kbFocus = addKeyboardFocusRing(this);
+  private readonly _focusRingManager = addKeyboardFocusRing(this);
 
   @query('[part="toggle"]', true)
-  private _nativeButton!: HTMLButtonElement;
+  private readonly _nativeButton!: HTMLButtonElement;
 
   /**
    * The value attribute of the control.
@@ -62,32 +62,33 @@ export default class IgcToggleButtonComponent extends LitElement {
 
   /* alternateName: focusComponent */
   /** Sets focus on the button. */
-  public override focus(options?: FocusOptions) {
+  public override focus(options?: FocusOptions): void {
     this._nativeButton.focus(options);
   }
 
   /* alternateName: blurComponent */
   /** Removes focus from the button. */
-  public override blur() {
+  public override blur(): void {
     this._nativeButton.blur();
   }
 
   /** Simulates a mouse click on the element. */
-  public override click() {
+  public override click(): void {
     this._nativeButton.click();
   }
 
   protected override render() {
     return html`
       <button
-        part=${partMap({ toggle: true, focused: this._kbFocus.focused })}
+        part=${partMap({
+          toggle: true,
+          focused: this._focusRingManager.focused,
+        })}
         type="button"
         ?disabled=${this.disabled}
         .ariaLabel=${this.ariaLabel}
         aria-pressed=${this.selected}
         aria-disabled=${this.disabled}
-        @click=${this._kbFocus.reset}
-        @blur=${this._kbFocus.reset}
       >
         <slot></slot>
       </button>

src/components/button/button-base.ts

@@ -28,13 +28,14 @@ export abstract class IgcButtonBaseComponent extends EventEmitterMixin<
     delegatesFocus: true,
   };
 
-  private _kbFocus = addKeyboardFocusRing(this);
+  protected readonly __internals: ElementInternals;
+
+  private readonly _focusRingManager = addKeyboardFocusRing(this);
 
-  protected __internals: ElementInternals;
   protected _disabled = false;
 
   @query('[part="base"]', true)
-  private _nativeButton!: HTMLButtonElement;
+  private readonly _nativeButton!: HTMLButtonElement;
 
   /* alternateName: displayType */
   /**
@@ -78,19 +79,19 @@ export abstract class IgcButtonBaseComponent extends EventEmitterMixin<
    * @attr [disabled=false]
    */
   @property({ type: Boolean, reflect: true })
-  public get disabled(): boolean {
-    return this._disabled;
-  }
-
   public set disabled(value: boolean) {
     this._disabled = value;
     this.toggleAttribute('disabled', Boolean(this._disabled));
   }
 
+  public get disabled(): boolean {
+    return this._disabled;
+  }
+
   /* blazorCSSuppress */
   /* alternateType: object */
   /** Returns the HTMLFormElement associated with this element. */
-  public get form() {
+  public get form(): HTMLFormElement | null {
     return this.__internals.form;
   }
 
@@ -101,51 +102,42 @@ export abstract class IgcButtonBaseComponent extends EventEmitterMixin<
 
   /* alternateName: focusComponent */
   /** Sets focus in the button. */
-  public override focus(options?: FocusOptions) {
+  public override focus(options?: FocusOptions): void {
     this._nativeButton.focus(options);
   }
 
   /** Simulates a mouse click on the element */
-  public override click() {
+  public override click(): void {
     this._nativeButton.click();
   }
 
   /* alternateName: blurComponent */
   /** Removes focus from the button. */
-  public override blur() {
+  public override blur(): void {
     this._nativeButton.blur();
   }
 
-  protected handleBlur() {
-    this._kbFocus.reset();
-  }
-
-  protected handleClick() {
-    this._kbFocus.reset();
-    switch (this.type) {
-      case 'submit':
-        return this.form?.requestSubmit();
-      case 'reset':
-        return this.form?.reset();
-      default:
-        return;
+  protected _handleClick(): void {
+    if (this.type === 'submit') {
+      this.form?.requestSubmit();
+    } else if (this.type === 'reset') {
+      this.form?.reset();
     }
   }
 
-  protected formDisabledCallback(state: boolean) {
+  protected formDisabledCallback(state: boolean): void {
     this._disabled = state;
     this.requestUpdate();
   }
 
   private renderButton() {
     return html`
       <button
-        part=${partMap({ base: true, focused: this._kbFocus.focused })}
+        part=${partMap({ base: true, focused: this._focusRingManager.focused })}
         aria-label=${ifDefined(this.ariaLabel ?? nothing)}
         ?disabled=${this.disabled}
         type=${ifDefined(this.type)}
-        @click=${this.handleClick}
-        @blur=${this.handleBlur}
+        @click=${this._handleClick}
       >
         ${this.renderContent()}
       </button>
@@ -155,15 +147,14 @@ export abstract class IgcButtonBaseComponent extends EventEmitterMixin<
   private renderLinkButton() {
     return html`
       <a
-        part=${partMap({ base: true, focused: this._kbFocus.focused })}
+        part=${partMap({ base: true, focused: this._focusRingManager.focused })}
         role="button"
         aria-label=${ifDefined(this.ariaLabel ?? nothing)}
         aria-disabled=${this.disabled}
         href=${ifDefined(this.href)}
         target=${ifDefined(this.target)}
         download=${ifDefined(this.download)}
         rel=${ifDefined(this.rel)}
-        @blur=${this.disabled ? nothing : this.handleBlur}
       >
         ${this.renderContent()}
       </a>

src/components/carousel/carousel-indicator-container.spec.ts

@@ -2,7 +2,12 @@ import { elementUpdated, expect, fixture, html } from '@open-wc/testing';
 
 import { tabKey } from '../common/controllers/key-bindings.js';
 import { defineComponents } from '../common/definitions/defineComponents.js';
-import { simulateClick, simulateKeyboard } from '../common/utils.spec.js';
+import { first } from '../common/util.js';
+import {
+  simulateClick,
+  simulateKeyboard,
+  simulatePointerDown,
+} from '../common/utils.spec.js';
 import IgcCarouselIndicatorContainerComponent from './carousel-indicator-container.js';
 import IgcCarouselIndicatorComponent from './carousel-indicator.js';
 
@@ -26,12 +31,8 @@ describe('Carousel Indicator Container', () => {
   let buttons: HTMLButtonElement[];
 
   beforeEach(async () => {
-    container = await fixture<IgcCarouselIndicatorContainerComponent>(
-      createIndicatorContainerComponent()
-    );
-    buttons = container.querySelectorAll(
-      'button'
-    ) as unknown as HTMLButtonElement[];
+    container = await fixture(createIndicatorContainerComponent());
+    buttons = Array.from(container.querySelectorAll('button'));
   });
 
   it('is correctly initialized', () => {
@@ -56,7 +57,7 @@ describe('Carousel Indicator Container', () => {
       </div>`
     );
 
-    simulateKeyboard(buttons[0], tabKey);
+    simulateKeyboard(first(buttons), tabKey);
     await elementUpdated(container);
 
     expect(container).shadowDom.to.equal(
@@ -67,7 +68,7 @@ describe('Carousel Indicator Container', () => {
   });
 
   it('should remove `focused` part on click', async () => {
-    simulateKeyboard(buttons[0], tabKey);
+    simulateKeyboard(first(buttons), tabKey);
     await elementUpdated(container);
 
     expect(container).shadowDom.to.equal(
@@ -76,7 +77,8 @@ describe('Carousel Indicator Container', () => {
       </div>`
     );
 
-    simulateClick(buttons[0]);
+    simulatePointerDown(first(buttons));
+    simulateClick(first(buttons));
     await elementUpdated(container);
 
     expect(container).shadowDom.to.equal(
@@ -87,7 +89,7 @@ describe('Carousel Indicator Container', () => {
   });
 
   it('it should remove `focused` part on focusout', async () => {
-    simulateKeyboard(buttons[0], tabKey);
+    simulateKeyboard(first(buttons), tabKey);
     await elementUpdated(container);
 
     expect(container).shadowDom.to.equal(
@@ -96,7 +98,7 @@ describe('Carousel Indicator Container', () => {
       </div>`
     );
 
-    buttons[0].dispatchEvent(new FocusEvent('focusout', { bubbles: true }));
+    first(buttons).dispatchEvent(new FocusEvent('focusout', { bubbles: true }));
     await elementUpdated(container);
 
     expect(container).shadowDom.to.equal(
@@ -107,7 +109,7 @@ describe('Carousel Indicator Container', () => {
   });
 
   it('it should not remove `focused` part on focusout if the target receiving focus is an `igc-carousel-indicator`', async () => {
-    simulateKeyboard(buttons[0], tabKey);
+    simulateKeyboard(first(buttons), tabKey);
     await elementUpdated(container);
 
     expect(container).shadowDom.to.equal(
@@ -116,14 +118,14 @@ describe('Carousel Indicator Container', () => {
       </div>`
     );
 
-    const indicator = await fixture<IgcCarouselIndicatorComponent>(
+    const indicator = await fixture(
       html`<igc-carousel-indicator>
         <span>0</span>
         <span slot="active">1</span>
       </igc-carousel-indicator>`
     );
 
-    buttons[0].dispatchEvent(
+    first(buttons).dispatchEvent(
       new FocusEvent('focusout', { bubbles: true, relatedTarget: indicator })
     );
     await elementUpdated(container);

src/components/carousel/carousel-indicator-container.ts

@@ -22,17 +22,19 @@ export default class IgcCarouselIndicatorContainerComponent extends LitElement {
   public static override styles = [styles, shared];
 
   /* blazorSuppress */
-  public static register() {
+  public static register(): void {
     registerComponent(IgcCarouselIndicatorContainerComponent);
   }
 
-  private _kbFocus = addKeyboardFocusRing(this);
+  private readonly _focusRingManager = addKeyboardFocusRing(this);
 
-  private handleFocusOut(event: FocusEvent) {
+  private _handleFocusOut(event: FocusEvent): void {
     const target = event.relatedTarget as Element;
 
-    if (!target?.matches(IgcCarouselIndicatorComponent.tagName)) {
-      this._kbFocus.reset();
+    if (target?.matches(IgcCarouselIndicatorComponent.tagName)) {
+      // Stop the event from hitting the _focusRingManager handler redrawing
+      // the keyboard focus styles
+      event.stopPropagation();
     }
   }
 
@@ -41,10 +43,9 @@ export default class IgcCarouselIndicatorContainerComponent extends LitElement {
       <div
         part=${partMap({
           base: true,
-          focused: this._kbFocus.focused,
+          focused: this._focusRingManager.focused,
         })}
-        @click=${this._kbFocus.reset}
-        @focusout=${this.handleFocusOut}
+        @focusout=${this._handleFocusOut}
       >
         <slot></slot>
       </div>