Fix Tab incorrectly activating on focus event (#1887)

* rework Tabs so that they don't change on focus

The "change on focus" was an incorrect implementation detail that made
it a bit easier but this causes a problem as seen in #1858.

If you want to conditionally check if you want to change the tab or note
(e.g. by using `window.confirm`) then the focus is lost while the popup
is shown. Regardless of your choice, the browser will re-focus the Tab
therefore asking you *again* what you want to do.

This fixes that by only activating the tab if needed while using arrow
keys or when you click the tab (not when it is focused).

* update changelog

Author: RobinMalfait

packages/@headlessui-react/CHANGELOG.md

@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Improve `Portal` detection for `Popover` components ([#1842](https://github.com/tailwindlabs/headlessui/pull/1842))
 - Fix `useOutsideClick` swallowing events inside ShadowDOM ([#1876](https://github.com/tailwindlabs/headlessui/pull/1876))
+- Fix `Tab` incorrectly activating on `focus` event ([#1887](https://github.com/tailwindlabs/headlessui/pull/1887))
 
 ## [1.7.2] - 2022-09-15
 

packages/@headlessui-react/src/components/tabs/tabs.tsx

@@ -19,7 +19,7 @@ import { render, Features, PropsForFeatures, forwardRefWithAs } from '../../util
 import { useId } from '../../hooks/use-id'
 import { match } from '../../utils/match'
 import { Keys } from '../../components/keyboard'
-import { focusIn, Focus, sortByDomNode } from '../../utils/focus-management'
+import { focusIn, Focus, sortByDomNode, FocusResult } from '../../utils/focus-management'
 import { useIsoMorphicEffect } from '../../hooks/use-iso-morphic-effect'
 import { useSyncRefs } from '../../hooks/use-sync-refs'
 import { useResolveButtonType } from '../../hooks/use-resolve-button-type'
@@ -28,6 +28,7 @@ import { FocusSentinel } from '../../internal/focus-sentinel'
 import { useEvent } from '../../hooks/use-event'
 import { microTask } from '../../utils/micro-task'
 import { Hidden } from '../../internal/hidden'
+import { getOwnerDocument } from '../../utils/owner'
 
 interface StateDefinition {
   selectedIndex: number
@@ -322,6 +323,7 @@ let TabRoot = forwardRefWithAs(function Tab<TTag extends ElementType = typeof DE
 
   let { orientation, activation, selectedIndex, tabs, panels } = useData('Tab')
   let actions = useActions('Tab')
+  let data = useData('Tab')
   let SSRContext = useSSRTabsCounter('Tab')
 
   let internalTabRef = useRef<HTMLElement | null>(null)
@@ -336,6 +338,16 @@ let TabRoot = forwardRefWithAs(function Tab<TTag extends ElementType = typeof DE
   if (myIndex === -1) myIndex = mySSRIndex
   let selected = myIndex === selectedIndex
 
+  let activateUsing = useEvent((cb: () => FocusResult) => {
+    let result = cb()
+    if (result === FocusResult.Success && activation === 'auto') {
+      let newTab = getOwnerDocument(internalTabRef)?.activeElement
+      let idx = data.tabs.findIndex((tab) => tab.current === newTab)
+      if (idx !== -1) actions.change(idx)
+    }
+    return result
+  })
+
   let handleKeyDown = useEvent((event: ReactKeyboardEvent<HTMLElement>) => {
     let list = tabs.map((tab) => tab.current).filter(Boolean) as HTMLElement[]
 
@@ -353,38 +365,36 @@ let TabRoot = forwardRefWithAs(function Tab<TTag extends ElementType = typeof DE
         event.preventDefault()
         event.stopPropagation()
 
-        return focusIn(list, Focus.First)
+        return activateUsing(() => focusIn(list, Focus.First))
 
       case Keys.End:
       case Keys.PageDown:
         event.preventDefault()
         event.stopPropagation()
 
-        return focusIn(list, Focus.Last)
+        return activateUsing(() => focusIn(list, Focus.Last))
     }
 
-    if (
-      match(orientation, {
+    let result = activateUsing(() => {
+      return match(orientation, {
         vertical() {
           if (event.key === Keys.ArrowUp) return focusIn(list, Focus.Previous | Focus.WrapAround)
           if (event.key === Keys.ArrowDown) return focusIn(list, Focus.Next | Focus.WrapAround)
-          return
+          return FocusResult.Error
         },
         horizontal() {
           if (event.key === Keys.ArrowLeft) return focusIn(list, Focus.Previous | Focus.WrapAround)
           if (event.key === Keys.ArrowRight) return focusIn(list, Focus.Next | Focus.WrapAround)
-          return
+          return FocusResult.Error
         },
       })
-    ) {
+    })
+
+    if (result === FocusResult.Success) {
       return event.preventDefault()
     }
   })
 
-  let handleFocus = useEvent(() => {
-    internalTabRef.current?.focus()
-  })
-
   let ready = useRef(false)
   let handleSelection = useEvent(() => {
     if (ready.current) return
@@ -411,7 +421,6 @@ let TabRoot = forwardRefWithAs(function Tab<TTag extends ElementType = typeof DE
   let ourProps = {
     ref: tabRef,
     onKeyDown: handleKeyDown,
-    onFocus: activation === 'manual' ? handleFocus : handleSelection,
     onMouseDown: handleMouseDown,
     onClick: handleSelection,
     id,

packages/@headlessui-vue/CHANGELOG.md

@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Improve `Portal` detection for `Popover` components ([#1842](https://github.com/tailwindlabs/headlessui/pull/1842))
 - Fix crash when `children` are `undefined` ([#1885](https://github.com/tailwindlabs/headlessui/pull/1885))
 - Fix `useOutsideClick` swallowing events inside ShadowDOM ([#1876](https://github.com/tailwindlabs/headlessui/pull/1876))
+- Fix `Tab` incorrectly activating on `focus` event ([#1887](https://github.com/tailwindlabs/headlessui/pull/1887))
 
 ## [1.7.2] - 2022-09-15
 

packages/@headlessui-vue/src/components/tabs/tabs.ts

@@ -20,11 +20,12 @@ import { useId } from '../../hooks/use-id'
 import { Keys } from '../../keyboard'
 import { dom } from '../../utils/dom'
 import { match } from '../../utils/match'
-import { focusIn, Focus } from '../../utils/focus-management'
+import { focusIn, Focus, FocusResult } from '../../utils/focus-management'
 import { useResolveButtonType } from '../../hooks/use-resolve-button-type'
 import { FocusSentinel } from '../../internal/focus-sentinel'
 import { microTask } from '../../utils/micro-task'
 import { Hidden } from '../../internal/hidden'
+import { getOwnerDocument } from '../../utils/owner'
 
 type StateDefinition = {
   // State
@@ -233,6 +234,16 @@ export let Tab = defineComponent({
     let myIndex = computed(() => api.tabs.value.indexOf(internalTabRef))
     let selected = computed(() => myIndex.value === api.selectedIndex.value)
 
+    function activateUsing(cb: () => FocusResult) {
+      let result = cb()
+      if (result === FocusResult.Success && api.activation.value === 'auto') {
+        let newTab = getOwnerDocument(internalTabRef)?.activeElement
+        let idx = api.tabs.value.findIndex((tab) => dom(tab) === newTab)
+        if (idx !== -1) api.setSelectedIndex(idx)
+      }
+      return result
+    }
+
     function handleKeyDown(event: KeyboardEvent) {
       let list = api.tabs.value.map((tab) => dom(tab)).filter(Boolean) as HTMLElement[]
 
@@ -250,39 +261,37 @@ export let Tab = defineComponent({
           event.preventDefault()
           event.stopPropagation()
 
-          return focusIn(list, Focus.First)
+          return activateUsing(() => focusIn(list, Focus.First))
 
         case Keys.End:
         case Keys.PageDown:
           event.preventDefault()
           event.stopPropagation()
 
-          return focusIn(list, Focus.Last)
+          return activateUsing(() => focusIn(list, Focus.Last))
       }
 
-      if (
+      let result = activateUsing(() =>
         match(api.orientation.value, {
           vertical() {
             if (event.key === Keys.ArrowUp) return focusIn(list, Focus.Previous | Focus.WrapAround)
             if (event.key === Keys.ArrowDown) return focusIn(list, Focus.Next | Focus.WrapAround)
-            return
+            return FocusResult.Error
           },
           horizontal() {
             if (event.key === Keys.ArrowLeft)
               return focusIn(list, Focus.Previous | Focus.WrapAround)
             if (event.key === Keys.ArrowRight) return focusIn(list, Focus.Next | Focus.WrapAround)
-            return
+            return FocusResult.Error
           },
         })
-      ) {
+      )
+
+      if (result === FocusResult.Success) {
         return event.preventDefault()
       }
     }
 
-    function handleFocus() {
-      dom(internalTabRef)?.focus()
-    }
-
     let ready = ref(false)
     function handleSelection() {
       if (ready.value) return
@@ -315,7 +324,6 @@ export let Tab = defineComponent({
       let ourProps = {
         ref: internalTabRef,
         onKeydown: handleKeyDown,
-        onFocus: api.activation.value === 'manual' ? handleFocus : handleSelection,
         onMousedown: handleMouseDown,
         onClick: handleSelection,
         id,