fix: address comments

Author: NakataCode

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(
 			<Select>
 				<Option>Option 1</Option>
@@ -1930,16 +1930,19 @@ describe("Select - active/down state", () => {
 			.find("[ui5-option]")
 			.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]")
 			.eq(0)
-			.realMouseUp()
-			.should("not.have.attr", "active");
+			.realMouseUp();
 	});
 
-	it("sets active attribute on ui5-option-custom while mouse is pressed", () => {
+	it("applies active background on ui5-option-custom while mouse is pressed", () => {
 		cy.mount(
 			<Select>
 				<OptionCustom>Option 1</OptionCustom>
@@ -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();
 	});
 });

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 }
 		</li>

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();

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();

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) {

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) {

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);
 }
 

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);
 }