From 2bb2bfc98c1606a84f425a45e9c5052893b53291 Mon Sep 17 00:00:00 2001 From: Nikola Anachkov Date: Thu, 23 Apr 2026 12:55:43 +0300 Subject: [PATCH 1/2] fix(ui5-option): add down/active state on press --- packages/main/cypress/specs/Select.cy.tsx | 50 ++++++++++++++++++++++ packages/main/src/ListItemBaseTemplate.tsx | 10 +++-- packages/main/src/Option.ts | 35 +++++++++++++++ packages/main/src/OptionCustom.ts | 35 +++++++++++++++ packages/main/src/OptionCustomTemplate.tsx | 7 ++- packages/main/src/OptionTemplate.tsx | 7 ++- packages/main/src/themes/ListItemBase.css | 1 + 7 files changed, 140 insertions(+), 5 deletions(-) diff --git a/packages/main/cypress/specs/Select.cy.tsx b/packages/main/cypress/specs/Select.cy.tsx index 54ae56d3906f..264655ccaf05 100644 --- a/packages/main/cypress/specs/Select.cy.tsx +++ b/packages/main/cypress/specs/Select.cy.tsx @@ -1913,3 +1913,53 @@ describe("Select general interaction", () => { .should("be.focused"); }); }); + +describe("Select - active/down state", () => { + it("sets active attribute on ui5-option while mouse is pressed", () => { + cy.mount( + + ); + + cy.get("[ui5-select]").realClick(); + cy.get("[ui5-select]").should("have.attr", "opened"); + + cy.get("[ui5-select]") + .find("[ui5-option]") + .eq(0) + .realMouseDown() + .should("have.attr", "active"); + + cy.get("[ui5-select]") + .find("[ui5-option]") + .eq(0) + .realMouseUp() + .should("not.have.attr", "active"); + }); + + it("sets active attribute on ui5-option-custom while mouse is pressed", () => { + cy.mount( + + ); + + cy.get("[ui5-select]").realClick(); + cy.get("[ui5-select]").should("have.attr", "opened"); + + cy.get("[ui5-select]") + .find("[ui5-option-custom]") + .eq(0) + .realMouseDown() + .should("have.attr", "active"); + + cy.get("[ui5-select]") + .find("[ui5-option-custom]") + .eq(0) + .realMouseUp() + .should("not.have.attr", "active"); + }); +}); diff --git a/packages/main/src/ListItemBaseTemplate.tsx b/packages/main/src/ListItemBaseTemplate.tsx index 5af5ef6b3628..fcca79d61ede 100644 --- a/packages/main/src/ListItemBaseTemplate.tsx +++ b/packages/main/src/ListItemBaseTemplate.tsx @@ -1,9 +1,11 @@ import type ListItemBase from "./ListItemBase.js"; -import type { AriaRole } from "@ui5/webcomponents-base/"; +import type { AriaRole, JsxTemplate } from "@ui5/webcomponents-base/"; -export default function ListItemBaseTemplate(this: ListItemBase, hooks?: { listItemContent: () => void }, injectedProps?: { +export default function ListItemBaseTemplate(this: ListItemBase, hooks?: { listItemContent: JsxTemplate }, injectedProps?: { role?: AriaRole, title?: string, + onMouseDown?: (e: MouseEvent) => void, + onTouchStart?: (e: TouchEvent) => void, }) { const listItemContent = hooks?.listItemContent || defaultListItemContent; @@ -20,8 +22,10 @@ export default function ListItemBaseTemplate(this: ListItemBase, hooks?: { listI onKeyUp={this._onkeyup} onKeyDown={this._onkeydown} onClick={this._onclick} + onMouseDown={injectedProps?.onMouseDown} + onTouchStart={injectedProps?.onTouchStart} > - { listItemContent.call(this) } + { listItemContent.call(this) as JSX.Element } ); } diff --git a/packages/main/src/Option.ts b/packages/main/src/Option.ts index 6cd5c44cdfc2..0c5d8066be85 100644 --- a/packages/main/src/Option.ts +++ b/packages/main/src/Option.ts @@ -41,6 +41,30 @@ import type { DefaultSlot } from "@ui5/webcomponents-base/dist/UI5Element.js"; class Option extends ListItemBase implements IOption { eventDetails!: ListItemBase["eventDetails"]; + // Note: same active state logic exists in OptionCustom. + deactivate: () => void; + + constructor() { + super(); + this.deactivate = () => { + if (this.active) { + this.active = false; + } + }; + } + + onEnterDOM() { + super.onEnterDOM(); + document.addEventListener("mouseup", this.deactivate); + document.addEventListener("touchend", this.deactivate); + } + + onExitDOM() { + super.onExitDOM(); + document.removeEventListener("mouseup", this.deactivate); + document.removeEventListener("touchend", this.deactivate); + } + /** * Defines the text of the component. * @@ -103,9 +127,20 @@ class Option extends ListItemBase implements IOption { return !!this.icon; } + /** + * Indicates if the option is active (pressed down). + * @private + */ + @property({ type: Boolean }) + active = false; + get effectiveDisplayText() { return this.textContent || ""; } + + _onmousedown() { + this.active = true; + } } Option.define(); diff --git a/packages/main/src/OptionCustom.ts b/packages/main/src/OptionCustom.ts index daedc153314b..81e1aaaa836e 100644 --- a/packages/main/src/OptionCustom.ts +++ b/packages/main/src/OptionCustom.ts @@ -40,6 +40,30 @@ import type { DefaultSlot } from "@ui5/webcomponents-base/dist/UI5Element.js"; class OptionCustom extends ListItemBase implements IOption { eventDetails!: ListItemBase["eventDetails"]; + // Note: same active state logic exists in Option. + deactivate: () => void; + + constructor() { + super(); + this.deactivate = () => { + if (this.active) { + this.active = false; + } + }; + } + + onEnterDOM() { + super.onEnterDOM(); + document.addEventListener("mouseup", this.deactivate); + document.addEventListener("touchend", this.deactivate); + } + + onExitDOM() { + super.onExitDOM(); + document.removeEventListener("mouseup", this.deactivate); + document.removeEventListener("touchend", this.deactivate); + } + /** * Defines the text, displayed inside the `ui5-select` input filed * when the option gets selected. @@ -84,6 +108,17 @@ class OptionCustom extends ListItemBase implements IOption { get effectiveDisplayText() { return this.displayText || this.textContent || ""; } + + /** + * Indicates if the option is active (pressed down). + * @private + */ + @property({ type: Boolean }) + active = false; + + _onmousedown() { + this.active = true; + } } OptionCustom.define(); diff --git a/packages/main/src/OptionCustomTemplate.tsx b/packages/main/src/OptionCustomTemplate.tsx index 04ff585299f7..af577565825c 100644 --- a/packages/main/src/OptionCustomTemplate.tsx +++ b/packages/main/src/OptionCustomTemplate.tsx @@ -2,7 +2,12 @@ import ListItemBaseTemplate from "./ListItemBaseTemplate.js"; import type OptionCustom from "./OptionCustom.js"; export default function OptionCustomTemplate(this: OptionCustom) { - return ListItemBaseTemplate.call(this, { listItemContent }, { role: "option", title: this.tooltip }); + return ListItemBaseTemplate.call(this, { listItemContent }, { + role: "option", + title: this.tooltip, + onMouseDown: this._onmousedown, + onTouchStart: this._onmousedown, + }); } function listItemContent(this: OptionCustom) { diff --git a/packages/main/src/OptionTemplate.tsx b/packages/main/src/OptionTemplate.tsx index 56bf7e404af7..916e7bdffb57 100644 --- a/packages/main/src/OptionTemplate.tsx +++ b/packages/main/src/OptionTemplate.tsx @@ -3,7 +3,12 @@ import ListItemBaseTemplate from "./ListItemBaseTemplate.js"; import type Option from "./Option.js"; export default function OptionTemplate(this: Option) { - return ListItemBaseTemplate.call(this, { listItemContent }, { role: "option", title: this.tooltip }); + return ListItemBaseTemplate.call(this, { listItemContent }, { + role: "option", + title: this.tooltip, + onMouseDown: this._onmousedown, + onTouchStart: this._onmousedown, + }); } function listItemContent(this: Option) { diff --git a/packages/main/src/themes/ListItemBase.css b/packages/main/src/themes/ListItemBase.css index 832c8d170177..cbe92d6cdea1 100644 --- a/packages/main/src/themes/ListItemBase.css +++ b/packages/main/src/themes/ListItemBase.css @@ -51,6 +51,7 @@ :host([active][actionable]:not([data-moving])), :host([active][actionable][selected]:not([data-moving])) { background-color: var(--sapList_Active_Background); + border-bottom-color: var(--sapList_Active_Background); } /* focused */ From 2cff725cb824ea7ae1662acea0ff974f2ec7b477 Mon Sep 17 00:00:00 2001 From: Nikola Anachkov Date: Thu, 30 Apr 2026 14:50:27 +0300 Subject: [PATCH 2/2] fix: address comments --- packages/main/cypress/specs/Select.cy.tsx | 22 +++++++++----- packages/main/src/ListItemBaseTemplate.tsx | 4 --- packages/main/src/Option.ts | 35 ---------------------- packages/main/src/OptionCustom.ts | 35 ---------------------- packages/main/src/OptionCustomTemplate.tsx | 7 +---- packages/main/src/OptionTemplate.tsx | 7 +---- packages/main/src/themes/ListItemBase.css | 6 ++-- packages/main/src/themes/OptionBase.css | 6 ++++ 8 files changed, 25 insertions(+), 97 deletions(-) diff --git a/packages/main/cypress/specs/Select.cy.tsx b/packages/main/cypress/specs/Select.cy.tsx index 264655ccaf05..aeaff395f997 100644 --- a/packages/main/cypress/specs/Select.cy.tsx +++ b/packages/main/cypress/specs/Select.cy.tsx @@ -1915,7 +1915,7 @@ describe("Select general interaction", () => { }); describe("Select - active/down state", () => { - it("sets active attribute on ui5-option while mouse is pressed", () => { + it("applies active background on ui5-option while mouse is pressed", () => { cy.mount( Option 1 @@ -1954,12 +1957,15 @@ describe("Select - active/down state", () => { .find("[ui5-option-custom]") .eq(0) .realMouseDown() - .should("have.attr", "active"); + .then($option => { + const bg = window.getComputedStyle($option[0]).backgroundColor; + expect(bg).not.to.equal("rgba(0, 0, 0, 0)"); + expect(bg).not.to.equal("transparent"); + }); cy.get("[ui5-select]") .find("[ui5-option-custom]") .eq(0) - .realMouseUp() - .should("not.have.attr", "active"); + .realMouseUp(); }); }); diff --git a/packages/main/src/ListItemBaseTemplate.tsx b/packages/main/src/ListItemBaseTemplate.tsx index fcca79d61ede..93726916589a 100644 --- a/packages/main/src/ListItemBaseTemplate.tsx +++ b/packages/main/src/ListItemBaseTemplate.tsx @@ -4,8 +4,6 @@ import type { AriaRole, JsxTemplate } from "@ui5/webcomponents-base/"; export default function ListItemBaseTemplate(this: ListItemBase, hooks?: { listItemContent: JsxTemplate }, injectedProps?: { role?: AriaRole, title?: string, - onMouseDown?: (e: MouseEvent) => void, - onTouchStart?: (e: TouchEvent) => void, }) { const listItemContent = hooks?.listItemContent || defaultListItemContent; @@ -22,8 +20,6 @@ export default function ListItemBaseTemplate(this: ListItemBase, hooks?: { listI onKeyUp={this._onkeyup} onKeyDown={this._onkeydown} onClick={this._onclick} - onMouseDown={injectedProps?.onMouseDown} - onTouchStart={injectedProps?.onTouchStart} > { listItemContent.call(this) as JSX.Element } diff --git a/packages/main/src/Option.ts b/packages/main/src/Option.ts index 0c5d8066be85..6cd5c44cdfc2 100644 --- a/packages/main/src/Option.ts +++ b/packages/main/src/Option.ts @@ -41,30 +41,6 @@ import type { DefaultSlot } from "@ui5/webcomponents-base/dist/UI5Element.js"; class Option extends ListItemBase implements IOption { eventDetails!: ListItemBase["eventDetails"]; - // Note: same active state logic exists in OptionCustom. - deactivate: () => void; - - constructor() { - super(); - this.deactivate = () => { - if (this.active) { - this.active = false; - } - }; - } - - onEnterDOM() { - super.onEnterDOM(); - document.addEventListener("mouseup", this.deactivate); - document.addEventListener("touchend", this.deactivate); - } - - onExitDOM() { - super.onExitDOM(); - document.removeEventListener("mouseup", this.deactivate); - document.removeEventListener("touchend", this.deactivate); - } - /** * Defines the text of the component. * @@ -127,20 +103,9 @@ class Option extends ListItemBase implements IOption { return !!this.icon; } - /** - * Indicates if the option is active (pressed down). - * @private - */ - @property({ type: Boolean }) - active = false; - get effectiveDisplayText() { return this.textContent || ""; } - - _onmousedown() { - this.active = true; - } } Option.define(); diff --git a/packages/main/src/OptionCustom.ts b/packages/main/src/OptionCustom.ts index 81e1aaaa836e..daedc153314b 100644 --- a/packages/main/src/OptionCustom.ts +++ b/packages/main/src/OptionCustom.ts @@ -40,30 +40,6 @@ import type { DefaultSlot } from "@ui5/webcomponents-base/dist/UI5Element.js"; class OptionCustom extends ListItemBase implements IOption { eventDetails!: ListItemBase["eventDetails"]; - // Note: same active state logic exists in Option. - deactivate: () => void; - - constructor() { - super(); - this.deactivate = () => { - if (this.active) { - this.active = false; - } - }; - } - - onEnterDOM() { - super.onEnterDOM(); - document.addEventListener("mouseup", this.deactivate); - document.addEventListener("touchend", this.deactivate); - } - - onExitDOM() { - super.onExitDOM(); - document.removeEventListener("mouseup", this.deactivate); - document.removeEventListener("touchend", this.deactivate); - } - /** * Defines the text, displayed inside the `ui5-select` input filed * when the option gets selected. @@ -108,17 +84,6 @@ class OptionCustom extends ListItemBase implements IOption { get effectiveDisplayText() { return this.displayText || this.textContent || ""; } - - /** - * Indicates if the option is active (pressed down). - * @private - */ - @property({ type: Boolean }) - active = false; - - _onmousedown() { - this.active = true; - } } OptionCustom.define(); diff --git a/packages/main/src/OptionCustomTemplate.tsx b/packages/main/src/OptionCustomTemplate.tsx index af577565825c..04ff585299f7 100644 --- a/packages/main/src/OptionCustomTemplate.tsx +++ b/packages/main/src/OptionCustomTemplate.tsx @@ -2,12 +2,7 @@ import ListItemBaseTemplate from "./ListItemBaseTemplate.js"; import type OptionCustom from "./OptionCustom.js"; export default function OptionCustomTemplate(this: OptionCustom) { - return ListItemBaseTemplate.call(this, { listItemContent }, { - role: "option", - title: this.tooltip, - onMouseDown: this._onmousedown, - onTouchStart: this._onmousedown, - }); + return ListItemBaseTemplate.call(this, { listItemContent }, { role: "option", title: this.tooltip }); } function listItemContent(this: OptionCustom) { diff --git a/packages/main/src/OptionTemplate.tsx b/packages/main/src/OptionTemplate.tsx index 916e7bdffb57..56bf7e404af7 100644 --- a/packages/main/src/OptionTemplate.tsx +++ b/packages/main/src/OptionTemplate.tsx @@ -3,12 +3,7 @@ import ListItemBaseTemplate from "./ListItemBaseTemplate.js"; import type Option from "./Option.js"; export default function OptionTemplate(this: Option) { - return ListItemBaseTemplate.call(this, { listItemContent }, { - role: "option", - title: this.tooltip, - onMouseDown: this._onmousedown, - onTouchStart: this._onmousedown, - }); + return ListItemBaseTemplate.call(this, { listItemContent }, { role: "option", title: this.tooltip }); } function listItemContent(this: Option) { diff --git a/packages/main/src/themes/ListItemBase.css b/packages/main/src/themes/ListItemBase.css index cbe92d6cdea1..1ef16a18cc19 100644 --- a/packages/main/src/themes/ListItemBase.css +++ b/packages/main/src/themes/ListItemBase.css @@ -34,16 +34,16 @@ } /* hovered */ -:host([actionable]:not([active]):not([selected]):not([ui5-li-group-header]):hover) { +:host([actionable]:not([active]):not(:active):not([selected]):not([ui5-li-group-header]):hover) { background-color: var(--sapList_Hover_Background); } -:host([actionable]:not([active]):not([selected]):not([ui5-li-group-header]):hover) .ui5-li-additional-text { +:host([actionable]:not([active]):not(:active):not([selected]):not([ui5-li-group-header]):hover) .ui5-li-additional-text { text-shadow: var(--sapContent_TextShadow); } /* selected and hovered */ -:host([actionable][selected]:not([active], [data-moving]):hover) { +:host([actionable][selected]:not([active]):not(:active):not([data-moving]):hover) { background-color: var(--sapList_Hover_SelectionBackground); } diff --git a/packages/main/src/themes/OptionBase.css b/packages/main/src/themes/OptionBase.css index 227ba508a016..c4d88a9d6bbe 100644 --- a/packages/main/src/themes/OptionBase.css +++ b/packages/main/src/themes/OptionBase.css @@ -1,4 +1,10 @@ :host { height: var(--_ui5_list_item_dropdown_base_height); --_ui5_list_item_title_size: var(--sapFontSize); +} + +:host(:active[actionable]:not([data-moving])), +:host(:active[actionable][selected]:not([data-moving])) { + background-color: var(--sapList_Active_Background); + border-bottom-color: var(--sapList_Active_Background); } \ No newline at end of file