From 8dcbf7af1d500117b46e42077cb60dc8a389ac19 Mon Sep 17 00:00:00 2001 From: hinzzx Date: Wed, 26 Mar 2025 16:20:46 +0200 Subject: [PATCH 1/3] fix(ui5-split-button): fire click event when focused with shift + tab --- packages/main/src/SplitButton.ts | 127 ++++++++++------------ packages/main/src/SplitButtonTemplate.tsx | 1 - 2 files changed, 59 insertions(+), 69 deletions(-) diff --git a/packages/main/src/SplitButton.ts b/packages/main/src/SplitButton.ts index 713d4a823b2f..0aaa7aa6fb62 100644 --- a/packages/main/src/SplitButton.ts +++ b/packages/main/src/SplitButton.ts @@ -173,12 +173,12 @@ class SplitButton extends UI5Element { _spacePressed = false; /** - * Indicates if there is SHIFT or ESCAPE key pressed + * Indicates if there is Shift key pressed while Space key is down. * @default false * @private */ @property({ type: Boolean, noAttribute: true }) - _shiftOrEscapePressed = false; + _shiftOrEscapePressedDuringSpace = false; /** * Defines the active state of the text button @@ -227,9 +227,6 @@ class SplitButton extends UI5Element { @slot({ type: Node, "default": true }) text!: Array; - _isDefaultActionPressed = false; - _isKeyDownOperation = false; - @i18n("@ui5/webcomponents") static i18nBundle: I18nBundle; @@ -248,17 +245,10 @@ class SplitButton extends UI5Element { return; } - this._shiftOrEscapePressed = false; + this._resetActionButtonStates(); this._setTabIndexValue(); } - _onFocusIn() { - if (this.disabled || this.getFocusDomRef()!.matches(":has(:focus-within)")) { - return; - } - this._shiftOrEscapePressed = false; - } - handleTouchStart(e: TouchEvent | MouseEvent) { e.stopPropagation(); this._textButtonActive = true; @@ -273,59 +263,73 @@ class SplitButton extends UI5Element { } _onKeyDown(e: KeyboardEvent) { - this._isKeyDownOperation = true; if (this._isArrowKeyAction(e)) { this._handleArrowButtonAction(e); this._activeArrowButton = true; - } else if (this._isDefaultAction(e)) { + return; + } + + if (this._isDefaultAction(e)) { this._handleDefaultAction(e); - this._isDefaultActionPressed = true; + return; } - if (this._spacePressed && this._isShiftOrEscape(e)) { - this._handleShiftOrEscapePressed(); + if (isSpace(e)) { + this._spacePressed = true; + this._shiftOrEscapePressedDuringSpace = false; // Reset whenever new Space action starts + return; } - // Handles button freeze issue when pressing Enter/Space and navigating with Tab/Shift+Tab simultaneously. - if (this._isDefaultActionPressed && (isTabNext(e) || isTabPrevious(e))) { - this._activeArrowButton = false; - this._textButtonActive = false; + if ((isShift(e) || isEscape(e)) && this._spacePressed) { + e.preventDefault(); + this._shiftOrEscapePressedDuringSpace = true; + } + + if (isEscape(e)) { + this._resetActionButtonStates(); } this._tabIndex = -1; } _onKeyUp(e: KeyboardEvent) { - this._isKeyDownOperation = false; + const target = e.target as Button; if (this._isArrowKeyAction(e)) { e.preventDefault(); this._activeArrowButton = false; + return; + } + + if (isSpace(e)) { + e.preventDefault(); + e.stopPropagation(); this._textButtonActive = false; - } else if (this._isDefaultAction(e)) { - this._isDefaultActionPressed = false; - this._textButtonActive = false; - if (isSpace(e)) { - e.preventDefault(); - e.stopPropagation(); + if (!this._shiftOrEscapePressedDuringSpace && target !== this.arrowButton) { // Do not fire click if Arrow button is focused by mouse and Space is pressed afterwards this._fireClick(); - this._spacePressed = false; - this._textButtonActive = false; } + + this._spacePressed = false; + return; } - if (this._isShiftOrEscape(e)) { - this._handleShiftOrEscapePressed(); + if (isEnter(e)) { + this._textButtonActive = false; + return; } - this._tabIndex = -1; + if (isShift(e) && this._textButtonActive) { + this._textButtonActive = false; + } + } + + _resetActionButtonStates() { + this._activeArrowButton = false; + this._textButtonActive = false; } _fireClick(e?: Event) { e?.stopPropagation(); - if (!this._shiftOrEscapePressed) { - this.fireDecoratorEvent("click"); - } - this._shiftOrEscapePressed = false; + this.fireDecoratorEvent("click"); } _fireArrowClick(e?: Event) { @@ -383,15 +387,6 @@ class SplitButton extends UI5Element { return isSpace(e) || isEnter(e); } - /** - * Checks if the pressed key is an escape key or shift key. - * @param e - keyboard event - * @private - */ - _isShiftOrEscape(e: KeyboardEvent): boolean { - return isEscape(e) || isShift(e); - } - /** * Handles the click event and the focus on the arrow button. * @param e - keyboard event @@ -401,10 +396,6 @@ class SplitButton extends UI5Element { e.preventDefault(); this._fireArrowClick(e); - - if (isSpace((e as KeyboardEvent))) { - this._spacePressed = true; - } } /** @@ -414,30 +405,30 @@ class SplitButton extends UI5Element { */ _handleDefaultAction(e: KeyboardEvent) { e.preventDefault(); - const wasSpacePressed = isSpace(e); const target = e.target as Button; if (this.arrowButton && target === this.arrowButton) { this._activeArrowButton = true; this._fireArrowClick(); - if (wasSpacePressed) { - this._spacePressed = true; - this._textButtonActive = false; - } - } else { - this._textButtonActive = true; - if (wasSpacePressed) { - this._spacePressed = true; - return; - } - this._fireClick(); + return; } - } - _handleShiftOrEscapePressed() { - this._shiftOrEscapePressed = true; - this._textButtonActive = false; - this._isKeyDownOperation = false; + this._textButtonActive = true; + + if (isSpace(e)) { + this._spacePressed = true; + this._shiftOrEscapePressedDuringSpace = false; + return; + } + + if (isEnter(e)) { + this._fireClick(e); + return; + } + + if (isTabPrevious(e) || isTabNext(e)) { + this._resetActionButtonStates(); + } } get effectiveActiveArrowButton() { diff --git a/packages/main/src/SplitButtonTemplate.tsx b/packages/main/src/SplitButtonTemplate.tsx index 8284ced6c30a..7fcc27baaaa6 100644 --- a/packages/main/src/SplitButtonTemplate.tsx +++ b/packages/main/src/SplitButtonTemplate.tsx @@ -10,7 +10,6 @@ export default function SplitButtonTemplate(this: SplitButton) { tabindex={this._tabIndex} aria-labelledby={`${this._id}-invisibleTextDefault ${this._id}}-invisibleText`} onFocusOut={this._onFocusOut} - onFocusIn={this._onFocusIn} onKeyDown={this._onKeyDown} onKeyUp={this._onKeyUp} > From d3cc45947dce37cb2fb4d70606e3c22d5b7768cb Mon Sep 17 00:00:00 2001 From: hinzzx Date: Fri, 28 Mar 2025 09:35:22 +0200 Subject: [PATCH 2/3] chore: remove duplicate code --- packages/main/src/SplitButton.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/main/src/SplitButton.ts b/packages/main/src/SplitButton.ts index 0aaa7aa6fb62..40ccec45ba4e 100644 --- a/packages/main/src/SplitButton.ts +++ b/packages/main/src/SplitButton.ts @@ -274,12 +274,6 @@ class SplitButton extends UI5Element { return; } - if (isSpace(e)) { - this._spacePressed = true; - this._shiftOrEscapePressedDuringSpace = false; // Reset whenever new Space action starts - return; - } - if ((isShift(e) || isEscape(e)) && this._spacePressed) { e.preventDefault(); this._shiftOrEscapePressedDuringSpace = true; @@ -417,7 +411,7 @@ class SplitButton extends UI5Element { if (isSpace(e)) { this._spacePressed = true; - this._shiftOrEscapePressedDuringSpace = false; + this._shiftOrEscapePressedDuringSpace = false; // Reset whenever new Space action starts return; } From b05753ce8152cafb8bba934e1eece8bed1dc0aa8 Mon Sep 17 00:00:00 2001 From: Stoyan <88034608+hinzzx@users.noreply.github.com> Date: Wed, 16 Apr 2025 09:18:01 +0300 Subject: [PATCH 3/3] chore: add missing doc --- packages/main/src/SplitButton.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/main/src/SplitButton.ts b/packages/main/src/SplitButton.ts index 40ccec45ba4e..baccf9fdb2ef 100644 --- a/packages/main/src/SplitButton.ts +++ b/packages/main/src/SplitButton.ts @@ -173,7 +173,7 @@ class SplitButton extends UI5Element { _spacePressed = false; /** - * Indicates if there is Shift key pressed while Space key is down. + * Indicates if there is Shift or Escape key pressed while Space key is down. * @default false * @private */