Skip to content

Commit aac7449

Browse files
authored
fix(ui5-split-button): fire click event when focused with shift + tab (#11197)
### Main Problem There was an issue, where if the `ui5-split-button` was to be focused by using `Shift + Tab` combination, the `click` event wasn't firing, after the first time a default action was executed (pressed `Enter` or `Space`). The reason behind this, was a flag called `_shiftOrEscapePressed`. Since the component has a handler for `focus-in` event which was just early returning if the component `matches` the `:focus-within` CSS selector, which was causing the flag to NOT reset. Therefore the component wasn't firing `click` event, due to the condition: ```ts _fireClick(e?: Event) { e?.stopPropagation(); if (!this._shiftOrEscapePressed) { this.fireDecoratorEvent("click"); } this._shiftOrEscapePressed = false; } ``` ### Solutions - We solved few issues, by introducing a new flag called `_shiftOrEscapePressedDuringSpace`, which handles the cases only when `Shift` or `Escape` keys are pressed simultaneously with the `Space` Bar. - Deleted an unused forgotten flag; - Handled an issue where the arrow button was being 'frozen' if simultaneously pressing `ArrowUp/Down` keys and `Tab/Shift + Tab`. - `Shift/Escape` keys now correctly prevent events from firing; - Fixed a bug where `arrow-button` fired `click` **AND** `arrow-click` events.
1 parent 0b3a47b commit aac7449

File tree

2 files changed

+45
-77
lines changed

2 files changed

+45
-77
lines changed

packages/main/src/SplitButton.ts

Lines changed: 45 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -165,20 +165,12 @@ class SplitButton extends UI5Element {
165165
_tabIndex = 0
166166

167167
/**
168-
* Indicates if there is Space key pressed
168+
* Indicates if there is Shift or Escape key pressed while Space key is down.
169169
* @default false
170170
* @private
171171
*/
172172
@property({ type: Boolean, noAttribute: true })
173-
_spacePressed = false;
174-
175-
/**
176-
* Indicates if there is SHIFT or ESCAPE key pressed
177-
* @default false
178-
* @private
179-
*/
180-
@property({ type: Boolean, noAttribute: true })
181-
_shiftOrEscapePressed = false;
173+
_shiftOrEscapePressedDuringSpace = false;
182174

183175
/**
184176
* Defines the active state of the text button
@@ -227,9 +219,6 @@ class SplitButton extends UI5Element {
227219
@slot({ type: Node, "default": true })
228220
text!: Array<Node>;
229221

230-
_isDefaultActionPressed = false;
231-
_isKeyDownOperation = false;
232-
233222
@i18n("@ui5/webcomponents")
234223
static i18nBundle: I18nBundle;
235224

@@ -248,17 +237,10 @@ class SplitButton extends UI5Element {
248237
return;
249238
}
250239

251-
this._shiftOrEscapePressed = false;
240+
this._resetActionButtonStates();
252241
this._setTabIndexValue();
253242
}
254243

255-
_onFocusIn() {
256-
if (this.disabled || this.getFocusDomRef()!.matches(":has(:focus-within)")) {
257-
return;
258-
}
259-
this._shiftOrEscapePressed = false;
260-
}
261-
262244
handleTouchStart(e: TouchEvent | MouseEvent) {
263245
e.stopPropagation();
264246
this._textButtonActive = true;
@@ -273,59 +255,65 @@ class SplitButton extends UI5Element {
273255
}
274256

275257
_onKeyDown(e: KeyboardEvent) {
276-
this._isKeyDownOperation = true;
277258
if (this._isArrowKeyAction(e)) {
278259
this._handleArrowButtonAction(e);
279260
this._activeArrowButton = true;
280-
} else if (this._isDefaultAction(e)) {
261+
return;
262+
}
263+
264+
if (this._isDefaultAction(e)) {
281265
this._handleDefaultAction(e);
282-
this._isDefaultActionPressed = true;
266+
return;
283267
}
284268

285-
if (this._spacePressed && this._isShiftOrEscape(e)) {
286-
this._handleShiftOrEscapePressed();
269+
if ((isShift(e) || isEscape(e)) && this._textButtonActive) {
270+
e.preventDefault();
271+
this._shiftOrEscapePressedDuringSpace = true;
287272
}
288273

289-
// Handles button freeze issue when pressing Enter/Space and navigating with Tab/Shift+Tab simultaneously.
290-
if (this._isDefaultActionPressed && (isTabNext(e) || isTabPrevious(e))) {
291-
this._activeArrowButton = false;
292-
this._textButtonActive = false;
274+
if (isEscape(e) && !this._textButtonActive) {
275+
this._resetActionButtonStates();
293276
}
294277

295278
this._tabIndex = -1;
296279
}
297280

298281
_onKeyUp(e: KeyboardEvent) {
299-
this._isKeyDownOperation = false;
282+
const target = e.target as Button;
300283
if (this._isArrowKeyAction(e)) {
301284
e.preventDefault();
302285
this._activeArrowButton = false;
286+
return;
287+
}
288+
289+
if (isSpace(e)) {
290+
e.preventDefault();
291+
e.stopPropagation();
303292
this._textButtonActive = false;
304-
} else if (this._isDefaultAction(e)) {
305-
this._isDefaultActionPressed = false;
306-
this._textButtonActive = false;
307-
if (isSpace(e)) {
308-
e.preventDefault();
309-
e.stopPropagation();
293+
if (!this._shiftOrEscapePressedDuringSpace && target !== this.arrowButton) { // Do not fire click if Arrow button is focused by mouse and Space is pressed afterwards
310294
this._fireClick();
311-
this._spacePressed = false;
312-
this._textButtonActive = false;
313295
}
296+
297+
this._shiftOrEscapePressedDuringSpace = false;
298+
return;
314299
}
315300

316-
if (this._isShiftOrEscape(e)) {
317-
this._handleShiftOrEscapePressed();
301+
const shouldToggleTextButtonActiveStateOff = isEnter(e) || (isShift(e) && this._textButtonActive);
302+
303+
if (shouldToggleTextButtonActiveStateOff) {
304+
this._textButtonActive = false;
318305
}
306+
}
319307

320-
this._tabIndex = -1;
308+
_resetActionButtonStates() {
309+
this._activeArrowButton = false;
310+
this._textButtonActive = false;
311+
this._shiftOrEscapePressedDuringSpace = false;
321312
}
322313

323314
_fireClick(e?: Event) {
324315
e?.stopPropagation();
325-
if (!this._shiftOrEscapePressed) {
326-
this.fireDecoratorEvent("click");
327-
}
328-
this._shiftOrEscapePressed = false;
316+
this.fireDecoratorEvent("click");
329317
}
330318

331319
_fireArrowClick(e?: Event) {
@@ -383,15 +371,6 @@ class SplitButton extends UI5Element {
383371
return isSpace(e) || isEnter(e);
384372
}
385373

386-
/**
387-
* Checks if the pressed key is an escape key or shift key.
388-
* @param e - keyboard event
389-
* @private
390-
*/
391-
_isShiftOrEscape(e: KeyboardEvent): boolean {
392-
return isEscape(e) || isShift(e);
393-
}
394-
395374
/**
396375
* Handles the click event and the focus on the arrow button.
397376
* @param e - keyboard event
@@ -401,10 +380,6 @@ class SplitButton extends UI5Element {
401380
e.preventDefault();
402381

403382
this._fireArrowClick(e);
404-
405-
if (isSpace((e as KeyboardEvent))) {
406-
this._spacePressed = true;
407-
}
408383
}
409384

410385
/**
@@ -414,30 +389,24 @@ class SplitButton extends UI5Element {
414389
*/
415390
_handleDefaultAction(e: KeyboardEvent) {
416391
e.preventDefault();
417-
const wasSpacePressed = isSpace(e);
418392
const target = e.target as Button;
419393

420394
if (this.arrowButton && target === this.arrowButton) {
421395
this._activeArrowButton = true;
422396
this._fireArrowClick();
423-
if (wasSpacePressed) {
424-
this._spacePressed = true;
425-
this._textButtonActive = false;
426-
}
427-
} else {
428-
this._textButtonActive = true;
429-
if (wasSpacePressed) {
430-
this._spacePressed = true;
431-
return;
432-
}
433-
this._fireClick();
397+
return;
434398
}
435-
}
436399

437-
_handleShiftOrEscapePressed() {
438-
this._shiftOrEscapePressed = true;
439-
this._textButtonActive = false;
440-
this._isKeyDownOperation = false;
400+
this._textButtonActive = true;
401+
402+
if (isEnter(e)) {
403+
this._fireClick(e);
404+
return;
405+
}
406+
407+
if (isTabPrevious(e) || isTabNext(e)) {
408+
this._resetActionButtonStates();
409+
}
441410
}
442411

443412
get effectiveActiveArrowButton() {

packages/main/src/SplitButtonTemplate.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ export default function SplitButtonTemplate(this: SplitButton) {
1010
tabindex={this._tabIndex}
1111
aria-labelledby={`${this._id}-invisibleTextDefault ${this._id}}-invisibleText`}
1212
onFocusOut={this._onFocusOut}
13-
onFocusIn={this._onFocusIn}
1413
onKeyDown={this._onKeyDown}
1514
onKeyUp={this._onKeyUp}
1615
>

0 commit comments

Comments
 (0)