From ba09556060e80ddf131b1aa1e7a64d8ee950ff9d Mon Sep 17 00:00:00 2001 From: Boyan Rakilovski Date: Mon, 11 Aug 2025 12:31:11 +0300 Subject: [PATCH 1/6] fix(ui5-color-palette): adjust keyboard navigation - Home and End keyboard shortcuts now work only in palette popover mode. - The current item navigation index is getting updated on "mousedown" event. - Home and End keyboard shortcuts now work in popover mode as per specification. Fixes: #8744 Fixes: #11439 --- .../cypress/specs/ColorPalettePopover.cy.tsx | 133 ++++++++++++++++++ packages/main/cypress/support/commands.ts | 1 + .../commands/ColorPalettePopover.commands.ts | 44 ++++++ packages/main/src/ColorPalette.ts | 49 +++++++ packages/main/src/ColorPaletteTemplate.tsx | 1 + 5 files changed, 228 insertions(+) create mode 100644 packages/main/cypress/specs/ColorPalettePopover.cy.tsx create mode 100644 packages/main/cypress/support/commands/ColorPalettePopover.commands.ts diff --git a/packages/main/cypress/specs/ColorPalettePopover.cy.tsx b/packages/main/cypress/specs/ColorPalettePopover.cy.tsx new file mode 100644 index 000000000000..24dfe038319f --- /dev/null +++ b/packages/main/cypress/specs/ColorPalettePopover.cy.tsx @@ -0,0 +1,133 @@ +import Button from "../../src/Button.js"; +import ColorPalettePopover from "../../src/ColorPalettePopover.js"; +import ColorPaletteItem from "../../src/ColorPaletteItem.js"; + +describe("Color Popover Palette tests", () => { + describe("Home and End keyboard navigation", () => { + it("showDefaultColor & showMoreColors", () => { + cy.mount(<> + + + + + + + + ); + + cy.get("[ui5-color-palette-popover]") + .ui5PaletteOpen(); + + cy.focused() + .should("have.attr", "aria-label", "Default Color"); + + cy.focused() + .realPress("End"); + + cy.focused() + .should("have.attr", "aria-label", "More Colors..."); + + cy.focused() + .realPress("Home"); + + cy.focused() + .should("have.attr", "aria-label", "Default Color"); + }); + + it("showDefaultColor", () => { + cy.mount(<> + + + + + + + + ); + + cy.get("[ui5-color-palette-popover]") + .ui5PaletteOpen(); + + cy.focused() + .realPress("End"); + + cy.focused() + .should("have.attr", "aria-label", "Color - 4: red"); + + cy.focused() + .realPress("Home"); + + cy.focused() + .should("have.attr", "aria-label", "Color - 1: cyan"); + + cy.focused() + .realPress("Home"); + + cy.focused() + .should("have.attr", "aria-label", "Default Color"); + }); + + it("showMoreColors", () => { + cy.mount(<> + + + + + + + + ); + + cy.get("[ui5-color-palette-popover]") + .ui5PaletteOpen(); + + cy.focused() + .should("have.attr", "aria-label", "Color - 1: cyan"); + + cy.focused() + .realPress("End"); + + cy.focused() + .should("have.attr", "aria-label", "Color - 4: red"); + + cy.focused() + .realPress("End"); + + cy.focused() + .should("have.attr", "aria-label", "More Colors..."); + }); + + it("Item navigation End", () => { + cy.mount(<> + + + + + + + + + + + ); + + cy.get("[ui5-color-palette-popover]") + .ui5PaletteOpen(); + + cy.focused() + .should("have.attr", "aria-label", "Color - 1: cyan"); + + cy.focused() + .realPress("ArrowDown"); + + cy.focused() + .should("have.attr", "aria-label", "Color - 6: purple"); + + cy.focused() + .realPress("End"); + + cy.focused() + .should("have.attr", "aria-label", "Color - 7: red"); + }); + }); +}); \ No newline at end of file diff --git a/packages/main/cypress/support/commands.ts b/packages/main/cypress/support/commands.ts index ec83e0f4d043..c74d0c439ed2 100644 --- a/packages/main/cypress/support/commands.ts +++ b/packages/main/cypress/support/commands.ts @@ -43,6 +43,7 @@ import { ModifierKey } from "./commands/common/types.js"; // Please keep this list in alphabetical order import "./commands/Calendar.commands.js"; import "./commands/ColorPalette.commands.js"; +import "./commands/ColorPalettePopover.commands.ts"; import "./commands/ColorPicker.commands.js"; import "./commands/DateTimePicker.commands.js"; import "./commands/DateRangePicker.commands.js"; diff --git a/packages/main/cypress/support/commands/ColorPalettePopover.commands.ts b/packages/main/cypress/support/commands/ColorPalettePopover.commands.ts new file mode 100644 index 000000000000..72e280b5b82e --- /dev/null +++ b/packages/main/cypress/support/commands/ColorPalettePopover.commands.ts @@ -0,0 +1,44 @@ +Cypress.Commands.add("ui5PaletteOpen", { prevSubject: true }, (prevSubject, options) => { + cy.wrap(prevSubject) + .as("palette") + .then($palette => { + if (options?.opener) { + cy.wrap($palette) + .invoke("attr", "opener", options.opener); + } + + cy.wrap($palette) + .invoke("attr", "open", true); + }); + + cy.get("@palette") + .ui5PaletteOpened(); +}); + +Cypress.Commands.add("ui5PaletteOpened", { prevSubject: true }, subject => { + cy.wrap(subject) + .as("palette"); + + cy.get("@palette") + .should("have.attr", "open"); + + cy.get("@palette") + .shadow() + .find("[ui5-responsive-popover]") + .should($rp => { + expect($rp.is(":popover-open")).to.be.true; + expect($rp.width()).to.not.equal(0); + expect($rp.height()).to.not.equal(0); + }) + .and("have.attr", "open"); +}); + + +declare global { + namespace Cypress { + interface Chainable { + ui5PaletteOpen(options?: { opener?: string }): Chainable + ui5PaletteOpened(): Chainable + } + } +} \ No newline at end of file diff --git a/packages/main/src/ColorPalette.ts b/packages/main/src/ColorPalette.ts index c6ca6b5c12a4..837e7ea01987 100644 --- a/packages/main/src/ColorPalette.ts +++ b/packages/main/src/ColorPalette.ts @@ -2,6 +2,7 @@ import UI5Element from "@ui5/webcomponents-base/dist/UI5Element.js"; import customElement from "@ui5/webcomponents-base/dist/decorators/customElement.js"; import property from "@ui5/webcomponents-base/dist/decorators/property.js"; import slot from "@ui5/webcomponents-base/dist/decorators/slot.js"; +import query from "@ui5/webcomponents-base/dist/decorators/query.js"; import event from "@ui5/webcomponents-base/dist/decorators/event-strict.js"; import jsxRenderer from "@ui5/webcomponents-base/dist/renderer/JsxRenderer.js"; import i18n from "@ui5/webcomponents-base/dist/decorators/i18n.js"; @@ -17,6 +18,8 @@ import { isDown, isUp, isTabNext, + isHome, + isEnd, } from "@ui5/webcomponents-base/dist/Keys.js"; import ColorPaletteTemplate from "./ColorPaletteTemplate.js"; import type ColorPaletteItem from "./ColorPaletteItem.js"; @@ -187,6 +190,12 @@ class ColorPalette extends UI5Element { _currentlySelected?: ColorPaletteItem; _shouldFocusRecentColors = false; + @query(".ui5-cp-default-color-button") + _defaultColorButton!: Button; + + @query(".ui5-cp-more-colors") + _moreColorsButton!: Button; + @i18n("@ui5/webcomponents") static i18nBundle: I18nBundle; @@ -309,6 +318,16 @@ class ColorPalette extends UI5Element { this.handleSelection(e.target as ColorPaletteItem); } + _onmousedown(e: MouseEvent) { + const target = e.target as ColorPaletteItem; + + if (this.displayedColors.includes(target)) { + this._itemNavigation.setCurrentItem(target); + } else if (this.recentColorsElements.includes(target)) { + this._itemNavigationRecentColors.setCurrentItem(target); + } + } + _onkeyup(e: KeyboardEvent) { const target = e.target as ColorPaletteItem; if (isSpace(e)) { @@ -326,6 +345,11 @@ class ColorPalette extends UI5Element { if (isSpace(e)) { e.preventDefault(); } + + if (!this.popupMode && (isHome(e) || isEnd(e))) { + e.preventDefault(); + e.stopPropagation(); + } } handleSelection(target: ColorPaletteItem) { @@ -393,6 +417,14 @@ class ColorPalette extends UI5Element { this.focusColorElement(this.displayedColors[colorPaletteFocusIndex], this._itemNavigation); } + } else if (isEnd(e)) { + e.stopPropagation(); + + if (this.showMoreColors && this._moreColorsButton) { + this._moreColorsButton.focus(); + } else if (this.displayedColors.length) { + this.focusColorElement(this.displayedColors[this.displayedColors.length - 1], this._itemNavigation); + } } } @@ -415,6 +447,14 @@ class ColorPalette extends UI5Element { } else { this.focusColorElement(this.displayedColors[0], this._itemNavigation); } + } else if (isHome(e)) { + e.stopPropagation(); + + if (this.showDefaultColor && this._defaultColorButton) { + this._defaultColorButton.focus(); + } else if (this.displayedColors.length) { + this.focusColorElement(this.displayedColors[0], this._itemNavigation); + } } } @@ -457,6 +497,15 @@ class ColorPalette extends UI5Element { } else if (!this.showDefaultColor && this.showMoreColors) { this.colorPaletteNavigationElements[1].focus(); } + } else if (isHome(e) && (target === this.displayedColors[0])) { + e.stopPropagation(); + this._defaultColorButton?.focus(); + } else if (isEnd(e) && (target === this.displayedColors[this.displayedColors.length - 1])) { + e.stopPropagation(); + this._moreColorsButton?.focus(); + } else if (isEnd(e) && (this.displayedColors.indexOf(target) >= this.displayedColors.length - (this.displayedColors.length % this.rowSize))) { + e.stopPropagation(); + this.focusColorElement(this.displayedColors[this.displayedColors.length - 1], this._itemNavigation); } } diff --git a/packages/main/src/ColorPaletteTemplate.tsx b/packages/main/src/ColorPaletteTemplate.tsx index 7d726ac84f1e..38030e8911fb 100644 --- a/packages/main/src/ColorPaletteTemplate.tsx +++ b/packages/main/src/ColorPaletteTemplate.tsx @@ -15,6 +15,7 @@ export default function ColorPaletteTemplate(this: ColorPalette) { onClick={this._onclick} onKeyUp={this._onkeyup} onKeyDown={this._onkeydown} + onMouseDown={this._onmousedown} > {this.showDefaultColor &&
From fb313d3fedaae4892531f12a608081aa40b1e0dc Mon Sep 17 00:00:00 2001 From: Boyan Rakilovski Date: Mon, 11 Aug 2025 18:03:48 +0300 Subject: [PATCH 2/6] fix: resolve comments --- packages/main/src/ColorPalette.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/main/src/ColorPalette.ts b/packages/main/src/ColorPalette.ts index 837e7ea01987..f7b4f9472a64 100644 --- a/packages/main/src/ColorPalette.ts +++ b/packages/main/src/ColorPalette.ts @@ -465,6 +465,8 @@ class ColorPalette extends UI5Element { _onColorContainerKeyDown(e: KeyboardEvent) { const target = e.target as ColorPaletteItem; const lastElementInNavigation = this.colorPaletteNavigationElements[this.colorPaletteNavigationElements.length - 1]; + const isInLastRow = this.colorPaletteNavigationElements.indexOf(target) + >= this.colorPaletteNavigationElements.length - (this.colorPaletteNavigationElements.length % this.rowSize); if (this._isUpOrDownNavigatableColorPaletteItem(e)) { this._currentlySelected = undefined; @@ -497,13 +499,13 @@ class ColorPalette extends UI5Element { } else if (!this.showDefaultColor && this.showMoreColors) { this.colorPaletteNavigationElements[1].focus(); } - } else if (isHome(e) && (target === this.displayedColors[0])) { + } else if (isHome(e) && this.showDefaultColor && (target === this.displayedColors[0])) { e.stopPropagation(); this._defaultColorButton?.focus(); - } else if (isEnd(e) && (target === this.displayedColors[this.displayedColors.length - 1])) { + } else if (isEnd(e) && this.showMoreColors && (target === this.displayedColors[this.displayedColors.length - 1])) { e.stopPropagation(); this._moreColorsButton?.focus(); - } else if (isEnd(e) && (this.displayedColors.indexOf(target) >= this.displayedColors.length - (this.displayedColors.length % this.rowSize))) { + } else if (isEnd(e) && isInLastRow) { e.stopPropagation(); this.focusColorElement(this.displayedColors[this.displayedColors.length - 1], this._itemNavigation); } From 4f6b5fbcba28caba903c79f085782b0dd4e32f78 Mon Sep 17 00:00:00 2001 From: Boyan Rakilovski Date: Mon, 11 Aug 2025 18:17:41 +0300 Subject: [PATCH 3/6] fix: resolve comments --- packages/main/src/ColorPalette.ts | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/packages/main/src/ColorPalette.ts b/packages/main/src/ColorPalette.ts index f7b4f9472a64..534bee988155 100644 --- a/packages/main/src/ColorPalette.ts +++ b/packages/main/src/ColorPalette.ts @@ -420,8 +420,8 @@ class ColorPalette extends UI5Element { } else if (isEnd(e)) { e.stopPropagation(); - if (this.showMoreColors && this._moreColorsButton) { - this._moreColorsButton.focus(); + if (this.showMoreColors) { + this._moreColorsButton?.focus(); } else if (this.displayedColors.length) { this.focusColorElement(this.displayedColors[this.displayedColors.length - 1], this._itemNavigation); } @@ -450,8 +450,8 @@ class ColorPalette extends UI5Element { } else if (isHome(e)) { e.stopPropagation(); - if (this.showDefaultColor && this._defaultColorButton) { - this._defaultColorButton.focus(); + if (this.showDefaultColor) { + this._defaultColorButton?.focus(); } else if (this.displayedColors.length) { this.focusColorElement(this.displayedColors[0], this._itemNavigation); } @@ -477,7 +477,7 @@ class ColorPalette extends UI5Element { this.selectColor(target); } - if (isUp(e) && target === this.displayedColors[0] && this.colorPaletteNavigationElements.length > 1) { + if (isUp(e) && this._isFirstDisplayedSwatch(target) && this.colorPaletteNavigationElements.length > 1) { e.stopPropagation(); if (this.showDefaultColor) { this.firstFocusableElement.focus(); @@ -486,7 +486,7 @@ class ColorPalette extends UI5Element { } else if (!this.showDefaultColor && this.showMoreColors) { lastElementInNavigation.focus(); } - } else if (isDown(e) && target === this.displayedColors[this.displayedColors.length - 1] && this.colorPaletteNavigationElements.length > 1) { + } else if (isDown(e) && this._isLastDisplayedSwatch(target) && this.colorPaletteNavigationElements.length > 1) { e.stopPropagation(); const isRecentColorsNextElement = (this.showDefaultColor && !this.showMoreColors && this.hasRecentColors) || (!this.showDefaultColor && !this.showMoreColors && this.hasRecentColors); @@ -499,10 +499,10 @@ class ColorPalette extends UI5Element { } else if (!this.showDefaultColor && this.showMoreColors) { this.colorPaletteNavigationElements[1].focus(); } - } else if (isHome(e) && this.showDefaultColor && (target === this.displayedColors[0])) { + } else if (isHome(e) && this.showDefaultColor && this._isFirstDisplayedSwatch(target)) { e.stopPropagation(); this._defaultColorButton?.focus(); - } else if (isEnd(e) && this.showMoreColors && (target === this.displayedColors[this.displayedColors.length - 1])) { + } else if (isEnd(e) && this.showMoreColors && this._isLastDisplayedSwatch(target)) { e.stopPropagation(); this._moreColorsButton?.focus(); } else if (isEnd(e) && isInLastRow) { @@ -511,6 +511,14 @@ class ColorPalette extends UI5Element { } } + _isFirstDisplayedSwatch(target: ColorPaletteItem) { + return this.displayedColors[0] === target; + } + + _isLastDisplayedSwatch(target: ColorPaletteItem) { + return this.displayedColors[this.displayedColors.length - 1] === target; + } + _onRecentColorsContainerKeyDown(e: KeyboardEvent) { if (this._isUpOrDownNavigatableColorPaletteItem(e)) { this._currentlySelected = undefined; From 83795a0fab4d6582ca9a1e28dd890651ec97d228 Mon Sep 17 00:00:00 2001 From: Boyan Rakilovski Date: Mon, 11 Aug 2025 18:20:01 +0300 Subject: [PATCH 4/6] fix: resolve comments --- packages/main/src/ColorPalette.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/main/src/ColorPalette.ts b/packages/main/src/ColorPalette.ts index 534bee988155..1279fce718e7 100644 --- a/packages/main/src/ColorPalette.ts +++ b/packages/main/src/ColorPalette.ts @@ -465,8 +465,6 @@ class ColorPalette extends UI5Element { _onColorContainerKeyDown(e: KeyboardEvent) { const target = e.target as ColorPaletteItem; const lastElementInNavigation = this.colorPaletteNavigationElements[this.colorPaletteNavigationElements.length - 1]; - const isInLastRow = this.colorPaletteNavigationElements.indexOf(target) - >= this.colorPaletteNavigationElements.length - (this.colorPaletteNavigationElements.length % this.rowSize); if (this._isUpOrDownNavigatableColorPaletteItem(e)) { this._currentlySelected = undefined; @@ -505,7 +503,7 @@ class ColorPalette extends UI5Element { } else if (isEnd(e) && this.showMoreColors && this._isLastDisplayedSwatch(target)) { e.stopPropagation(); this._moreColorsButton?.focus(); - } else if (isEnd(e) && isInLastRow) { + } else if (isEnd(e) && this._isElementInLastRow(target)) { e.stopPropagation(); this.focusColorElement(this.displayedColors[this.displayedColors.length - 1], this._itemNavigation); } @@ -519,6 +517,11 @@ class ColorPalette extends UI5Element { return this.displayedColors[this.displayedColors.length - 1] === target; } + _isElementInLastRow(target: ColorPaletteItem) { + const index = this.colorPaletteNavigationElements.indexOf(target); + return index >= this.colorPaletteNavigationElements.length - (this.colorPaletteNavigationElements.length % this.rowSize); + } + _onRecentColorsContainerKeyDown(e: KeyboardEvent) { if (this._isUpOrDownNavigatableColorPaletteItem(e)) { this._currentlySelected = undefined; From a013ab37ba9fe1108e05f452359ef4437f7f84e7 Mon Sep 17 00:00:00 2001 From: Boyan Rakilovski Date: Mon, 11 Aug 2025 18:22:38 +0300 Subject: [PATCH 5/6] fix: resolve comments --- packages/main/src/ColorPalette.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/main/src/ColorPalette.ts b/packages/main/src/ColorPalette.ts index 1279fce718e7..2f6192c9de3b 100644 --- a/packages/main/src/ColorPalette.ts +++ b/packages/main/src/ColorPalette.ts @@ -518,8 +518,8 @@ class ColorPalette extends UI5Element { } _isElementInLastRow(target: ColorPaletteItem) { - const index = this.colorPaletteNavigationElements.indexOf(target); - return index >= this.colorPaletteNavigationElements.length - (this.colorPaletteNavigationElements.length % this.rowSize); + const index = this.displayedColors.indexOf(target); + return index >= this.displayedColors.length - (this.displayedColors.length % this.rowSize); } _onRecentColorsContainerKeyDown(e: KeyboardEvent) { From 369d659dca1234f8071af9c28db0d48a0d2fa2f3 Mon Sep 17 00:00:00 2001 From: Boyan Rakilovski Date: Mon, 11 Aug 2025 18:24:45 +0300 Subject: [PATCH 6/6] fix: resolve comments --- packages/main/src/ColorPalette.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/main/src/ColorPalette.ts b/packages/main/src/ColorPalette.ts index 2f6192c9de3b..fc4253b41105 100644 --- a/packages/main/src/ColorPalette.ts +++ b/packages/main/src/ColorPalette.ts @@ -519,7 +519,8 @@ class ColorPalette extends UI5Element { _isElementInLastRow(target: ColorPaletteItem) { const index = this.displayedColors.indexOf(target); - return index >= this.displayedColors.length - (this.displayedColors.length % this.rowSize); + const length = this.displayedColors.length; + return index >= length - (length % this.rowSize); } _onRecentColorsContainerKeyDown(e: KeyboardEvent) {