Repository: laboratoriobridge/bold (ts)

Issue:
Options disabled are receiving focus at dropdowns
Disabled element should not receive focus because makes navigation more difficult.
Browser's VoiceOver extensions take cares of those elements so they can be accessible through keyboard and so read their content and tooltip

Pull request:
Fix #511 - Options disabled are receiving focus at dropdowns
closes #511 

Patch:
diff --git a/src/components/ButtonGroup/ButtonGroup.tsx b/src/components/ButtonGroup/ButtonGroup.tsx
index 08c6e46fe..25c62e25d 100644
--- a/src/components/ButtonGroup/ButtonGroup.tsx
+++ b/src/components/ButtonGroup/ButtonGroup.tsx
@@ -1,6 +1,5 @@
 import React, { CSSProperties } from 'react'
 
-import { useRovingTabIndex } from '../../hooks/useRovingTabIndex'
 import { ExternalStyles, useStyles } from '../../styles'
 import { Omit } from '../../util'
 
@@ -12,11 +11,7 @@ export function ButtonGroup(props: ButtonGroupProps) {
   const { style, ...rest } = props
   const { classes, css } = useStyles(createStyles)
 
-  const rootRef = useRovingTabIndex({
-    getItems: root => Array.from(root.querySelectorAll('button')),
-  })
-
-  return <div ref={rootRef} className={css(classes.root, style)} role='group' {...rest} />
+  return <div className={css(classes.root, style)} role='group' {...rest} />
 }
 
 export const createStyles = () => ({
diff --git a/src/components/Dropdown/DropdownMenu.tsx b/src/components/Dropdown/DropdownMenu.tsx
index a296d78bf..69e92c8be 100644
--- a/src/components/Dropdown/DropdownMenu.tsx
+++ b/src/components/Dropdown/DropdownMenu.tsx
@@ -15,7 +15,7 @@ export function DropdownMenu(props: DropdownMenuProps) {
   const { css, classes } = useStyles(styles)
 
   const rootRef = useRovingTabIndex({
-    getItems: root => Array.from(root.querySelectorAll('[role="menuitem"]')),
+    getItems: root => Array.from(root.querySelectorAll('[role="menuitem"]:not([aria-disabled="true"])')),
   })
 
   return <ul ref={composeRefs(innerRef, rootRef)} className={css(classes.root, style)} role='menu' {...rest} />
diff --git a/src/components/Tabs/Tabs.tsx b/src/components/Tabs/Tabs.tsx
index 7bd4c5714..cc21b5d42 100644
--- a/src/components/Tabs/Tabs.tsx
+++ b/src/components/Tabs/Tabs.tsx
@@ -13,7 +13,7 @@ export function Tabs(props: TabsProps) {
   const { classes, css } = useStyles(createStyles)
 
   const rootRef = useRovingTabIndex({
-    getItems: root => Array.from(root.querySelectorAll('[role="tab"]')),
+    getItems: root => Array.from(root.querySelectorAll(`[role="tab"]:not(.disabled)`)),
   })
 
   return <ul ref={rootRef} className={css(classes.ul, style)} role='tablist' {...rest} />


Test patch:
diff --git a/src/components/ButtonGroup/ButtonGroup.test.tsx b/src/components/ButtonGroup/ButtonGroup.test.tsx
index 499d67b7a..b3e815122 100644
--- a/src/components/ButtonGroup/ButtonGroup.test.tsx
+++ b/src/components/ButtonGroup/ButtonGroup.test.tsx
@@ -1,4 +1,4 @@
-import { fireEvent, render } from '@testing-library/react'
+import { render } from '@testing-library/react'
 import React from 'react'
 
 import { Button } from '../Button'
@@ -23,31 +23,3 @@ it('should should accept HTML div element props', () => {
   const { container } = render(<ButtonGroup id='test' />)
   expect(container.querySelector('div').getAttribute('id')).toEqual('test')
 })
-
-it('should have a roving tab index', () => {
-  const { container } = render(
-    <ButtonGroup>
-      <Button>First</Button>
-      <Button>Second</Button>
-      <Button>Third</Button>
-    </ButtonGroup>
-  )
-
-  const div = container.querySelector('div')
-  const buttons = container.querySelectorAll('button')
-  expect(buttons[0].getAttribute('tabindex')).toEqual('0')
-  expect(buttons[1].getAttribute('tabindex')).toEqual('-1')
-
-  buttons[1].focus()
-  expect(buttons[0].getAttribute('tabindex')).toEqual('-1')
-  expect(buttons[1].getAttribute('tabindex')).toEqual('0')
-  expect(buttons[2].getAttribute('tabindex')).toEqual('-1')
-
-  fireEvent.keyDown(div, { key: 'ArrowDown' })
-  expect(buttons[1].getAttribute('tabindex')).toEqual('-1')
-  expect(buttons[2].getAttribute('tabindex')).toEqual('0')
-
-  fireEvent.keyDown(div, { key: 'ArrowUp' })
-  expect(buttons[1].getAttribute('tabindex')).toEqual('0')
-  expect(buttons[2].getAttribute('tabindex')).toEqual('-1')
-})
diff --git a/src/components/ButtonGroup/__snapshots__/ButtonGroup.test.tsx.snap b/src/components/ButtonGroup/__snapshots__/ButtonGroup.test.tsx.snap
index cfb72d6e6..f0775032c 100644
--- a/src/components/ButtonGroup/__snapshots__/ButtonGroup.test.tsx.snap
+++ b/src/components/ButtonGroup/__snapshots__/ButtonGroup.test.tsx.snap
@@ -81,7 +81,6 @@ exports[`should render correctly 1`] = `
   >
     <button
       class="emotion-0"
-      tabindex="0"
       type="button"
     >
       <span>
diff --git a/src/components/Dropdown/DropdownMenu.test.tsx b/src/components/Dropdown/DropdownMenu.test.tsx
index c631a2c8e..bb9caa123 100644
--- a/src/components/Dropdown/DropdownMenu.test.tsx
+++ b/src/components/Dropdown/DropdownMenu.test.tsx
@@ -47,21 +47,19 @@ it('should have a roving tabIndex', () => {
   expect(li[1].tabIndex).toEqual(-1)
 
   fireEvent.keyDown(ul, { key: 'ArrowDown' })
-  expect(document.activeElement).toEqual(li[1])
+  expect(document.activeElement).toEqual(li[3])
   expect(li[0].tabIndex).toEqual(-1)
-  expect(li[1].tabIndex).toEqual(0)
+  expect(li[1].tabIndex).toEqual(-1)
   expect(li[2].tabIndex).toEqual(-1)
 
   fireEvent.keyDown(ul, { key: 'ArrowDown' })
-  expect(document.activeElement).toEqual(li[3])
+  expect(document.activeElement).toEqual(li[4])
   expect(li[0].tabIndex).toEqual(-1)
   expect(li[1].tabIndex).toEqual(-1)
   expect(li[2].tabIndex).toEqual(-1)
-  expect(li[3].tabIndex).toEqual(0)
-  expect(li[4].tabIndex).toEqual(-1)
+  expect(li[3].tabIndex).toEqual(-1)
+  expect(li[4].tabIndex).toEqual(0)
 
-  fireEvent.keyDown(ul, { key: 'ArrowDown' })
-  fireEvent.keyDown(ul, { key: 'ArrowDown' })
   fireEvent.keyDown(ul, { key: 'ArrowDown' })
   expect(document.activeElement).toEqual(li[4])
   expect(li[3].tabIndex).toEqual(-1)
diff --git a/src/components/Dropdown/__snapshots__/Dropdown.test.tsx.snap b/src/components/Dropdown/__snapshots__/Dropdown.test.tsx.snap
index 7e2363ad4..bace40d7c 100644
--- a/src/components/Dropdown/__snapshots__/Dropdown.test.tsx.snap
+++ b/src/components/Dropdown/__snapshots__/Dropdown.test.tsx.snap
@@ -113,7 +113,6 @@ exports[`should accept popper props 1`] = `
       aria-disabled="true"
       class="emotion-2"
       role="menuitem"
-      tabindex="-1"
     >
       Item #3
     </li>
@@ -236,7 +235,6 @@ exports[`should extend DropdownMenu props 1`] = `
       aria-disabled="true"
       class="emotion-2"
       role="menuitem"
-      tabindex="-1"
     >
       Item #3
     </li>
@@ -369,7 +367,6 @@ exports[`should render correctly when open 1`] = `
       aria-disabled="true"
       class="emotion-2"
       role="menuitem"
-      tabindex="-1"
     >
       Item #3
     </li>
diff --git a/src/components/Dropdown/__snapshots__/DropdownMenu.test.tsx.snap b/src/components/Dropdown/__snapshots__/DropdownMenu.test.tsx.snap
index f428d312f..17508dc81 100644
--- a/src/components/Dropdown/__snapshots__/DropdownMenu.test.tsx.snap
+++ b/src/components/Dropdown/__snapshots__/DropdownMenu.test.tsx.snap
@@ -156,7 +156,6 @@ exports[`should render correctly 1`] = `
       aria-disabled="true"
       class="emotion-1"
       role="menuitem"
-      tabindex="-1"
     >
       Disabled
     </li>
diff --git a/src/components/Tabs/Tabs.test.tsx b/src/components/Tabs/Tabs.test.tsx
index b35e21c24..988fd3a04 100644
--- a/src/components/Tabs/Tabs.test.tsx
+++ b/src/components/Tabs/Tabs.test.tsx
@@ -39,13 +39,11 @@ it('should have a roving tabindex', () => {
   expect(items[0].getAttribute('tabindex')).toEqual('-1')
   expect(items[1].getAttribute('tabindex')).toEqual('-1')
   expect(items[2].getAttribute('tabindex')).toEqual('0')
-  expect(items[3].getAttribute('tabindex')).toEqual('-1')
 
   fireEvent.keyDown(ul, { key: 'ArrowDown' })
-  expect(items[2].getAttribute('tabindex')).toEqual('-1')
-  expect(items[3].getAttribute('tabindex')).toEqual('0')
+  expect(items[2].getAttribute('tabindex')).toEqual('0')
 
   fireEvent.keyDown(ul, { key: 'ArrowUp' })
-  expect(items[2].getAttribute('tabindex')).toEqual('0')
-  expect(items[3].getAttribute('tabindex')).toEqual('-1')
+  expect(items[1].getAttribute('tabindex')).toEqual('0')
+  expect(items[2].getAttribute('tabindex')).toEqual('-1')
 })
diff --git a/src/components/Tabs/__snapshots__/TabItem.test.tsx.snap b/src/components/Tabs/__snapshots__/TabItem.test.tsx.snap
index f7fa96fca..45ed0fed4 100644
--- a/src/components/Tabs/__snapshots__/TabItem.test.tsx.snap
+++ b/src/components/Tabs/__snapshots__/TabItem.test.tsx.snap
@@ -162,7 +162,6 @@ exports[`should accept "component" prop 1`] = `
         class="emotion-6 disabled"
         href="/"
         role="tab"
-        tabindex="-1"
       >
         Disabled
       </a>
diff --git a/src/components/Tabs/__snapshots__/Tabs.test.tsx.snap b/src/components/Tabs/__snapshots__/Tabs.test.tsx.snap
index 435a52afe..056008370 100644
--- a/src/components/Tabs/__snapshots__/Tabs.test.tsx.snap
+++ b/src/components/Tabs/__snapshots__/Tabs.test.tsx.snap
@@ -158,7 +158,6 @@ exports[`renders correctly 1`] = `
       <span
         class="emotion-6 disabled"
         role="tab"
-        tabindex="-1"
       >
         Disabled
       </span>