search

microsoft / fluentui

Fluent UI web represents a collection of utilities, React components, and web components for building web applications.
https://react.fluentui.dev
Other
18.51k stars 2.73k forks source link

Rating Star component losing tabindex once tabbed out of it in readonly mode. #10126

Closed Hardhik closed 5 years ago

Hardhik commented 5 years ago

Environment Information

Describe the issue:

The Rating Star component in readonly mode, sets the tabindex = 0 for the rendered component. But when you are tabbing through it, after the first time it receives focus, the tabindex attribute is removed.

Please provide a reproduction of the issue in a codepen:

Can get a repro here: https://fabricweb.z5.web.core.windows.net/oufr/7.22.1/#/examples/rating

and also here: https://developer.microsoft.com/en-us/fabric/#/controls/web/rating

Steps to repro: Using keyboard and tabs get the focus to the "Usage" section examples, tab through the elements, observe that focus actually goes to the Stars under "Half star in readOnly mode:" the first time, but when you try to tab to it again, (shift + Tab / Tab), the focus doesn't go into the element. On inspecting the element, it is seen that the tabindex property gets removed for the component's FocusZone class element.

Check in gif: ratingstarfocus

Expected behavior:

The focus should actually be going to the Rating Star component in the first place, as it is a readonly component.

Documentation describing expected behavior

https://www.w3.org/WAI/WCAG21/Understanding/focus-order.html

In short "All interactive elements must be focusable and non-interactive elements must not be focusable"

1) In this case, the readonly rating star component is not interactive and should not be focusable to begin with. 2) The tabindex is removed once we focus out of the component.

Upon investigatign further, found that the code here: https://github.com/OfficeDev/office-ui-fabric-react/pull/7818/files#r252543706 is causing the tabindex to be removed.

There's two potential issues here: 1) The Rating Star component shouldn't be setting the tabindex as 0 for non interactive elements. (https://github.com/OfficeDev/office-ui-fabric-react/blob/a5bbc76367eb01e993f20796955569f422a538b2/packages/office-ui-fabric-react/src/components/Rating/Rating.base.tsx#L140) 2) The _setParkedFocus could be incorrectly removing the tabindex attribute for elements. (https://github.com/OfficeDev/office-ui-fabric-react/blob/0dde4ac50fcaee5bdfaa9a44c827e8396a16d02d/packages/office-ui-fabric-react/src/components/FocusZone/FocusZone.tsx#L398)

JasonGore commented 5 years ago

Thanks for the repro links and root cause analysis. It does appear to be a bug.

JasonGore commented 5 years ago

@Hardhik I have created #10625 to address the issue but with one difference regarding focusability. There's more detail in the PR. Let me know if you have any concerns or input. Thanks!

msft-github-bot commented 5 years ago

:tada:This issue was addressed in #10625, which has now been successfully released as office-ui-fabric-react@v7.41.0.:tada:

Handy links: