fix(ui5-split-button): correct focus behavior #7320
Conversation
There was a problem hiding this comment.
I suggest to have a discussion with architects about this refactoring as it is easily possible to achieve a blocking behavior with the component initial implementation.
For example if we were to retrieve the focus to the root component element each time we press the arrow or the text button we'll receive a rerendering operation. Therefore if we press the text button and then the arrow button multiple times we'll receive a DOM blocking behavior.
The problem is that we do want to retrieve the focus to the root element for each individual button press.
| this._shiftOrEscapePressed = false; | ||
| this.focused = true; | ||
| } | ||
|
|
||
| _singleButtonFocusIn(e: FocusEvent) { | ||
| e.preventDefault(); |
There was a problem hiding this comment.
We could add:
this.focused = true;
as the focus should be maintained over the split button container, when we click over the text or the arrow button.
| @@ -43,7 +43,7 @@ | |||
| aria-expanded="{{accessibilityInfo.ariaExpanded}}" | |||
| aria-haspopup="{{accessibilityInfo.ariaHaspopup}}" | |||
| @click="{{_fireArrowClick}}" | |||
| @focusin={{_setTabIndexValue}} | |||
| @focusin={{_singleButtonFocusIn}} | |||
| @focusout={{_onFocusOut}} | |||
There was a problem hiding this comment.
Think that we shouldn't react on focus out of the arrow button as we do not expect to have it focused on the first place.
The same would apply for the text button.
| this._shiftOrEscapePressed = false; | ||
| this.focused = true; | ||
| } | ||
|
|
||
| _singleButtonFocusIn(e: FocusEvent) { |
There was a problem hiding this comment.
We could react in the _onFocusIn Instead of reacting on focus in for the text and arrow buttons separately.
That way we could remove that attached handlers for focusin and focusout for the arrow and text buttons.
In both cases we want to place the focus back to the container element.
WIP & Ongoing discussions
Fixes: #7281