From 80de947d08f9c7c34db625187113d0a3d9753245 Mon Sep 17 00:00:00 2001 From: Elizabeth Mitchell Date: Wed, 30 Aug 2023 03:22:45 -0700 Subject: [PATCH] chore(tabs): minor code/visibility refactors Changes: - Use ElementInternals for aria roles outside of Firefox - Don't export Tabs interface used by tab - Get parent tabs element with closest() rather than - Remove md-tabs delegate focus (not used) - Remove selectedAttribute property (always 'selected') - Made focusedItem private PiperOrigin-RevId: 561287221 --- tabs/internal/tab.ts | 14 +++++--------- tabs/internal/tabs.ts | 26 ++++++++++++++------------ 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/tabs/internal/tab.ts b/tabs/internal/tab.ts index 3ba318880..ef5404d67 100644 --- a/tabs/internal/tab.ts +++ b/tabs/internal/tab.ts @@ -17,10 +17,7 @@ import {requestUpdateOnAriaChange} from '../../internal/aria/delegate.js'; import {dispatchActivationClick, isActivationClick} from '../../internal/controller/events.js'; import {EASING} from '../../internal/motion/animation.js'; -/** - * An element that can select items. - */ -export interface Tabs extends HTMLElement { +interface Tabs extends HTMLElement { selected?: number; selectedItem?: Tab; previousSelectedItem?: Tab; @@ -132,10 +129,6 @@ export class Tab extends LitElement { dispatchActivationClick(this.button); }; - private get tabs() { - return this.parentElement as Tabs; - } - private animateSelected() { this.indicator.getAnimations().forEach(a => { a.cancel(); @@ -152,9 +145,12 @@ export class Tab extends LitElement { if (!this.selected) { return reduceMotion ? [{'opacity': 1}, {'transform': 'none'}] : null; } + + // TODO(b/298105040): avoid hardcoding selector + const tabs = this.closest('md-tabs'); const from: Keyframe = {}; const fromRect = - (this.tabs?.previousSelectedItem?.indicator.getBoundingClientRect() ?? + (tabs?.previousSelectedItem?.indicator.getBoundingClientRect() ?? ({} as DOMRect)); const fromPos = fromRect.left; const fromExtent = fromRect.width; diff --git a/tabs/internal/tabs.ts b/tabs/internal/tabs.ts index 7ec0b03cd..47e56e040 100644 --- a/tabs/internal/tabs.ts +++ b/tabs/internal/tabs.ts @@ -41,12 +41,6 @@ const NAVIGATION_KEYS = new Map([ * */ export class Tabs extends LitElement { - /** @nocollapse */ - static override readonly shadowRootOptions = { - ...LitElement.shadowRootOptions, - delegatesFocus: true - }; - /** * Index of the selected item. */ @@ -71,8 +65,6 @@ export class Tabs extends LitElement { // be kept in sync @state() private itemsDirty = false; - private readonly selectedAttribute = `selected`; - /** * The item currently selected. */ @@ -90,10 +82,13 @@ export class Tabs extends LitElement { /** * The item currently focused. */ - protected get focusedItem() { + private get focusedItem() { return this.items.find((el: HTMLElement) => el.matches(':focus-within')); } + private readonly internals = + (this as HTMLElement /* needed for closure */).attachInternals(); + constructor() { super(); if (!isServer) { @@ -105,7 +100,14 @@ export class Tabs extends LitElement { override connectedCallback() { super.connectedCallback(); - this.setAttribute('role', 'tablist'); + // Firefox does not support ElementInternals aria yet, so we need to hydrate + // an attribute. + if (!('role' in this.internals)) { + this.setAttribute('role', 'tablist'); + return; + } + + this.internals.role = 'tablist'; } // focus item on keydown and optionally select it @@ -218,8 +220,8 @@ export class Tabs extends LitElement { } if (itemsChanged || changed.has('selected')) { if (this.previousSelectedItem !== this.selectedItem) { - this.previousSelectedItem?.removeAttribute(this.selectedAttribute); - this.selectedItem?.setAttribute(this.selectedAttribute, ''); + this.previousSelectedItem?.removeAttribute('selected'); + this.selectedItem?.setAttribute('selected', ''); } if (this.selectedItem !== this.focusedItem) { this.updateFocusableItem(this.selectedItem);