From afc9129b6b42ee0bd369f7e176f14ac715b8d57d Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 30 Sep 2022 13:44:06 +0200 Subject: [PATCH 1/2] 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). --- .../src/components/tabs/tabs.tsx | 35 ++++++++++++------- .../src/components/tabs/tabs.ts | 32 ++++++++++------- 2 files changed, 42 insertions(+), 25 deletions(-) diff --git a/packages/@headlessui-react/src/components/tabs/tabs.tsx b/packages/@headlessui-react/src/components/tabs/tabs.tsx index 02f51d8420..0924972a12 100644 --- a/packages/@headlessui-react/src/components/tabs/tabs.tsx +++ b/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(null) @@ -336,6 +338,16 @@ let TabRoot = forwardRefWithAs(function Tab 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) => { let list = tabs.map((tab) => tab.current).filter(Boolean) as HTMLElement[] @@ -353,38 +365,36 @@ let TabRoot = forwardRefWithAs(function Tab 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