mixins: add DelegatesFocusMixin

clshortfuse · Jul 22, 2023 · 2839945 · 2839945
1 parent 423c942
commit 2839945
Show file tree

Hide file tree

Showing 11 changed files with 71 additions and 59 deletions.
diff --git a/components/Button.js b/components/Button.js
@@ -26,7 +26,6 @@ export default CustomElement
   })
   .set({
     stateLayer: true,
-    delegatesFocus: true,
     _allowedTypes: ['button', 'submit', 'reset'],
   })
   .observe({

diff --git a/components/Card.js b/components/Card.js
@@ -1,6 +1,7 @@
 import CustomElement from '../core/CustomElement.js';
 import { EVENT_HANDLER_TYPE } from '../core/customTypes.js';
 import AriaReflectorMixin from '../mixins/AriaReflectorMixin.js';
+import DelegatesFocusMixin from '../mixins/DelegatesFocusMixin.js';
 import FlexableMixin from '../mixins/FlexableMixin.js';
 import FormAssociatedMixin from '../mixins/FormAssociatedMixin.js';
 import ShapeMixin from '../mixins/ShapeMixin.js';
@@ -19,9 +20,7 @@ export default CustomElement
   .mixin(FormAssociatedMixin) // Tap into FormAssociated for disabledState
   .mixin(StateMixin)
   .mixin(AriaReflectorMixin)
-  .setStatic({
-    delegatesFocus: true,
-  })
+  .mixin(DelegatesFocusMixin)
   .set({
     _ariaRole: 'figure',
   })

diff --git a/components/ListItem.js b/components/ListItem.js
@@ -21,7 +21,6 @@ export default CustomElement
   .mixin(HyperlinkMixin)
   .set({
     _ariaRole: 'listitem',
-    delegatesFocus: false,
     stateLayer: true,
   })
   .observe({

diff --git a/components/ListOption.js b/components/ListOption.js
@@ -1,17 +1,19 @@
 // https://www.w3.org/WAI/ARIA/apg/patterns/listbox/
 
+import DelegatesFocusMixin from '../mixins/DelegatesFocusMixin.js';
+
 import ListItem from './ListItem.js';
 
 // https://html.spec.whatwg.org/multipage/form-elements.html#htmloptionelement
 
 export default ListItem
   .extend()
+  .mixin(DelegatesFocusMixin)
   .setStatic({
     formAssociated: true,
   })
   .set({
     _ariaRole: 'none',
-    delegatesFocus: true,
     _index: -1,
     _selectedDirty: false,
     isInteractive: true,

diff --git a/components/Listbox.js b/components/Listbox.js
@@ -1,4 +1,5 @@
 import { constructHTMLOptionsCollectionProxy } from '../dom/HTMLOptionsCollectionProxy.js';
+import DelegatesFocusMixin from '../mixins/DelegatesFocusMixin.js';
 import FormAssociatedMixin from '../mixins/FormAssociatedMixin.js';
 import KeyboardNavMixin from '../mixins/KeyboardNavMixin.js';
 import StateMixin from '../mixins/StateMixin.js';
@@ -12,13 +13,13 @@ export default List
   .mixin(StateMixin)
   .mixin(FormAssociatedMixin)
   .mixin(KeyboardNavMixin)
+  .mixin(DelegatesFocusMixin)
   .observe({
     multiple: 'boolean',
     size: { type: 'integer', empty: 0 },
   })
   .set({
     _ariaRole: 'listbox',
-    delegatesFocus: true,
     /** @type {HTMLCollectionOf<ListOption> & HTMLOptionsCollection} */
     _optionsCollection: null,
     /** @type {HTMLCollectionOf<ListOption>} */

diff --git a/components/NavItem.js b/components/NavItem.js
@@ -3,6 +3,7 @@ import './Ripple.js';
 import './Badge.js';
 
 import CustomElement from '../core/CustomElement.js';
+import DelegatesFocusMixin from '../mixins/DelegatesFocusMixin.js';
 import HyperlinkMixin from '../mixins/HyperlinkMixin.js';
 import RippleMixin from '../mixins/RippleMixin.js';
 import ShapeMixin from '../mixins/ShapeMixin.js';
@@ -18,8 +19,8 @@ export default CustomElement
   .mixin(RippleMixin)
   .mixin(ShapeMixin)
   .mixin(HyperlinkMixin)
+  .mixin(DelegatesFocusMixin)
   .set({
-    delegatesFocus: true,
     stateLayer: true,
   })
   .observe({

diff --git a/components/Tab.js b/components/Tab.js
@@ -4,6 +4,7 @@ import './Icon.js';
 
 import CustomElement from '../core/CustomElement.js';
 import { CHROME_VERSION } from '../core/dom.js';
+import DelegatesFocusMixin from '../mixins/DelegatesFocusMixin.js';
 import HyperlinkMixin from '../mixins/HyperlinkMixin.js';
 import RippleMixin from '../mixins/RippleMixin.js';
 import ScrollListenerMixin from '../mixins/ScrollListenerMixin.js';
@@ -17,15 +18,11 @@ export default CustomElement
   .mixin(RippleMixin)
   .mixin(ScrollListenerMixin)
   .mixin(HyperlinkMixin)
+  .mixin(DelegatesFocusMixin)
   .define({
     stateTargetElement() { return this.refs.anchor; },
-    /**
-     * Used to compute primary indicator size.
-     * Default to 24.
-     */
   })
   .set({
-    delegatesFocus: true,
     stateLayer: true,
   })
   .observe({

diff --git a/core/CustomElement.js b/core/CustomElement.js
@@ -3,7 +3,7 @@
 import Composition from './Composition.js';
 import { ICustomElement } from './ICustomElement.js';
 import { css } from './css.js';
-import { CHROME_VERSION, attrNameFromPropName, attrValueFromDataValue, isFocused } from './dom.js';
+import { attrNameFromPropName, attrValueFromDataValue } from './dom.js';
 import { applyMergePatch } from './jsonMergePatch.js';
 import { defineObservableProperty } from './observe.js';
 import { addInlineFunction, html } from './template.js';
@@ -844,49 +844,7 @@ export default class CustomElement extends ICustomElement {
     return this._propAttributeCache;
   }
 
-  get tabIndex() {
-    return super.tabIndex;
-  }
-
-  set tabIndex(value) {
-    if (CHROME_VERSION < 111) {
-      if (value === super.tabIndex && value !== -1) {
-      // Non -1 value already set
-        return;
-      }
-
-      if (this.delegatesFocus && isFocused(this)) {
-        if (this.getAttribute('tabindex') === value.toString()) {
-          // Skip if possible
-          return;
-        }
-
-        // Chrome blurs on tabindex changes with delegatesFocus
-        // https://bugs.chromium.org/p/chromium/issues/detail?id=1346606
-        /** @type {EventListener} */
-        const listener = (e) => {
-          e.stopImmediatePropagation();
-          e.stopPropagation();
-          if (e.type === 'blur') {
-            console.warn('Chromium bug 1346606: Tabindex change caused blur. Giving focusing back.', this);
-            this.focus();
-          } else {
-            console.warn(`Chromium bug 1346606: Blocking ${e.type} event.`, this);
-          }
-        };
-        for (const type of ['blur', 'focus', 'focusout', 'focusin']) {
-          this.addEventListener(type, listener, { capture: true, once: true });
-        }
-        super.tabIndex = value;
-        for (const type of ['blur', 'focus', 'focusout', 'focusin']) {
-          this.removeEventListener(type, listener, { capture: true });
-        }
-        return;
-      }
-    }
-
-    super.tabIndex = value;
-  }
+
 
   get static() { return /** @type {typeof CustomElement} */ (/** @type {unknown} */ (this.constructor)); }
 

diff --git a/mixins/ControlMixin.js b/mixins/ControlMixin.js
@@ -2,6 +2,7 @@
 
 import { cloneAttributeCallback } from '../core/CustomElement.js';
 import { FIREFOX_VERSION, SAFARI_VERSION } from '../core/dom.js';
+import DelegatesFocusMixin from './DelegatesFocusMixin.js';
 
 import FormAssociatedMixin from './FormAssociatedMixin.js';
 
@@ -17,12 +18,12 @@ import FormAssociatedMixin from './FormAssociatedMixin.js';
 export default function ControlMixin(Base) {
   return Base
     .mixin(FormAssociatedMixin)
+    .mixin(DelegatesFocusMixin)
     .observe({
       ariaLabel: 'string',
       _slotInnerText: 'string',
     })
     .set({
-      delegatesFocus: true,
       focusableOnDisabled: false,
       controlTagName: 'input',
       controlVoidElement: true,

diff --git a/mixins/DelegatesFocusMixin.js b/mixins/DelegatesFocusMixin.js
@@ -0,0 +1,54 @@
+import { CHROME_VERSION, isFocused } from '../core/dom.js';
+
+/** @param {typeof import('../core/CustomElement.js').default} Base */
+export default function DelegatesFocusMixin(Base) {
+  return Base
+    .set({
+      delegatesFocus: true,
+    })
+    .extend(CHROME_VERSION >= 111
+      ? (BaseClass) => BaseClass
+      : (BaseClass) => class extends BaseClass {
+        get tabIndex() {
+          return super.tabIndex;
+        }
+
+        set tabIndex(value) {
+          if (value === super.tabIndex && value !== -1) {
+            // Non -1 value already set
+            return;
+          }
+
+          if (this.delegatesFocus && isFocused(this)) {
+            if (this.getAttribute('tabindex') === value.toString()) {
+              // Skip if possible
+              return;
+            }
+
+            // Chrome blurs on tabindex changes with delegatesFocus
+            // https://bugs.chromium.org/p/chromium/issues/detail?id=1346606
+            /** @type {EventListener} */
+            const listener = (e) => {
+              e.stopImmediatePropagation();
+              e.stopPropagation();
+              if (e.type === 'blur') {
+                console.warn('Chromium bug 1346606: Tabindex change caused blur. Giving focusing back.', this);
+                this.focus();
+              } else {
+                console.warn(`Chromium bug 1346606: Blocking ${e.type} event.`, this);
+              }
+            };
+            for (const type of ['blur', 'focus', 'focusout', 'focusin']) {
+              this.addEventListener(type, listener, { capture: true, once: true });
+            }
+            super.tabIndex = value;
+            for (const type of ['blur', 'focus', 'focusout', 'focusin']) {
+              this.removeEventListener(type, listener, { capture: true });
+            }
+            return;
+          }
+
+          super.tabIndex = value;
+        }
+      });
+}
diff --git a/mixins/PopupMixin.js b/mixins/PopupMixin.js
@@ -13,6 +13,7 @@
 import CustomElement from '../core/CustomElement.js';
 import { attemptFocus, isRtl } from '../core/dom.js';
 import { canAnchorPopup } from '../utils/popup.js';
+import DelegatesFocusMixin from './DelegatesFocusMixin.js';
 
 CustomElement
   .extend()
@@ -161,6 +162,7 @@ function onBeforeUnload(event) {
  */
 export default function PopupMixin(Base) {
   return Base
+    .mixin(DelegatesFocusMixin)
     .observe({
       open: 'boolean',
       modal: 'boolean',
@@ -177,7 +179,6 @@ export default function PopupMixin(Base) {
     })
     .set({
       returnValue: '',
-      delegatesFocus: true,
       _closing: false,
       _useScrim: false,
     })