Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

fix(dropdown): applied roving tabindex technique to selected dropdown items #1004

Merged
merged 10 commits into from
Mar 5, 2019

Conversation

silviuaavram
Copy link
Collaborator

Without it, when in a Popup and a selected item is focused, shift tab will make focus escape the Popup, as FocusTrapZone will not handle it as first focusable item in the Popup (due to the lack of tabindex="0").

Also it does not hurt to be able to come back to the selected item via tab/shift tab once you tabbed/shift tabbed away.

@@ -90,7 +90,10 @@ class DropdownSelectedItem extends UIComponent<ReactProps<DropdownSelectedItemPr

componentDidUpdate(prevProps: DropdownSelectedItemProps) {
if (!prevProps.active && this.props.active) {
this.itemRef.current.setAttribute('tabindex', '0')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We definitely should start extracting behavior for the Dropdown component, we have too much coupling of accessibility concerns in the component... That way we would test this things separately.

Copy link
Contributor

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, just update the changelog, and let's start with extracting these things to behavior

@codecov
Copy link

codecov bot commented Mar 1, 2019

Codecov Report

Merging #1004 into master will increase coverage by 0.12%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1004      +/-   ##
=========================================
+ Coverage   81.37%   81.5%   +0.12%     
=========================================
  Files         673     673              
  Lines        8699    8700       +1     
  Branches     1475    1476       +1     
=========================================
+ Hits         7079    7091      +12     
+ Misses       1605    1594      -11     
  Partials       15      15
Impacted Files Coverage Δ
...t/src/components/Dropdown/DropdownSelectedItem.tsx 69.76% <100%> (+26.91%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 19dfbe3...7aae40f. Read the comment docs.

Copy link
Member

@layershifter layershifter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What it is the reason to use DOM API?


<Label tabIndex={active ? 0 : -1} />

https://codesandbox.io/s/wnnlqm80w8?module=/example.js

@silviuaavram silviuaavram reopened this Mar 5, 2019
@silviuaavram silviuaavram merged commit 12d5e4a into master Mar 5, 2019
@silviuaavram silviuaavram deleted the fix-selected-dropdown-item-navigation branch March 5, 2019 13:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants