feat(ui5-split-button): address code review changes

packages/main/src/Button.ts

@@ -101,6 +101,11 @@ let activeButton: Button | null = null;
  * @native
  */
 @event("click")
+/**
+ * Fired whenever the active state of the component changes.
+ * @private
+ */
+@event("_active-state-change")
 class Button extends UI5Element implements IFormElement {
 	/**
 	 * Defines the component design.
@@ -313,13 +318,6 @@ class Button extends UI5Element implements IFormElement {
 	@property({ type: Boolean })
 	_isTouch!: boolean;
 
-	/**
-	 * Defines whether to keep the active state or not.
-	 * @protected
-	 */
-	@property({ type: Boolean, noAttribute: true })
-	_keepActiveState!: boolean;
-
 	/**
 	 * Defines the text of the component.
 	 * <br><br>
@@ -343,8 +341,8 @@ class Button extends UI5Element implements IFormElement {
 		super();
 
 		this._deactivate = () => {
-			if (activeButton && !this._keepActiveState) {
-				activeButton.active = false;
+			if (activeButton) {
+				activeButton._setActiveState(false);
 			}
 		};
 
@@ -361,7 +359,7 @@ class Button extends UI5Element implements IFormElement {
 				return;
 			}
 
-			this.active = true;
+			this._setActiveState(true);
 		};
 
 		this._ontouchstart = {
@@ -389,12 +387,6 @@ class Button extends UI5Element implements IFormElement {
 		this.buttonTitle = this.tooltip || await getIconAccessibleName(this.icon);
 	}
 
-	onAfterRendering() {
-		if (this._keepActiveState) {
-			this.active = true;
-		}
-	}
-
 	_onclick(e: MouseEvent) {
 		if (this.nonInteractive) {
 			return;
@@ -420,7 +412,7 @@ class Button extends UI5Element implements IFormElement {
 		}
 
 		markEvent(e, "button");
-		this.active = true;
+		this._setActiveState(true);
 		activeButton = this; // eslint-disable-line
 	}
 
@@ -430,12 +422,12 @@ class Button extends UI5Element implements IFormElement {
 			e.stopPropagation();
 		}
 
-		if (this.active && !this._keepActiveState) {
-			this.active = false;
+		if (this.active) {
+			this._setActiveState(false);
 		}
 
 		if (activeButton) {
-			activeButton.active = false;
+			activeButton._setActiveState(false);
 		}
 	}
 
@@ -447,14 +439,14 @@ class Button extends UI5Element implements IFormElement {
 		markEvent(e, "button");
 
 		if (isSpace(e) || isEnter(e)) {
-			this.active = true;
+			this._setActiveState(true);
 		}
 	}
 
 	_onkeyup(e: KeyboardEvent) {
 		if (isSpace(e) || isEnter(e)) {
-			if (this.active && !this._keepActiveState) {
-				this.active = false;
+			if (this.active) {
+				this._setActiveState(false);
 			}
 		}
 	}
@@ -464,8 +456,8 @@ class Button extends UI5Element implements IFormElement {
 			return;
 		}
 
-		if (this.active && !this._keepActiveState) {
-			this.active = false;
+		if (this.active) {
+			this._setActiveState(false);
 		}
 
 		if (isDesktop()) {
@@ -484,6 +476,16 @@ class Button extends UI5Element implements IFormElement {
 		}
 	}
 
+	_setActiveState(active: boolean) {
+		const eventPrevented = !this.fireEvent("_active-state-change", null, true);
+
+		if (eventPrevented) {
+			return;
+		}
+
+		this.active = active;
+	}
+
 	get hasButtonType() {
 		return this.design !== ButtonDesign.Default && this.design !== ButtonDesign.Transparent;
 	}

packages/main/src/SplitButton.hbs

@@ -39,13 +39,14 @@
 			icon="slim-arrow-down"
 			tabindex="-1"
 			?disabled="{{disabled}}"
-			?active="{{_activeArrowButton}}"
+			?active="{{effectiveActiveArrowButton}}"
 			aria-expanded="{{accessibilityInfo.ariaExpanded}}"
 			aria-haspopup="{{accessibilityInfo.ariaHaspopup}}"
 			@click="{{_handleArrowButtonAction}}"
 			@mousedown={{_arrowButtonPress}}
 			@mouseup={{_arrowButtonRelease}}
 			@focusin={{_setTabIndexValue}}
+			@ui5-_active-state-change={{_onArrowButtonActiveStateChange}}
 		>
 		</ui5-button>
 	</div>

packages/main/src/SplitButton.ts

@@ -141,7 +141,7 @@ class SplitButton extends UI5Element {
 	 * @name sap.ui.webc.main.SplitButton.prototype.activeArrowButton
 	 * @defaultvalue false
 	 * @public
-	 * @since 1.19.0
+	 * @since 1.20.0
 	 */
 	@property({ type: Boolean })
 	activeArrowButton!: boolean;
@@ -311,13 +311,6 @@ class SplitButton extends UI5Element {
 		}
 	}
 
-	onAfterRendering() {
-		if (this._isKeyDownOperation && !this.activeArrowButton) {
-			return;
-		}
-		this._setEffectiveActiveState();
-	}
-
 	_handleMouseClick(e: MouseEvent) {
 		const target = e.target as Button;
 
@@ -439,9 +432,10 @@ class SplitButton extends UI5Element {
 		}
 	}
 
-	_setEffectiveActiveState() {
-		this._activeArrowButton = !!this.activeArrowButton;
-		this.arrowButton!._keepActiveState = !!this.activeArrowButton;
+	_onArrowButtonActiveStateChange(e: CustomEvent) {
+		if (this.activeArrowButton) {
+			e.preventDefault();
+		}
 	}
 
 	/**
@@ -523,6 +517,10 @@ class SplitButton extends UI5Element {
 		this._isKeyDownOperation = false;
 	}
 
+	get effectiveActiveArrowButton() {
+		return this.activeArrowButton || this._activeArrowButton;
+	}
+
 	get textButtonAccText() {
 		return this.textContent;
 	}

packages/main/src/themes/SplitButton.css

@@ -37,6 +37,7 @@
 }
 
 :host([active-arrow-button][design="Transparent"]) .ui5-split-arrow-button,
+:host([active-arrow-button]) .ui5-split-arrow-button,
 :host([design="Transparent"]) .ui5-split-arrow-button[active],
 :host([design="Default"]) .ui5-split-arrow-button[active],
 .ui5-split-arrow-button[active], .ui5-split-arrow-button[active]:hover {

packages/main/src/themes/sap_horizon_dark/SplitButton-parameters.css

@@ -4,7 +4,6 @@
 	--_ui5_split_button_focused_border: 0.125rem solid var(--sapContent_FocusColor);
 	--_ui5_split_button_focused_border_radius: 0.375rem;
 	--_ui5_split_button_hover_border_radius: var(--_ui5_button_border_radius);
-	--_ui5_split_button_attention_separator_color: var(--sapButton_Attention_TextColor);
 	--_ui5_split_button_middle_separator_width: 0;
 	--_ui5_split_button_middle_separator_left: -0.0625rem;
 	--_ui5_split_button_middle_separator_hover_display: none;

packages/main/src/themes/sap_horizon_dark_exp/SplitButton-parameters.css

@@ -4,7 +4,6 @@
 	--_ui5_split_button_focused_border: 0.125rem solid var(--sapContent_FocusColor);
 	--_ui5_split_button_focused_border_radius: 0.375rem;
 	--_ui5_split_button_hover_border_radius: var(--_ui5_button_border_radius);
-	--_ui5_split_button_attention_separator_color: var(--sapButton_Attention_TextColor);
 	--_ui5_split_button_middle_separator_width: 0;
 	--_ui5_split_button_middle_separator_left: -0.0625rem;
 	--_ui5_split_button_middle_separator_hover_display: none;

packages/main/src/themes/sap_horizon_exp/SplitButton-parameters.css

@@ -4,7 +4,6 @@
 	--_ui5_split_button_focused_border: 0.125rem solid var(--sapContent_FocusColor);
 	--_ui5_split_button_focused_border_radius: 0.375rem;
 	--_ui5_split_button_hover_border_radius: var(--_ui5_button_border_radius);
-	--_ui5_split_button_attention_separator_color: var(--sapButton_Attention_TextColor);
 	--_ui5_split_button_middle_separator_width: 0;
 	--_ui5_split_button_middle_separator_left: -0.0625rem;
 	--_ui5_split_button_middle_separator_hover_display: none;

packages/main/test/pages/SplitButton.html

@@ -71,7 +71,7 @@ <h3>Test textContent</h3>
 	<ui5-split-button id="emptySpBtn" design="Default"></ui5-split-button>
 	<ui5-split-button id="defaultSpBtn" design="Default">Default</ui5-split-button>
 
-	<ui5-split-button id="splitBtnWithMenuDefaultActionDefaultAction" design="Emphasized">openMenu</ui5-split-button>
+	<ui5-split-button id="splitBtnWithMenuDefaultActionDefaultAction">openMenu</ui5-split-button>
 	<ui5-menu id="menu">
 		<ui5-menu-item text="New File" accessible-name="Opens a file explorer" additional-text="Ctrl+Alt+Shift+N" icon="add-document"></ui5-menu-item>
 		<ui5-menu-item text="New Folder with very long title for a menu item" additional-text="Ctrl+F" icon="add-folder" disabled></ui5-menu-item>