fix(tab): tab order on radio and tabindex=-1 (#444)

Co-authored-by: marcosvega91 <marcomoretti0103@gmail.com>

src/__tests__/tab.js

@@ -244,6 +244,26 @@ test('should suport a mix of elements with/without tab index', () => {
   expect(radio).toHaveFocus()
 })
 
+test('ignore tabindex when active element has tabindex="-1"', () => {
+  const {element} = setup(`
+    <input tabindex='1'/>
+    <input tabindex='0'/>
+    <input tabindex='-1'/>
+    <input tabindex='2'/>
+  `)
+  const [inputA, inputB, inputC, inputD] = element.parentElement.children
+
+  inputB.focus()
+  userEvent.tab()
+
+  expect(inputA).toHaveFocus()
+
+  inputC.focus()
+  userEvent.tab()
+
+  expect(inputD).toHaveFocus()
+})
+
 test('should not tab to <a> with no href', () => {
   setup(`
     <div>
@@ -388,51 +408,102 @@ test('should keep focus on the document if there are no enabled, focusable eleme
   expect(document.body).toHaveFocus()
 })
 
-test('should respect radio groups', () => {
-  setup(`
-    <div>
-      <input
-        data-testid="element"
-        type="radio"
-        name="first"
-        value="first_left"
-      />
-      <input
-        data-testid="element"
-        type="radio"
-        name="first"
-        value="first_right"
-      />
-      <input
-        data-testid="element"
-        type="radio"
-        name="second"
-        value="second_left"
-      />
-      <input
-        data-testid="element"
-        type="radio"
-        name="second"
-        value="second_right"
-        checked
-      />
-    </div>`)
-
-  const [firstLeft, firstRight, , secondRight] = document.querySelectorAll(
-    '[data-testid="element"]',
-  )
+test('skip consecutive radios of same group', () => {
+  const {element} = setup(`
+    <input/>
+    <input type="radio" name="radio1"/>
+    <input type="radio" name="radio1"/>
+    <input/>
+    <input type="radio" name="radio1"/>
+    <input type="radio" name="radio2"/>
+    <input type="radio" name="radio2"/>
+    <input/>
+  `)
+  const [
+    inputA,
+    radioA,
+    radioB,
+    inputB,
+    radioC,
+    radioD,
+    radioE,
+    inputC,
+  ] = element.parentElement.children
+
+  inputA.focus()
 
+  userEvent.tab()
+  expect(radioA).toHaveFocus()
+  userEvent.tab()
+  expect(inputB).toHaveFocus()
+  userEvent.tab()
+  expect(radioC).toHaveFocus()
+  userEvent.tab()
+  expect(radioD).toHaveFocus()
   userEvent.tab()
 
-  expect(firstLeft).toHaveFocus()
+  expect(inputC).toHaveFocus()
 
+  userEvent.tab({shift: true})
+  expect(radioE).toHaveFocus()
+  userEvent.tab({shift: true})
+  expect(radioC).toHaveFocus()
+  userEvent.tab({shift: true})
+  expect(inputB).toHaveFocus()
+  userEvent.tab({shift: true})
+  expect(radioB).toHaveFocus()
+  userEvent.tab({shift: true})
+
+  expect(inputA).toHaveFocus()
+})
+
+test('skip unchecked radios if that group has a checked one', () => {
+  const {element} = setup(`
+    <input/>
+    <input type="radio" name="radio"/>
+    <input/>
+    <input type="radio" name="radio" checked/>
+    <input/>
+    <input type="radio" name="radio"/>
+    <input/>
+  `)
+  const [
+    inputA,
+    ,
+    inputB,
+    radioB,
+    inputC,
+    ,
+    inputD,
+  ] = element.parentElement.children
+
+  inputA.focus()
+
+  userEvent.tab()
+  expect(inputB).toHaveFocus()
+  userEvent.tab()
+  expect(radioB).toHaveFocus()
+  userEvent.tab()
+  expect(inputC).toHaveFocus()
   userEvent.tab()
+  expect(inputD).toHaveFocus()
+})
 
-  expect(secondRight).toHaveFocus()
+test('tab from active radio when another one is checked', () => {
+  const {element} = setup(`
+    <input/>
+    <input type="radio" name="radio" checked/>
+    <input/>
+    <input type="radio" name="radio"/>
+    <input/>
+  `)
+  const [, , , radioB, inputC] = element.parentElement.children
 
-  userEvent.tab({shift: true})
+  radioB.focus()
+
+  userEvent.tab()
 
-  expect(firstRight).toHaveFocus()
+  expect(inputC).toHaveFocus()
 })
 
 test('calls FocusEvents with relatedTarget', () => {

src/tab.js

@@ -29,14 +29,24 @@ function tab({shift = false, focusTrap} = {}) {
   const focusableElements = focusTrap.querySelectorAll(FOCUSABLE_SELECTOR)
 
   const enabledElements = [...focusableElements].filter(
-    el => el.getAttribute('tabindex') !== '-1' && !el.disabled,
+    el =>
+      el === previousElement ||
+      (el.getAttribute('tabindex') !== '-1' && !el.disabled),
   )
 
   if (enabledElements.length === 0) return
 
   const orderedElements = enabledElements
     .map((el, idx) => ({el, idx}))
     .sort((a, b) => {
+      // tabindex has no effect if the active element has tabindex="-1"
+      if (
+        previousElement &&
+        previousElement.getAttribute('tabindex') === '-1'
+      ) {
+        return a.idx - b.idx
+      }
+
       const tabIndexA = a.el.getAttribute('tabindex')
       const tabIndexB = b.el.getAttribute('tabindex')
 
@@ -46,33 +56,45 @@ function tab({shift = false, focusTrap} = {}) {
     })
     .map(({el}) => el)
 
-  if (shift) orderedElements.reverse()
-
-  // keep only the checked or first element in each radio group
-  const prunedElements = []
-  for (const el of orderedElements) {
+  const checkedRadio = {}
+  let prunedElements = []
+  orderedElements.forEach(el => {
+    // For radio groups keep only the active radio
+    // If there is no active radio, keep only the checked radio
+    // If there is no checked radio, treat like everything else
     if (el.type === 'radio' && el.name) {
-      const replacedIndex = prunedElements.findIndex(
-        ({name}) => name === el.name,
-      )
+      // If the active element is part of the group, add only that
+      if (
+        previousElement &&
+        previousElement.type === el.type &&
+        previousElement.name === el.name
+      ) {
+        if (el === previousElement) {
+          prunedElements.push(el)
+        }
+        return
+      }
 
-      if (replacedIndex === -1) {
-        prunedElements.push(el)
-      } else if (el.checked) {
-        prunedElements.splice(replacedIndex, 1)
+      // If we stumble upon a checked radio, remove the others
+      if (el.checked) {
+        prunedElements = prunedElements.filter(
+          e => e.type !== el.type || e.name !== el.name,
+        )
         prunedElements.push(el)
+        checkedRadio[el.name] = el
+        return
       }
-    } else {
-      prunedElements.push(el)
-    }
-  }
 
-  if (shift) prunedElements.reverse()
+      // If we already found the checked one, skip
+      if (checkedRadio[el.name]) {
+        return
+      }
+    }
 
-  const index = prunedElements.findIndex(
-    el => el === el.ownerDocument.activeElement,
-  )
+    prunedElements.push(el)
+  })
 
+  const index = prunedElements.findIndex(el => el === previousElement)
   const nextElement = getNextElement(index, shift, prunedElements, focusTrap)
 
   const shiftKeyInit = {