From 6c7702599ed11e0f15d9ddc62134ca3fe9cb81a3 Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Tue, 21 Jan 2025 15:16:13 +0400 Subject: [PATCH 1/7] feat: make disabled buttons accessible with feature flag (#8513) --- packages/a11y-base/src/tabindex-mixin.js | 21 ++++++- packages/button/src/vaadin-button-base.js | 3 +- packages/button/src/vaadin-button-mixin.js | 4 ++ packages/button/src/vaadin-button.d.ts | 21 ++++++- packages/button/src/vaadin-button.js | 31 +++++++++ packages/button/src/vaadin-lit-button.js | 5 ++ packages/button/test/button.common.ts | 46 ++++++++++++++ .../button/theme/lumo/vaadin-button-styles.js | 4 +- .../theme/material/vaadin-button-styles.js | 8 +-- .../mixins/input-field-shared.js | 6 +- test/integration/component-tooltip.test.js | 63 ++++++++++++++++--- 11 files changed, 193 insertions(+), 19 deletions(-) diff --git a/packages/a11y-base/src/tabindex-mixin.js b/packages/a11y-base/src/tabindex-mixin.js index 5eb7baec59..77e661f2a2 100644 --- a/packages/a11y-base/src/tabindex-mixin.js +++ b/packages/a11y-base/src/tabindex-mixin.js @@ -52,6 +52,10 @@ export const TabindexMixin = (superclass) => _disabledChanged(disabled, oldDisabled) { super._disabledChanged(disabled, oldDisabled); + if (this.__shouldAllowFocusWhenDisabled()) { + return; + } + if (disabled) { if (this.tabindex !== undefined) { this._lastTabIndex = this.tabindex; @@ -70,6 +74,10 @@ export const TabindexMixin = (superclass) => * @protected */ _tabindexChanged(tabindex) { + if (this.__shouldAllowFocusWhenDisabled()) { + return; + } + if (this.disabled && tabindex !== -1) { this._lastTabIndex = tabindex; this.tabindex = -1; @@ -86,8 +94,19 @@ export const TabindexMixin = (superclass) => * @override */ focus() { - if (!this.disabled) { + if (!this.disabled || this.__shouldAllowFocusWhenDisabled()) { super.focus(); } } + + /** + * Returns whether the component should be focusable when disabled. + * Returns false by default. + * + * @private + * @return {boolean} + */ + __shouldAllowFocusWhenDisabled() { + return false; + } }; diff --git a/packages/button/src/vaadin-button-base.js b/packages/button/src/vaadin-button-base.js index c1ab603ee6..2b6b1de807 100644 --- a/packages/button/src/vaadin-button-base.js +++ b/packages/button/src/vaadin-button-base.js @@ -20,7 +20,8 @@ export const buttonStyles = css` } :host([disabled]) { - pointer-events: none; + pointer-events: var(--_vaadin-button-disabled-pointer-events, none); + cursor: not-allowed; } /* Aligns the button with form fields when placed on the same line. diff --git a/packages/button/src/vaadin-button-mixin.js b/packages/button/src/vaadin-button-mixin.js index 0acbe59531..f4ce0b9a3b 100644 --- a/packages/button/src/vaadin-button-mixin.js +++ b/packages/button/src/vaadin-button-mixin.js @@ -53,6 +53,10 @@ export const ButtonMixin = (superClass) => if (!this.hasAttribute('role')) { this.setAttribute('role', 'button'); } + + if (this.__shouldAllowFocusWhenDisabled()) { + this.style.setProperty('--_vaadin-button-disabled-pointer-events', 'auto'); + } } /** diff --git a/packages/button/src/vaadin-button.d.ts b/packages/button/src/vaadin-button.d.ts index 7dba442edd..09d1479308 100644 --- a/packages/button/src/vaadin-button.d.ts +++ b/packages/button/src/vaadin-button.d.ts @@ -36,7 +36,26 @@ import { ButtonMixin } from './vaadin-button-mixin.js'; * * See [Styling Components](https://vaadin.com/docs/latest/styling/styling-components) documentation. */ -declare class Button extends ButtonMixin(ElementMixin(ThemableMixin(ControllerMixin(HTMLElement)))) {} +declare class Button extends ButtonMixin(ElementMixin(ThemableMixin(ControllerMixin(HTMLElement)))) { + /** + * When set to true, prevents all user interactions with the button such as + * clicking or hovering, and removes the button from the tab order, which + * makes it unreachable via the keyboard navigation. + * + * While the default behavior effectively prevents accidental interactions, + * it has an accessibility drawback: screen readers skip disabled buttons + * entirely, and users can't see tooltips that might explain why the button + * is disabled. To address this, an experimental enhancement allows disabled + * buttons to receive focus and show tooltips, while still preventing other + * interactions. This feature can be enabled with the following feature flag: + * + * ``` + * // Set before any button is attached to the DOM. + * window.Vaadin.featureFlags.accessibleDisabledButtons = true + * ``` + */ + disabled: boolean; +} declare global { interface HTMLElementTagNameMap { diff --git a/packages/button/src/vaadin-button.js b/packages/button/src/vaadin-button.js index a8a628cf29..f13ed5edec 100644 --- a/packages/button/src/vaadin-button.js +++ b/packages/button/src/vaadin-button.js @@ -50,6 +50,32 @@ registerStyles('vaadin-button', buttonStyles, { moduleId: 'vaadin-button-styles' * @mixes ThemableMixin */ class Button extends ButtonMixin(ElementMixin(ThemableMixin(ControllerMixin(PolymerElement)))) { + static get properties() { + return { + /** + * When set to true, prevents all user interactions with the button such as + * clicking or hovering, and removes the button from the tab order, which + * makes it unreachable via the keyboard navigation. + * + * While the default behavior effectively prevents accidental interactions, + * it has an accessibility drawback: screen readers skip disabled buttons + * entirely, and users can't see tooltips that might explain why the button + * is disabled. To address this, an experimental enhancement allows disabled + * buttons to receive focus and show tooltips, while still preventing other + * interactions. This feature can be enabled with the following feature flag: + * + * ``` + * // Set before any button is attached to the DOM. + * window.Vaadin.featureFlags.accessibleDisabledButtons = true + * ``` + */ + disabled: { + type: Boolean, + value: false, + }, + }; + } + static get is() { return 'vaadin-button'; } @@ -65,6 +91,11 @@ class Button extends ButtonMixin(ElementMixin(ThemableMixin(ControllerMixin(Poly this._tooltipController = new TooltipController(this); this.addController(this._tooltipController); } + + /** @override */ + __shouldAllowFocusWhenDisabled() { + return window.Vaadin.featureFlags.accessibleDisabledButtons; + } } defineCustomElement(Button); diff --git a/packages/button/src/vaadin-lit-button.js b/packages/button/src/vaadin-lit-button.js index f074fa296e..a33eb79d66 100644 --- a/packages/button/src/vaadin-lit-button.js +++ b/packages/button/src/vaadin-lit-button.js @@ -42,6 +42,11 @@ class Button extends ButtonMixin(ElementMixin(ThemableMixin(PolylitMixin(LitElem this._tooltipController = new TooltipController(this); this.addController(this._tooltipController); } + + /** @override */ + __shouldAllowFocusWhenDisabled() { + return window.Vaadin.featureFlags.accessibleDisabledButtons; + } } defineCustomElement(Button); diff --git a/packages/button/test/button.common.ts b/packages/button/test/button.common.ts index b23a1295b4..e49775efa9 100644 --- a/packages/button/test/button.common.ts +++ b/packages/button/test/button.common.ts @@ -161,4 +161,50 @@ describe('vaadin-button', () => { }); }); }); + + describe('disabled and accessible', () => { + let lastGlobalFocusable: HTMLInputElement; + + before(() => { + window.Vaadin.featureFlags ??= {}; + window.Vaadin.featureFlags.accessibleDisabledButtons = true; + }); + + after(() => { + window.Vaadin.featureFlags!.accessibleDisabledButtons = false; + }); + + beforeEach(async () => { + [button, lastGlobalFocusable] = fixtureSync( + `
+ Press me + +
`, + ).children as unknown as [Button, HTMLInputElement]; + await nextRender(button); + }); + + afterEach(async () => { + await resetMouse(); + }); + + it('should allow programmatic focus when disabled', () => { + button.focus(); + expect(document.activeElement).to.equal(button); + }); + + it('should allow pointer focus when disabled', async () => { + const { x, y } = middleOfNode(button); + await sendMouse({ type: 'click', position: [Math.floor(x), Math.floor(y)] }); + expect(document.activeElement).to.equal(button); + }); + + it('should allow keyboard focus when disabled', async () => { + await sendKeys({ press: 'Tab' }); + expect(document.activeElement).to.equal(button); + + await sendKeys({ press: 'Tab' }); + expect(document.activeElement).to.equal(lastGlobalFocusable); + }); + }); }); diff --git a/packages/button/theme/lumo/vaadin-button-styles.js b/packages/button/theme/lumo/vaadin-button-styles.js index 26b5e5e6c8..cb7cc141de 100644 --- a/packages/button/theme/lumo/vaadin-button-styles.js +++ b/packages/button/theme/lumo/vaadin-button-styles.js @@ -75,7 +75,7 @@ const button = css` /* Hover */ @media (any-hover: hover) { - :host(:hover)::before { + :host(:not([disabled]):hover)::before { opacity: 0.02; } } @@ -159,7 +159,7 @@ const button = css` } @media (any-hover: hover) { - :host([theme~='primary']:hover)::before { + :host([theme~='primary']:not([disabled]):hover)::before { opacity: 0.05; } } diff --git a/packages/button/theme/material/vaadin-button-styles.js b/packages/button/theme/material/vaadin-button-styles.js index 6e4042e4f9..a461353885 100644 --- a/packages/button/theme/material/vaadin-button-styles.js +++ b/packages/button/theme/material/vaadin-button-styles.js @@ -60,7 +60,7 @@ const button = css` vertical-align: middle; } - :host(:hover)::before, + :host(:hover:not([disabled]))::before, :host([focus-ring])::before { opacity: 0.08; transition-duration: 0.2s; @@ -77,7 +77,7 @@ const button = css` transition: 0s; } - :host(:hover:not([active]))::after { + :host(:hover:not([active]):not([disabled]))::after { transform: translate(-50%, -50%) scale(1); opacity: 0; } @@ -106,7 +106,7 @@ const button = css` background-color: var(--material-secondary-background-color); } - :host([theme~='contained']:hover) { + :host([theme~='contained']:not([disabled]):hover) { box-shadow: var(--material-shadow-elevation-4dp); } @@ -149,7 +149,7 @@ const button = css` transform: translate(50%, -50%) scale(0.0000001); } - :host(:hover:not([active])[dir='rtl'])::after { + :host(:hover:not([active]):not([disabled])[dir='rtl'])::after { transform: translate(50%, -50%) scale(1); } diff --git a/packages/vaadin-lumo-styles/mixins/input-field-shared.js b/packages/vaadin-lumo-styles/mixins/input-field-shared.js index bb1351a8b3..ada64acf8c 100644 --- a/packages/vaadin-lumo-styles/mixins/input-field-shared.js +++ b/packages/vaadin-lumo-styles/mixins/input-field-shared.js @@ -47,17 +47,17 @@ const inputField = css` } /* Hover */ - :host(:hover:not([readonly]):not([focused])) [part='input-field']::after { + :host(:hover:not([readonly]):not([focused]):not([disabled])) [part='input-field']::after { opacity: var(--vaadin-input-field-hover-highlight-opacity, 0.1); } /* Touch device adjustment */ @media (pointer: coarse) { - :host(:hover:not([readonly]):not([focused])) [part='input-field']::after { + :host(:hover:not([readonly]):not([focused]):not([disabled])) [part='input-field']::after { opacity: 0; } - :host(:active:not([readonly]):not([focused])) [part='input-field']::after { + :host(:active:not([readonly]):not([focused]):not([disabled])) [part='input-field']::after { opacity: 0.2; } } diff --git a/test/integration/component-tooltip.test.js b/test/integration/component-tooltip.test.js index c9b6c0cfd5..562234c324 100644 --- a/test/integration/component-tooltip.test.js +++ b/test/integration/component-tooltip.test.js @@ -1,5 +1,7 @@ import { expect } from '@vaadin/chai-plugins'; -import { fixtureSync, nextRender, tabKeyDown } from '@vaadin/testing-helpers'; +import { fixtureSync, middleOfNode, nextRender, tabKeyDown } from '@vaadin/testing-helpers'; +import { resetMouse, sendKeys, sendMouse } from '@web/test-runner-commands'; +import './not-animated-styles.js'; import { Button } from '@vaadin/button'; import { Checkbox } from '@vaadin/checkbox'; import { CheckboxGroup } from '@vaadin/checkbox-group'; @@ -25,6 +27,12 @@ import { TimePicker } from '@vaadin/time-picker'; import { Tooltip } from '@vaadin/tooltip'; import { mouseenter, mouseleave } from '@vaadin/tooltip/test/helpers.js'; +before(() => { + Tooltip.setDefaultFocusDelay(0); + Tooltip.setDefaultHoverDelay(0); + Tooltip.setDefaultHideDelay(0); +}); + [ { tagName: Button.is }, { tagName: Checkbox.is, ariaTargetSelector: 'input' }, @@ -85,12 +93,6 @@ import { mouseenter, mouseleave } from '@vaadin/tooltip/test/helpers.js'; describe(`${tagName} with a slotted tooltip`, () => { let element, tooltip, tooltipOverlay; - before(() => { - Tooltip.setDefaultFocusDelay(0); - Tooltip.setDefaultHoverDelay(0); - Tooltip.setDefaultHideDelay(0); - }); - beforeEach(() => { element = fixtureSync(` <${tagName}> @@ -199,3 +201,50 @@ import { mouseenter, mouseleave } from '@vaadin/tooltip/test/helpers.js'; }); }); }); + +describe('accessible disabled button', () => { + let button, tooltip; + + before(() => { + window.Vaadin.featureFlags ??= {}; + window.Vaadin.featureFlags.accessibleDisabledButtons = true; + }); + + after(() => { + window.Vaadin.featureFlags.accessibleDisabledButtons = false; + }); + + beforeEach(() => { + button = fixtureSync( + `
+ + Press me + + + +
`, + ).firstElementChild; + tooltip = button.querySelector('vaadin-tooltip'); + }); + + afterEach(async () => { + await resetMouse(); + }); + + it('should toggle tooltip on hover when button is disabled', async () => { + const { x, y } = middleOfNode(button); + await sendMouse({ type: 'move', position: [Math.floor(x), Math.floor(y)] }); + expect(tooltip._overlayElement.opened).to.be.true; + + await sendMouse({ type: 'move', position: [0, 0] }); + expect(tooltip._overlayElement.opened).to.be.false; + }); + + it('should toggle tooltip on focus when button is disabled', async () => { + await sendKeys({ press: 'Tab' }); + expect(tooltip._overlayElement.opened).to.be.true; + + await sendKeys({ press: 'Tab' }); + expect(tooltip._overlayElement.opened).to.be.false; + }); +}); From 595b762a3f7ffc08bd4d75773a471f7d4ac1f20e Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Tue, 21 Jan 2025 13:16:27 +0200 Subject: [PATCH 2/7] chore: remove unnecessary native dnd support check in grid (#8543) --- packages/grid/src/vaadin-grid-drag-and-drop-mixin.js | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/packages/grid/src/vaadin-grid-drag-and-drop-mixin.js b/packages/grid/src/vaadin-grid-drag-and-drop-mixin.js index b3911dbb17..0ae54c89f6 100644 --- a/packages/grid/src/vaadin-grid-drag-and-drop-mixin.js +++ b/packages/grid/src/vaadin-grid-drag-and-drop-mixin.js @@ -25,9 +25,6 @@ const DropLocation = { EMPTY: 'empty', }; -// Detects if the browser doesn't support HTML5 Drag & Drop API (and falls back to the @vaadin/vaadin-mobile-drag-drop polyfill) -const usesDnDPolyfill = !('draggable' in document.createElement('div')); - /** * @polymerMixin */ @@ -178,13 +175,8 @@ export const DragAndDropMixin = (superClass) => const rowRect = row.getBoundingClientRect(); - if (usesDnDPolyfill) { - // The polyfill drag image is automatically centered so there is no need to adjust the position - e.dataTransfer.setDragImage(row); - } else { - // The native drag image needs to be shifted manually to compensate for the touch position offset - e.dataTransfer.setDragImage(row, e.clientX - rowRect.left, e.clientY - rowRect.top); - } + // The native drag image needs to be shifted manually to compensate for the touch position offset + e.dataTransfer.setDragImage(row, e.clientX - rowRect.left, e.clientY - rowRect.top); let rows = [row]; if (this._isSelected(row._item)) { From 4498a52b1a5871fed1317ba44de3b643952adb92 Mon Sep 17 00:00:00 2001 From: Serhii Kulykov Date: Tue, 21 Jan 2025 13:50:34 +0200 Subject: [PATCH 3/7] test: prevent date-picker aria-hidden warning in ITs (#8544) --- test/integration/component-tooltip.test.js | 3 ++- test/integration/confirm-dialog-fields.test.js | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/test/integration/component-tooltip.test.js b/test/integration/component-tooltip.test.js index 562234c324..567272e0ab 100644 --- a/test/integration/component-tooltip.test.js +++ b/test/integration/component-tooltip.test.js @@ -119,13 +119,14 @@ before(() => { expect(tooltipOverlay.position).to.equal(position || 'bottom'); }); - it('should or should not show tooltip', () => { + it('should or should not show tooltip', async () => { mouseenter(tooltip.target); expect(tooltipOverlay.opened).to.be.true; mouseleave(tooltip.target); if (applyShouldNotShowCondition) { applyShouldNotShowCondition(element); + await nextRender(); mouseenter(tooltip.target); expect(tooltipOverlay.opened).to.be.false; diff --git a/test/integration/confirm-dialog-fields.test.js b/test/integration/confirm-dialog-fields.test.js index dce56df88a..22f6ca4f0e 100644 --- a/test/integration/confirm-dialog-fields.test.js +++ b/test/integration/confirm-dialog-fields.test.js @@ -30,6 +30,9 @@ describe('confirm-dialog with fields', () => { dialog = document.createElement('vaadin-confirm-dialog'); field = document.createElement(`vaadin-${component}`); field.label = 'Label'; + if (component === 'date-picker') { + field.autoOpenDisabled = true; + } dialog.appendChild(field); document.body.appendChild(dialog); dialog.opened = true; From 69e64bf73dca0812a09c9e3b9646cbe9f1240e27 Mon Sep 17 00:00:00 2001 From: Serhii Kulykov Date: Tue, 21 Jan 2025 14:11:09 +0200 Subject: [PATCH 4/7] chore: align vaadin-card JSDoc annotation in .d.ts file (#8545) --- packages/card/src/vaadin-card.d.ts | 32 +++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/packages/card/src/vaadin-card.d.ts b/packages/card/src/vaadin-card.d.ts index 774413b179..09e68ccb78 100644 --- a/packages/card/src/vaadin-card.d.ts +++ b/packages/card/src/vaadin-card.d.ts @@ -7,13 +7,39 @@ import { ElementMixin } from '@vaadin/component-base/src/element-mixin.js'; import { ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js'; /** - * `` is a visual content container. + * `` is a versatile container for grouping related content and actions. + * It presents information in a structured and visually appealing manner, with + * customization options to fit various design requirements. * * ```html - * - *
Card content
+ * + * + *
Lapland
+ *
The Exotic North
+ *
Lapland is the northern-most region of Finland and an active outdoor destination.
+ * Book Vacation * * ``` + * + * ### Styling + * + * The following shadow DOM parts are available for styling: + * + * Part name | Description + * ----------|------------- + * `media` | The container for the media element (e.g., image, video, icon). Shown above of before the card content. + * `header` | The container for title and subtitle - or for custom header content - and header prefix and suffix elements. + * `content` | The container for the card content (usually text content). + * `footer` | The container for footer elements. This part is always at the bottom of the card. + * + * The following custom properties are available for styling: + * + * Custom property | Description | Default + * ----------------|-------------|------------- + * `--vaadin-card-padding` | The space between the card edge and its content. Needs to a unified value for all edges, i.e., a single length value. | `1em` + * `--vaadin-card-gap` | The space between content elements within the card. | `1em` + * + * See [Styling Components](https://vaadin.com/docs/latest/styling/styling-components) documentation. */ declare class Card extends ElementMixin(ThemableMixin(HTMLElement)) {} From 360bba221f8cbba23ae5c44f8479015d736045d5 Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Tue, 21 Jan 2025 16:16:15 +0400 Subject: [PATCH 5/7] feat: make disabled menu bar buttons accessible with feature flag (#8518) --- .../src/keyboard-direction-mixin.d.ts | 6 + .../a11y-base/src/keyboard-direction-mixin.js | 14 +- packages/button/src/vaadin-button-mixin.js | 12 +- .../src/vaadin-lit-menu-bar-button.js | 14 + .../menu-bar/src/vaadin-menu-bar-button.js | 14 + .../menu-bar/src/vaadin-menu-bar-mixin.d.ts | 20 + .../menu-bar/src/vaadin-menu-bar-mixin.js | 42 +- .../accessible-disabled-buttons-lit.test.js | 3 + ...ccessible-disabled-buttons-polymer.test.js | 3 + .../accessible-disabled-buttons.common.js | 84 +++ test/integration/menu-bar-tooltip.test.js | 570 ++++++++++-------- 11 files changed, 534 insertions(+), 248 deletions(-) create mode 100644 packages/menu-bar/test/accessible-disabled-buttons-lit.test.js create mode 100644 packages/menu-bar/test/accessible-disabled-buttons-polymer.test.js create mode 100644 packages/menu-bar/test/accessible-disabled-buttons.common.js diff --git a/packages/a11y-base/src/keyboard-direction-mixin.d.ts b/packages/a11y-base/src/keyboard-direction-mixin.d.ts index 5a5ab71c4c..bcbf6ef00c 100644 --- a/packages/a11y-base/src/keyboard-direction-mixin.d.ts +++ b/packages/a11y-base/src/keyboard-direction-mixin.d.ts @@ -38,4 +38,10 @@ export declare class KeyboardDirectionMixinClass { * Focus the given item. Override this method to add custom logic. */ protected _focusItem(item: Element, navigating: boolean): void; + + /** + * Returns whether the item is focusable. By default, + * returns true if the item is not disabled. + */ + protected _isItemFocusable(item: Element): boolean; } diff --git a/packages/a11y-base/src/keyboard-direction-mixin.js b/packages/a11y-base/src/keyboard-direction-mixin.js index 5b36382da5..4acbecb52a 100644 --- a/packages/a11y-base/src/keyboard-direction-mixin.js +++ b/packages/a11y-base/src/keyboard-direction-mixin.js @@ -171,7 +171,7 @@ export const KeyboardDirectionMixin = (superclass) => const item = items[idx]; - if (!item.hasAttribute('disabled') && this.__isMatchingItem(item, condition)) { + if (this._isItemFocusable(item) && this.__isMatchingItem(item, condition)) { return idx; } } @@ -189,4 +189,16 @@ export const KeyboardDirectionMixin = (superclass) => __isMatchingItem(item, condition) { return typeof condition === 'function' ? condition(item) : true; } + + /** + * Returns whether the item is focusable. By default, + * returns true if the item is not disabled. + * + * @param {Element} item + * @return {boolean} + * @protected + */ + _isItemFocusable(item) { + return !item.hasAttribute('disabled'); + } }; diff --git a/packages/button/src/vaadin-button-mixin.js b/packages/button/src/vaadin-button-mixin.js index f4ce0b9a3b..068616064e 100644 --- a/packages/button/src/vaadin-button-mixin.js +++ b/packages/button/src/vaadin-button-mixin.js @@ -92,8 +92,18 @@ export const ButtonMixin = (superClass) => /** @private */ __onInteractionEvent(event) { - if (this.disabled) { + if (this.__shouldSuppressInteractionEvent(event)) { event.stopImmediatePropagation(); } } + + /** + * Returns whether to suppress interaction events like `click`, `keydown`, etc. + * By default suppresses all interaction events when the button is disabled. + * + * @private + */ + __shouldSuppressInteractionEvent(_event) { + return this.disabled; + } }; diff --git a/packages/menu-bar/src/vaadin-lit-menu-bar-button.js b/packages/menu-bar/src/vaadin-lit-menu-bar-button.js index c892838743..035760fd1a 100644 --- a/packages/menu-bar/src/vaadin-lit-menu-bar-button.js +++ b/packages/menu-bar/src/vaadin-lit-menu-bar-button.js @@ -48,6 +48,20 @@ class MenuBarButton extends Button { super._onKeyDown(event); this.__triggeredWithActiveKeys = null; } + + /** + * Override method inherited from `ButtonMixin` to allow keyboard navigation with + * arrow keys in the menu bar when the button is focusable in the disabled state. + * + * @override + */ + __shouldSuppressInteractionEvent(event) { + if (event.type === 'keydown' && ['ArrowLeft', 'ArrowRight'].includes(event.key)) { + return false; + } + + return super.__shouldSuppressInteractionEvent(event); + } } defineCustomElement(MenuBarButton); diff --git a/packages/menu-bar/src/vaadin-menu-bar-button.js b/packages/menu-bar/src/vaadin-menu-bar-button.js index 452ce932da..38f89b36f6 100644 --- a/packages/menu-bar/src/vaadin-menu-bar-button.js +++ b/packages/menu-bar/src/vaadin-menu-bar-button.js @@ -45,6 +45,20 @@ class MenuBarButton extends Button { super._onKeyDown(event); this.__triggeredWithActiveKeys = null; } + + /** + * Override method inherited from `ButtonMixin` to allow keyboard navigation with + * arrow keys in the menu bar when the button is focusable in the disabled state. + * + * @override + */ + __shouldSuppressInteractionEvent(event) { + if (event.type === 'keydown' && ['ArrowLeft', 'ArrowRight'].includes(event.key)) { + return false; + } + + return super.__shouldSuppressInteractionEvent(event); + } } defineCustomElement(MenuBarButton); diff --git a/packages/menu-bar/src/vaadin-menu-bar-mixin.d.ts b/packages/menu-bar/src/vaadin-menu-bar-mixin.d.ts index f3b7733ba9..2b878936aa 100644 --- a/packages/menu-bar/src/vaadin-menu-bar-mixin.d.ts +++ b/packages/menu-bar/src/vaadin-menu-bar-mixin.d.ts @@ -99,6 +99,26 @@ export declare class MenuBarMixinClass { * {text: 'Help'} * ]; * ``` + * + * #### Disabled buttons + * + * When a root-level item (menu bar button) is disabled, it prevents all user + * interactions with it, such as focusing, clicking, opening a sub-menu, etc. + * The button is also removed from tab order, which makes it unreachable via + * the keyboard navigation. + * + * While the default behavior effectively prevents accidental interactions, + * it has an accessibility drawback: screen readers skip disabled buttons + * entirely, and users can't see tooltips that might explain why the button + * is disabled. To address this, an experimental enhancement allows disabled + * menu bar buttons to receive focus and show tooltips, while still preventing + * other interactions. This feature can be enabled with the following feature + * flag: + * + * ``` + * // Set before any menu bar is attached to the DOM. + * window.Vaadin.featureFlags.accessibleDisabledButtons = true; + * ``` */ items: MenuBarItem[]; diff --git a/packages/menu-bar/src/vaadin-menu-bar-mixin.js b/packages/menu-bar/src/vaadin-menu-bar-mixin.js index d147dd0fe8..652fee7339 100644 --- a/packages/menu-bar/src/vaadin-menu-bar-mixin.js +++ b/packages/menu-bar/src/vaadin-menu-bar-mixin.js @@ -78,6 +78,26 @@ export const MenuBarMixin = (superClass) => * ]; * ``` * + * #### Disabled buttons + * + * When a root-level item (menu bar button) is disabled, it prevents all user + * interactions with it, such as focusing, clicking, opening a sub-menu, etc. + * The button is also removed from tab order, which makes it unreachable via + * the keyboard navigation. + * + * While the default behavior effectively prevents accidental interactions, + * it has an accessibility drawback: screen readers skip disabled buttons + * entirely, and users can't see tooltips that might explain why the button + * is disabled. To address this, an experimental enhancement allows disabled + * menu bar buttons to receive focus and show tooltips, while still preventing + * other interactions. This feature can be enabled with the following feature + * flag: + * + * ``` + * // Set before any menu bar is attached to the DOM. + * window.Vaadin.featureFlags.accessibleDisabledButtons = true; + * ``` + * * @type {!Array} */ items: { @@ -688,7 +708,7 @@ export const MenuBarMixin = (superClass) => /** @protected */ _setTabindex(button, focused) { - if (this.tabNavigation && !button.disabled) { + if (this.tabNavigation && this._isItemFocusable(button)) { button.setAttribute('tabindex', '0'); } else { button.setAttribute('tabindex', focused ? '0' : '-1'); @@ -907,6 +927,10 @@ export const MenuBarMixin = (superClass) => /** @private */ __openSubMenu(button, keydown, options = {}) { + if (button.disabled) { + return; + } + const subMenu = this._subMenu; const item = button.item; @@ -1030,4 +1054,20 @@ export const MenuBarMixin = (superClass) => close() { this._close(); } + + /** + * Override method inherited from `KeyboardDirectionMixin` to allow + * focusing disabled buttons that are configured so. + * + * @param {Element} button + * @protected + * @override + */ + _isItemFocusable(button) { + if (button.disabled && button.__shouldAllowFocusWhenDisabled) { + return button.__shouldAllowFocusWhenDisabled(); + } + + return super._isItemFocusable(button); + } }; diff --git a/packages/menu-bar/test/accessible-disabled-buttons-lit.test.js b/packages/menu-bar/test/accessible-disabled-buttons-lit.test.js new file mode 100644 index 0000000000..099addfb4b --- /dev/null +++ b/packages/menu-bar/test/accessible-disabled-buttons-lit.test.js @@ -0,0 +1,3 @@ +import './not-animated-styles.js'; +import '../vaadin-lit-menu-bar.js'; +import './accessible-disabled-buttons.common.js'; diff --git a/packages/menu-bar/test/accessible-disabled-buttons-polymer.test.js b/packages/menu-bar/test/accessible-disabled-buttons-polymer.test.js new file mode 100644 index 0000000000..1c7e3648bc --- /dev/null +++ b/packages/menu-bar/test/accessible-disabled-buttons-polymer.test.js @@ -0,0 +1,3 @@ +import './not-animated-styles.js'; +import '../vaadin-menu-bar.js'; +import './accessible-disabled-buttons.common.js'; diff --git a/packages/menu-bar/test/accessible-disabled-buttons.common.js b/packages/menu-bar/test/accessible-disabled-buttons.common.js new file mode 100644 index 0000000000..e264d464b2 --- /dev/null +++ b/packages/menu-bar/test/accessible-disabled-buttons.common.js @@ -0,0 +1,84 @@ +import { expect } from '@vaadin/chai-plugins'; +import { fixtureSync, middleOfNode, nextRender } from '@vaadin/testing-helpers'; +import { resetMouse, sendKeys, sendMouse } from '@web/test-runner-commands'; + +describe('accessible disabled buttons', () => { + let menuBar, buttons; + + before(() => { + window.Vaadin.featureFlags ??= {}; + window.Vaadin.featureFlags.accessibleDisabledButtons = true; + }); + + after(() => { + window.Vaadin.featureFlags.accessibleDisabledButtons = false; + }); + + beforeEach(async () => { + menuBar = fixtureSync(''); + menuBar.items = [ + { text: 'Item 0' }, + { text: 'Item 1', disabled: true, children: [{ text: 'SubItem 0' }] }, + { text: 'Item 2' }, + ]; + await nextRender(menuBar); + buttons = menuBar._buttons; + }); + + afterEach(async () => { + await resetMouse(); + }); + + it('should not open sub-menu on disabled button click', async () => { + const { x, y } = middleOfNode(buttons[1]); + await sendMouse({ type: 'click', position: [Math.floor(x), Math.floor(y)] }); + expect(buttons[1].hasAttribute('expanded')).to.be.false; + }); + + it('should not open sub-menu on disabled button hover', async () => { + menuBar.openOnHover = true; + const { x, y } = middleOfNode(buttons[1]); + await sendMouse({ type: 'move', position: [Math.floor(x), Math.floor(y)] }); + expect(buttons[1].hasAttribute('expanded')).to.be.false; + }); + + it('should include disabled buttons in arrow key navigation', async () => { + await sendKeys({ press: 'Tab' }); + expect(document.activeElement).to.equal(buttons[0]); + + await sendKeys({ press: 'ArrowRight' }); + expect(document.activeElement).to.equal(buttons[1]); + + await sendKeys({ press: 'ArrowRight' }); + expect(document.activeElement).to.equal(buttons[2]); + + await sendKeys({ press: 'ArrowLeft' }); + expect(document.activeElement).to.equal(buttons[1]); + + await sendKeys({ press: 'ArrowLeft' }); + expect(document.activeElement).to.equal(buttons[0]); + }); + + it('should include disabled buttons in Tab navigation', async () => { + menuBar.tabNavigation = true; + + await sendKeys({ press: 'Tab' }); + expect(document.activeElement).to.equal(buttons[0]); + + await sendKeys({ press: 'Tab' }); + expect(document.activeElement).to.equal(buttons[1]); + + await sendKeys({ press: 'Tab' }); + expect(document.activeElement).to.equal(buttons[2]); + + await sendKeys({ down: 'Shift' }); + await sendKeys({ press: 'Tab' }); + await sendKeys({ up: 'Shift' }); + expect(document.activeElement).to.equal(buttons[1]); + + await sendKeys({ down: 'Shift' }); + await sendKeys({ press: 'Tab' }); + await sendKeys({ up: 'Shift' }); + expect(document.activeElement).to.equal(buttons[0]); + }); +}); diff --git a/test/integration/menu-bar-tooltip.test.js b/test/integration/menu-bar-tooltip.test.js index 86ead0c7e0..929b16c808 100644 --- a/test/integration/menu-bar-tooltip.test.js +++ b/test/integration/menu-bar-tooltip.test.js @@ -7,11 +7,14 @@ import { fixtureSync, focusin, focusout, + middleOfNode, mousedown, nextRender, tabKeyDown, } from '@vaadin/testing-helpers'; +import { resetMouse, sendKeys, sendMouse } from '@web/test-runner-commands'; import sinon from 'sinon'; +import './not-animated-styles.js'; import '@vaadin/menu-bar'; import { Tooltip } from '@vaadin/tooltip'; import { mouseenter, mouseleave } from '@vaadin/tooltip/test/helpers.js'; @@ -34,316 +37,393 @@ describe('menu-bar with tooltip', () => { Tooltip.setDefaultHideDelay(0); }); - beforeEach(async () => { - menuBar = fixtureSync(` - - - - `); - menuBar.items = [ - { text: 'Edit', tooltip: 'Edit tooltip' }, - { - text: 'Share', - children: [{ text: 'By email' }], - }, - { - text: 'Move', - tooltip: 'Move tooltip', - children: [{ text: 'To folder' }], - }, - ]; - - await nextRender(); - buttons = menuBar._buttons; - - tooltip = menuBar.querySelector('vaadin-tooltip'); - }); - - it('should set manual on the tooltip to true', () => { - expect(tooltip.manual).to.be.true; - }); - - it('should show tooltip on menu button mouseover', () => { - mouseover(buttons[0]); - expect(tooltip.opened).to.be.true; - }); - - it('should use the tooltip property of an item as tooltip', () => { - mouseover(buttons[0]); - expect(getTooltipText()).to.equal('Edit tooltip'); - }); - - it('should not show tooltip for an item which has no tooltip', () => { - mouseover(buttons[1]); - expect(getTooltipText()).not.to.be.ok; - }); - - it('should not show tooltip on another parent menu button mouseover when open', async () => { - mouseover(buttons[1]); - buttons[1].click(); - await nextRender(); - mouseover(buttons[2]); - await nextRender(); - expect(tooltip.opened).to.be.false; - }); - - it('should hide tooltip on menu bar mouseleave', () => { - mouseover(buttons[0]); - mouseleave(menuBar); - expect(tooltip.opened).to.be.false; - }); - - it('should hide tooltip on menu bar container mouseover', () => { - mouseover(buttons[0]); - mouseover(menuBar._container); - expect(tooltip.opened).to.be.false; - }); - - it('should show tooltip again on menu bar button mouseover', () => { - mouseover(buttons[0]); - mouseover(menuBar._container); - mouseover(buttons[1]); - expect(tooltip.opened).to.be.true; - }); - - it('should set tooltip target on menu button mouseover', () => { - mouseover(buttons[0]); - expect(tooltip.target).to.be.equal(buttons[0]); - }); - - it('should set tooltip context on menu button mouseover', () => { - mouseover(buttons[0]); - expect(tooltip.context).to.be.instanceOf(Object); - expect(tooltip.context.item.text).to.equal('Edit'); - }); - - it('should change tooltip context on another menu button mouseover', () => { - mouseover(buttons[0]); - mouseover(buttons[1]); - expect(tooltip.context.item.text).to.equal('Share'); - }); - - it('should hide tooltip on menu button mousedown', () => { - mouseover(buttons[0]); - mousedown(buttons[0]); - expect(tooltip.opened).to.be.false; - }); - - it('should hide tooltip on mouseleave from overlay to outside', () => { - const overlay = tooltip._overlayElement; - mouseover(buttons[0]); - mouseenter(overlay); - mouseleave(overlay); - expect(overlay.opened).to.be.false; - }); - - it('should not show tooltip on focus without keyboard interaction', async () => { - buttons[0].focus(); - await nextRender(); - expect(tooltip.opened).to.be.false; - }); - - it('should show tooltip on menu button keyboard focus', () => { - tabKeyDown(document.body); - focusin(buttons[0]); - expect(tooltip.opened).to.be.true; - }); - - it('should not show tooltip on another parent menu button focus when open', async () => { - buttons[0].focus(); - arrowRight(buttons[0]); - arrowDown(buttons[1]); - arrowRight(buttons[1]); - await nextRender(); - expect(tooltip.opened).to.be.false; - }); + describe('default', () => { + beforeEach(async () => { + menuBar = fixtureSync(` + + + + `); + menuBar.items = [ + { text: 'Edit', tooltip: 'Edit tooltip' }, + { + text: 'Share', + children: [{ text: 'By email' }], + }, + { + text: 'Move', + tooltip: 'Move tooltip', + children: [{ text: 'To folder' }], + }, + ]; - it('should set tooltip target on menu button keyboard focus', () => { - tabKeyDown(document.body); - focusin(buttons[0]); - expect(tooltip.target).to.be.equal(buttons[0]); - }); + await nextRender(); + buttons = menuBar._buttons; - it('should set tooltip context on menu button keyboard focus', () => { - tabKeyDown(document.body); - focusin(buttons[0]); - expect(tooltip.context).to.be.instanceOf(Object); - expect(tooltip.context.item.text).to.equal('Edit'); - }); + tooltip = menuBar.querySelector('vaadin-tooltip'); + }); - it('should hide tooltip on menu-bar focusout', () => { - tabKeyDown(document.body); - focusin(buttons[0]); - focusout(menuBar); - expect(tooltip.opened).to.be.false; - }); + it('should set manual on the tooltip to true', () => { + expect(tooltip.manual).to.be.true; + }); - it('should hide tooltip on menuBar menu button content Esc', () => { - tabKeyDown(document.body); - focusin(buttons[0]); - escKeyDown(buttons[0]); - expect(tooltip.opened).to.be.false; - }); + it('should show tooltip on menu button mouseover', () => { + mouseover(buttons[0]); + expect(tooltip.opened).to.be.true; + }); - it('should set tooltip opened to false when the menuBar is removed', () => { - mouseover(buttons[0]); + it('should use the tooltip property of an item as tooltip', () => { + mouseover(buttons[0]); + expect(getTooltipText()).to.equal('Edit tooltip'); + }); - menuBar.remove(); + it('should not show tooltip for an item which has no tooltip', () => { + mouseover(buttons[1]); + expect(getTooltipText()).not.to.be.ok; + }); - expect(tooltip.opened).to.be.false; - }); + it('should not show tooltip on another parent menu button mouseover when open', async () => { + mouseover(buttons[1]); + buttons[1].click(); + await nextRender(); + mouseover(buttons[2]); + await nextRender(); + expect(tooltip.opened).to.be.false; + }); - it('should not set tooltip properties if there is no tooltip', async () => { - const spyTarget = sinon.spy(menuBar._tooltipController, 'setTarget'); - const spyContent = sinon.spy(menuBar._tooltipController, 'setContext'); - const spyOpened = sinon.spy(menuBar._tooltipController, 'setOpened'); + it('should hide tooltip on menu bar mouseleave', () => { + mouseover(buttons[0]); + mouseleave(menuBar); + expect(tooltip.opened).to.be.false; + }); - tooltip.remove(); - await nextRender(); + it('should hide tooltip on menu bar container mouseover', () => { + mouseover(buttons[0]); + mouseover(menuBar._container); + expect(tooltip.opened).to.be.false; + }); - mouseover(buttons[0]); + it('should show tooltip again on menu bar button mouseover', () => { + mouseover(buttons[0]); + mouseover(menuBar._container); + mouseover(buttons[1]); + expect(tooltip.opened).to.be.true; + }); - expect(spyTarget.called).to.be.false; - expect(spyContent.called).to.be.false; - expect(spyOpened.called).to.be.false; - }); + it('should set tooltip target on menu button mouseover', () => { + mouseover(buttons[0]); + expect(tooltip.target).to.be.equal(buttons[0]); + }); - it('should not override a custom generator', () => { - tooltip.generator = () => 'Custom tooltip'; - mouseover(buttons[0]); - expect(getTooltipText()).to.equal('Custom tooltip'); - }); + it('should set tooltip context on menu button mouseover', () => { + mouseover(buttons[0]); + expect(tooltip.context).to.be.instanceOf(Object); + expect(tooltip.context.item.text).to.equal('Edit'); + }); - describe('overflow button', () => { - beforeEach(async () => { - // Reduce the width by the width of the last visible button - menuBar.style.display = 'inline-block'; - menuBar.style.width = `${menuBar.offsetWidth - buttons[2].offsetWidth}px`; - await nextRender(); - await nextRender(); - // Increase the width by the width of the overflow button - menuBar.style.width = `${menuBar.offsetWidth + menuBar._overflow.offsetWidth}px`; - await nextRender(); - await nextRender(); + it('should change tooltip context on another menu button mouseover', () => { + mouseover(buttons[0]); + mouseover(buttons[1]); + expect(tooltip.context.item.text).to.equal('Share'); }); - it('should not show tooltip on overflow button mouseover', () => { - mouseover(buttons[buttons.length - 1]); + it('should hide tooltip on menu button mousedown', () => { + mouseover(buttons[0]); + mousedown(buttons[0]); expect(tooltip.opened).to.be.false; }); - it('should close tooltip on overflow button mouseover', () => { + it('should hide tooltip on mouseleave from overlay to outside', () => { + const overlay = tooltip._overlayElement; mouseover(buttons[0]); - mouseover(buttons[buttons.length - 1]); - expect(tooltip.opened).to.be.false; + mouseenter(overlay); + mouseleave(overlay); + expect(overlay.opened).to.be.false; }); - it('should close tooltip on overflow button keyboard navigation', () => { + it('should not show tooltip on focus without keyboard interaction', async () => { buttons[0].focus(); - arrowRight(buttons[0]); - expect(tooltip.opened).to.be.true; - arrowRight(buttons[1]); + await nextRender(); expect(tooltip.opened).to.be.false; }); - it('should not show tooltip on overflow button keyboard focus', () => { + it('should show tooltip on menu button keyboard focus', () => { + tabKeyDown(document.body); + focusin(buttons[0]); + expect(tooltip.opened).to.be.true; + }); + + it('should not show tooltip on another parent menu button focus when open', async () => { buttons[0].focus(); arrowRight(buttons[0]); + arrowDown(buttons[1]); arrowRight(buttons[1]); + await nextRender(); + expect(tooltip.opened).to.be.false; + }); + + it('should set tooltip target on menu button keyboard focus', () => { + tabKeyDown(document.body); + focusin(buttons[0]); + expect(tooltip.target).to.be.equal(buttons[0]); + }); + + it('should set tooltip context on menu button keyboard focus', () => { tabKeyDown(document.body); - focusin(buttons[buttons.length - 1]); + focusin(buttons[0]); + expect(tooltip.context).to.be.instanceOf(Object); + expect(tooltip.context.item.text).to.equal('Edit'); + }); + + it('should hide tooltip on menu-bar focusout', () => { + tabKeyDown(document.body); + focusin(buttons[0]); + focusout(menuBar); expect(tooltip.opened).to.be.false; }); - }); - describe('open on hover', () => { - beforeEach(() => { - menuBar.openOnHover = true; + it('should hide tooltip on menuBar menu button content Esc', () => { + tabKeyDown(document.body); + focusin(buttons[0]); + escKeyDown(buttons[0]); + expect(tooltip.opened).to.be.false; }); - it('should show tooltip on mouseover for button without children', () => { - menuBar.openOnHover = true; + it('should set tooltip opened to false when the menuBar is removed', () => { mouseover(buttons[0]); - expect(tooltip.opened).to.be.true; - }); - it('should not show tooltip on mouseover for button with children', () => { - menuBar.openOnHover = true; - mouseover(buttons[1]); + menuBar.remove(); + expect(tooltip.opened).to.be.false; }); - }); - describe('delay', () => { - const DEFAULT_DELAY = 500; + it('should not set tooltip properties if there is no tooltip', async () => { + const spyTarget = sinon.spy(menuBar._tooltipController, 'setTarget'); + const spyContent = sinon.spy(menuBar._tooltipController, 'setContext'); + const spyOpened = sinon.spy(menuBar._tooltipController, 'setOpened'); + + tooltip.remove(); + await nextRender(); + + mouseover(buttons[0]); + + expect(spyTarget.called).to.be.false; + expect(spyContent.called).to.be.false; + expect(spyOpened.called).to.be.false; + }); + + it('should not override a custom generator', () => { + tooltip.generator = () => 'Custom tooltip'; + mouseover(buttons[0]); + expect(getTooltipText()).to.equal('Custom tooltip'); + }); + + describe('overflow button', () => { + beforeEach(async () => { + // Reduce the width by the width of the last visible button + menuBar.style.display = 'inline-block'; + menuBar.style.width = `${menuBar.offsetWidth - buttons[2].offsetWidth}px`; + await nextRender(); + await nextRender(); + // Increase the width by the width of the overflow button + menuBar.style.width = `${menuBar.offsetWidth + menuBar._overflow.offsetWidth}px`; + await nextRender(); + await nextRender(); + }); + + it('should not show tooltip on overflow button mouseover', () => { + mouseover(buttons[buttons.length - 1]); + expect(tooltip.opened).to.be.false; + }); - let clock; + it('should close tooltip on overflow button mouseover', () => { + mouseover(buttons[0]); + mouseover(buttons[buttons.length - 1]); + expect(tooltip.opened).to.be.false; + }); - beforeEach(() => { - clock = sinon.useFakeTimers({ - shouldClearNativeTimers: true, + it('should close tooltip on overflow button keyboard navigation', () => { + buttons[0].focus(); + arrowRight(buttons[0]); + expect(tooltip.opened).to.be.true; + arrowRight(buttons[1]); + expect(tooltip.opened).to.be.false; + }); + + it('should not show tooltip on overflow button keyboard focus', () => { + buttons[0].focus(); + arrowRight(buttons[0]); + arrowRight(buttons[1]); + tabKeyDown(document.body); + focusin(buttons[buttons.length - 1]); + expect(tooltip.opened).to.be.false; }); }); - afterEach(() => { - // Wait for cooldown - clock.tick(DEFAULT_DELAY); - clock.restore(); + describe('open on hover', () => { + beforeEach(() => { + menuBar.openOnHover = true; + }); + + it('should show tooltip on mouseover for button without children', () => { + menuBar.openOnHover = true; + mouseover(buttons[0]); + expect(tooltip.opened).to.be.true; + }); + + it('should not show tooltip on mouseover for button with children', () => { + menuBar.openOnHover = true; + mouseover(buttons[1]); + expect(tooltip.opened).to.be.false; + }); }); - it('should use hover delay on menu button mouseover', () => { - tooltip.hoverDelay = 100; + describe('delay', () => { + const DEFAULT_DELAY = 500; - mouseover(buttons[0]); - expect(tooltip.opened).to.be.false; + let clock; - clock.tick(100); - expect(tooltip.opened).to.be.true; + beforeEach(() => { + clock = sinon.useFakeTimers({ + shouldClearNativeTimers: true, + }); + }); + + afterEach(() => { + // Wait for cooldown + clock.tick(DEFAULT_DELAY); + clock.restore(); + }); + + it('should use hover delay on menu button mouseover', () => { + tooltip.hoverDelay = 100; + + mouseover(buttons[0]); + expect(tooltip.opened).to.be.false; + + clock.tick(100); + expect(tooltip.opened).to.be.true; + }); + + it('should use hide delay on menu button mouseleave', () => { + tooltip.hideDelay = 100; + + mouseover(buttons[0]); + clock.tick(DEFAULT_DELAY); + + mouseleave(menuBar); + expect(tooltip.opened).to.be.true; + + clock.tick(100); + expect(tooltip.opened).to.be.false; + }); + + it('should not use hide delay on menu button mousedown', () => { + tooltip.hideDelay = 100; + + mouseover(buttons[0]); + clock.tick(DEFAULT_DELAY); + + mousedown(buttons[0]); + expect(tooltip.opened).to.be.false; + }); + + it('should use focus delay on menu button keyboard focus', () => { + tooltip.focusDelay = 100; + + tabKeyDown(document.body); + focusin(buttons[0]); + expect(tooltip.opened).to.be.false; + + clock.tick(100); + expect(tooltip.opened).to.be.true; + }); + + it('should not use hide delay on menu button Esc', () => { + tooltip.hideDelay = 100; + + tabKeyDown(document.body); + focusin(buttons[0]); + clock.tick(DEFAULT_DELAY); + + escKeyDown(buttons[0]); + expect(tooltip.opened).to.be.false; + }); }); + }); - it('should use hide delay on menu button mouseleave', () => { - tooltip.hideDelay = 100; + describe('accessible disabled button', () => { + before(() => { + window.Vaadin.featureFlags ??= {}; + window.Vaadin.featureFlags.accessibleDisabledButtons = true; + }); - mouseover(buttons[0]); - clock.tick(DEFAULT_DELAY); + after(() => { + window.Vaadin.featureFlags.accessibleDisabledButtons = false; + }); - mouseleave(menuBar); - expect(tooltip.opened).to.be.true; + beforeEach(async () => { + menuBar = fixtureSync( + `
+ + + + +
`, + ).firstElementChild; + + menuBar.items = [ + { text: 'Edit' }, + { + text: 'Copy', + tooltip: 'Copy tooltip', + disabled: true, + }, + ]; - clock.tick(100); - expect(tooltip.opened).to.be.false; + await nextRender(); + buttons = menuBar._buttons; + + tooltip = menuBar.querySelector('vaadin-tooltip'); }); - it('should not use hide delay on menu button mousedown', () => { - tooltip.hideDelay = 100; + afterEach(async () => { + await resetMouse(); + }); - mouseover(buttons[0]); - clock.tick(DEFAULT_DELAY); + it('should toggle tooltip on disabled button hover', async () => { + const { x, y } = middleOfNode(buttons[1]); + await sendMouse({ type: 'move', position: [Math.floor(x), Math.floor(y)] }); + expect(getTooltipText()).to.equal('Copy tooltip'); - mousedown(buttons[0]); - expect(tooltip.opened).to.be.false; + await sendMouse({ type: 'move', position: [0, 0] }); + expect(getTooltipText()).to.be.null; }); - it('should use focus delay on menu button keyboard focus', () => { - tooltip.focusDelay = 100; + it('should toggle tooltip on disabled button focus when navigating with arrow keys', async () => { + await sendKeys({ press: 'Tab' }); + expect(getTooltipText()).to.be.null; - tabKeyDown(document.body); - focusin(buttons[0]); - expect(tooltip.opened).to.be.false; + await sendKeys({ press: 'ArrowRight' }); + expect(getTooltipText()).to.equal('Copy tooltip'); - clock.tick(100); - expect(tooltip.opened).to.be.true; + await sendKeys({ press: 'ArrowLeft' }); + expect(getTooltipText()).to.be.null; }); - it('should not use hide delay on menu button Esc', () => { - tooltip.hideDelay = 100; + it('should toggle tooltip on disabled button focus when navigating with Tab', async () => { + menuBar.tabNavigation = true; - tabKeyDown(document.body); - focusin(buttons[0]); - clock.tick(DEFAULT_DELAY); + await sendKeys({ press: 'Tab' }); + expect(getTooltipText()).to.be.null; - escKeyDown(buttons[0]); - expect(tooltip.opened).to.be.false; + await sendKeys({ press: 'Tab' }); + expect(getTooltipText()).to.equal('Copy tooltip'); + + await sendKeys({ down: 'Shift' }); + await sendKeys({ press: 'Tab' }); + await sendKeys({ up: 'Shift' }); + expect(getTooltipText()).to.be.null; }); }); }); From 3c6492d1665bbcb3f88444bd70494a463c105b69 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Tue, 21 Jan 2025 14:27:56 +0200 Subject: [PATCH 6/7] refactor: remove use of calculateSplices in user tags (#8495) --- .../field-highlighter/src/vaadin-user-tags.js | 40 +++++++------------ .../field-highlighter/test/user-tags.test.js | 8 ++++ 2 files changed, 22 insertions(+), 26 deletions(-) diff --git a/packages/field-highlighter/src/vaadin-user-tags.js b/packages/field-highlighter/src/vaadin-user-tags.js index e860f1dcbe..3bff755b78 100644 --- a/packages/field-highlighter/src/vaadin-user-tags.js +++ b/packages/field-highlighter/src/vaadin-user-tags.js @@ -5,7 +5,6 @@ */ import './vaadin-user-tag.js'; import './vaadin-user-tags-overlay.js'; -import { calculateSplices } from '@polymer/polymer/lib/utils/array-splice.js'; import { html, PolymerElement } from '@polymer/polymer/polymer-element.js'; import { timeOut } from '@vaadin/component-base/src/async.js'; import { Debouncer } from '@vaadin/component-base/src/debounce.js'; @@ -233,27 +232,6 @@ export class UserTags extends PolymerElement { return { added, removed }; } - getChangedUsers(users, splices) { - const usersToAdd = []; - const usersToRemove = []; - - splices.forEach((splice) => { - splice.removed.forEach((user) => { - usersToRemove.push(user); - }); - - for (let i = splice.addedCount - 1; i >= 0; i--) { - usersToAdd.push(users[splice.index + i]); - } - }); - - // Filter out users that are only moved - const addedUsers = usersToAdd.filter((u) => !usersToRemove.some((u2) => u.id === u2.id)); - const removedUsers = usersToRemove.filter((u) => !usersToAdd.some((u2) => u.id === u2.id)); - - return { addedUsers, removedUsers }; - } - applyTagsStart({ added, removed }) { const wrapper = this.wrapper; removed.forEach((tag) => { @@ -279,12 +257,22 @@ export class UserTags extends PolymerElement { // Apply pending change if needed this.requestContentUpdate(); - const splices = calculateSplices(users, this.users); - if (splices.length === 0) { - return; + let addedUsers = []; + let removedUsers = []; + + const hasNewUsers = Array.isArray(users); + const hasOldUsers = Array.isArray(this.users); + + if (hasOldUsers) { + const newUserIds = (users || []).map((user) => user.id); + removedUsers = this.users.filter((item) => !newUserIds.includes(item.id)); + } + + if (hasNewUsers) { + const oldUserIds = (this.users || []).map((user) => user.id); + addedUsers = users.filter((item) => !oldUserIds.includes(item.id)).reverse(); } - const { addedUsers, removedUsers } = this.getChangedUsers(users, splices); if (addedUsers.length === 0 && removedUsers.length === 0) { return; } diff --git a/packages/field-highlighter/test/user-tags.test.js b/packages/field-highlighter/test/user-tags.test.js index 672cc2d8a7..040ca6d6ad 100644 --- a/packages/field-highlighter/test/user-tags.test.js +++ b/packages/field-highlighter/test/user-tags.test.js @@ -108,6 +108,14 @@ describe('user-tags', () => { expect(getComputedStyle(tags[1]).backgroundColor).to.equal('rgb(255, 0, 0)'); }); + it('should not clear tags with a new instance of same user', () => { + setUsers([user1]); + setUsers([{ ...user1 }]); + + const tags = getTags(); + expect(tags).to.have.lengthOf(1); + }); + it('should not set custom property if index is null', () => { addUser({ name: 'xyz', colorIndex: null }); const tags = getTags(); From dc87442bb489e6d447c42fdcb95b4e2921abf86e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?= Date: Tue, 21 Jan 2025 15:04:37 +0200 Subject: [PATCH 7/7] refactor: extract vertical layout styles into CSS literal (#8546) --- .../src/vaadin-lit-vertical-layout.js | 36 ++++--------------- .../src/vaadin-vertical-layout-styles.d.ts | 8 +++++ .../src/vaadin-vertical-layout-styles.js | 32 +++++++++++++++++ .../src/vaadin-vertical-layout.js | 35 +++--------------- 4 files changed, 52 insertions(+), 59 deletions(-) create mode 100644 packages/vertical-layout/src/vaadin-vertical-layout-styles.d.ts create mode 100644 packages/vertical-layout/src/vaadin-vertical-layout-styles.js diff --git a/packages/vertical-layout/src/vaadin-lit-vertical-layout.js b/packages/vertical-layout/src/vaadin-lit-vertical-layout.js index 758b6e3d31..549214af92 100644 --- a/packages/vertical-layout/src/vaadin-lit-vertical-layout.js +++ b/packages/vertical-layout/src/vaadin-lit-vertical-layout.js @@ -3,11 +3,12 @@ * Copyright (c) 2017 - 2025 Vaadin Ltd. * This program is available under Apache License Version 2.0, available at https://vaadin.com/license/ */ -import { css, html, LitElement } from 'lit'; +import { html, LitElement } from 'lit'; import { defineCustomElement } from '@vaadin/component-base/src/define.js'; import { ElementMixin } from '@vaadin/component-base/src/element-mixin.js'; import { PolylitMixin } from '@vaadin/component-base/src/polylit-mixin.js'; import { ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js'; +import { verticalLayoutStyles } from './vaadin-vertical-layout-styles.js'; /** * LitElement based version of `` web component. @@ -19,38 +20,15 @@ import { ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mix * Feel free to try this code in your apps as per Apache 2.0 license. */ class VerticalLayout extends ThemableMixin(ElementMixin(PolylitMixin(LitElement))) { - static get styles() { - return css` - :host { - display: flex; - flex-direction: column; - align-items: flex-start; - box-sizing: border-box; - } - - :host([hidden]) { - display: none !important; - } - - /* Theme variations */ - :host([theme~='margin']) { - margin: 1em; - } - - :host([theme~='padding']) { - padding: 1em; - } - - :host([theme~='spacing']) { - gap: 1em; - } - `; - } - static get is() { return 'vaadin-vertical-layout'; } + static get styles() { + return verticalLayoutStyles; + } + + /** @protected */ render() { return html``; } diff --git a/packages/vertical-layout/src/vaadin-vertical-layout-styles.d.ts b/packages/vertical-layout/src/vaadin-vertical-layout-styles.d.ts new file mode 100644 index 0000000000..570e7aedb2 --- /dev/null +++ b/packages/vertical-layout/src/vaadin-vertical-layout-styles.d.ts @@ -0,0 +1,8 @@ +/** + * @license + * Copyright (c) 2017 - 2025 Vaadin Ltd. + * This program is available under Apache License Version 2.0, available at https://vaadin.com/license/ + */ +import type { CSSResult } from 'lit'; + +export const verticalLayoutStyles: CSSResult; diff --git a/packages/vertical-layout/src/vaadin-vertical-layout-styles.js b/packages/vertical-layout/src/vaadin-vertical-layout-styles.js new file mode 100644 index 0000000000..f345c8f98d --- /dev/null +++ b/packages/vertical-layout/src/vaadin-vertical-layout-styles.js @@ -0,0 +1,32 @@ +/** + * @license + * Copyright (c) 2017 - 2025 Vaadin Ltd. + * This program is available under Apache License Version 2.0, available at https://vaadin.com/license/ + */ +import { css } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js'; + +export const verticalLayoutStyles = css` + :host { + display: flex; + flex-direction: column; + align-items: flex-start; + box-sizing: border-box; + } + + :host([hidden]) { + display: none !important; + } + + /* Theme variations */ + :host([theme~='margin']) { + margin: 1em; + } + + :host([theme~='padding']) { + padding: 1em; + } + + :host([theme~='spacing']) { + gap: 1em; + } +`; diff --git a/packages/vertical-layout/src/vaadin-vertical-layout.js b/packages/vertical-layout/src/vaadin-vertical-layout.js index 8403751b70..146e4532ed 100644 --- a/packages/vertical-layout/src/vaadin-vertical-layout.js +++ b/packages/vertical-layout/src/vaadin-vertical-layout.js @@ -6,7 +6,10 @@ import { html, PolymerElement } from '@polymer/polymer/polymer-element.js'; import { defineCustomElement } from '@vaadin/component-base/src/define.js'; import { ElementMixin } from '@vaadin/component-base/src/element-mixin.js'; -import { ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js'; +import { registerStyles, ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js'; +import { verticalLayoutStyles } from './vaadin-vertical-layout-styles.js'; + +registerStyles('vaadin-vertical-layout', verticalLayoutStyles, { moduleId: 'vaadin-vertical-layout-styles' }); /** * `` provides a simple way to vertically align your HTML elements. @@ -36,35 +39,7 @@ import { ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mix */ class VerticalLayout extends ElementMixin(ThemableMixin(PolymerElement)) { static get template() { - return html` - - - - `; + return html``; } static get is() {