fix(ui5-shellbar): enable items keyboard handling #1473
Conversation
ilhan007
changed the title
ilhan007
changed the title
ilhan007
changed the title
There was a problem hiding this comment.
I really like the simplification! Most of the tricky code is gone.
A couple of things I found:
- On IE the focus looks bad, a bit off and with the wrong color. However, on IE pressing space/enter activates the buttons so you really know you pressed something.
- On Chrome, buttons are not activated by pressing space/enter, even though they work. When you click with the mouse, they activate properly
- On all browsers: pressing the menu items button does not focus the first menu item when opening the popup. Also, when closing the popup, the focus does not go back to the button so that you can continue tabbing. The other menu (clicking the profile picuter) however works fine.
- When on any of the menus (menu items dropdown, or profile dropdown) pressing Tab does nothing. I assume it has to close the menu and go to the next button on the shellbar, but I might be wrong.
| @@ -35,6 +35,11 @@ | |||
| font-weight: bold; | |||
| } | |||
|
|
|||
| .ui5-shellbar-menu-button, | |||
There was a problem hiding this comment.
IE fails when selectors such as :host, ::slotted are mixed with other selectors.
Same with the IE CSS processing script.
On IE the button focus looks broken (black) maybe this is part of the reason?
Prior to my last commit it was Work in progress, now with the last commit: (1) The focus on IE should look fine (2) The buttons on Chrome and IE are activated with SPACE and ENTER. If a button opens a popover the ENTER seems not to change the background, but this is general issue (if it is issue) not related to this change, see Popover.html test page. (3) I will check why the first menu works differently, but it is the same in the master, not broken with this change (4) Again, but this is in general issue, see the Popover.html with a list inside its content, pressing TAB does not make anything, because someone implemented the focus to always remain in the popover. However, you can use ESC to close the popover and the focus will be brought back to the opener. |
There was a problem hiding this comment.
<button tabindex="0" class="{{classes.button}}" @click="{{_header.press}}">
This button, when inactive, still gets the focus and it looks like the focus is lost. Probably tabindex should be "-1" whenever the button is non-interactive.
On IE, for me the focus is still broken for the small buttons on the right:
This is the focus on master:
I know this is not due to your change, and it was broken before, but all the shellbar buttons do not get active state on space/enter, only on clicking with the mouse. I think it would be best to address this problem with this change (but I'm fine with doing this later, if you prefer so).
I look at: https://ui5.sap.com/#/entity/sap.f.ShellBar/sample/sap.f.sample.ShellBar
as a reference. There, the shellbar buttons have active state as I would expect.
The co-pilot button's focus is blue, should be white, as in the link above.
| @@ -34,42 +33,18 @@ | |||
|
|
|||
| <div class="ui5-shellbar-overflow-container ui5-shellbar-overflow-container-middle"> | |||
| {{#if showCoPilot}} | |||
| {{> coPilot}} | |||
| <div class="ui5-shellbar-copilot-wrapper" | |||
There was a problem hiding this comment.
When this is focused, the focus is blue and only seen on the sides, not on top/bottom. Is this expected?
Fixes
Background
All ui5-icon elements now become ui5-button, the ui5-button has tabindex=0 and also can be triggered with SPACE and ENTER. To ensure the correct tab order, the layout of the items is changed in such way that their DOM order is the same as their display order.
Fixes: #860