Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Close Menu component when using tab key #1673

Merged
merged 4 commits into from Jul 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/@headlessui-react/CHANGELOG.md
Expand Up @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Don’t close dialog when opened during mouse up event ([#1667](https://github.com/tailwindlabs/headlessui/pull/1667))
- Don’t close dialog when drag ends outside dialog ([#1667](https://github.com/tailwindlabs/headlessui/pull/1667))
- Fix outside clicks to close dialog when nested, unopened dialogs are present ([#1667](https://github.com/tailwindlabs/headlessui/pull/1667))
- Close `Menu` component when using `tab` key ([#1673](https://github.com/tailwindlabs/headlessui/pull/1673))

## [1.6.6] - 2022-07-07

Expand Down
14 changes: 7 additions & 7 deletions packages/@headlessui-react/src/components/menu/menu.test.tsx
Expand Up @@ -1359,7 +1359,7 @@ describe('Keyboard interactions', () => {

describe('`Tab` key', () => {
it(
'should focus trap when we use Tab',
'should close when we use Tab',
suppressConsoleLogs(async () => {
render(
<Menu>
Expand Down Expand Up @@ -1401,9 +1401,9 @@ describe('Keyboard interactions', () => {
// Try to tab
await press(Keys.Tab)

// Verify it is still open
assertMenuButton({ state: MenuState.Visible })
assertMenu({ state: MenuState.Visible })
// Verify it is closed
assertMenuButton({ state: MenuState.InvisibleUnmounted })
assertMenu({ state: MenuState.InvisibleUnmounted })
})
)

Expand Down Expand Up @@ -1450,9 +1450,9 @@ describe('Keyboard interactions', () => {
// Try to Shift+Tab
await press(shift(Keys.Tab))

// Verify it is still open
assertMenuButton({ state: MenuState.Visible })
assertMenu({ state: MenuState.Visible })
// Verify it is closed
assertMenuButton({ state: MenuState.InvisibleUnmounted })
assertMenu({ state: MenuState.InvisibleUnmounted })
})
)
})
Expand Down
15 changes: 14 additions & 1 deletion packages/@headlessui-react/src/components/menu/menu.tsx
Expand Up @@ -29,7 +29,13 @@ import { useId } from '../../hooks/use-id'
import { Keys } from '../keyboard'
import { Focus, calculateActiveIndex } from '../../utils/calculate-active-index'
import { isDisabledReactIssue7711 } from '../../utils/bugs'
import { isFocusableElement, FocusableMode, sortByDomNode } from '../../utils/focus-management'
import {
isFocusableElement,
FocusableMode,
sortByDomNode,
Focus as FocusManagementFocus,
focusFrom,
} from '../../utils/focus-management'
import { useOutsideClick } from '../../hooks/use-outside-click'
import { useTreeWalker } from '../../hooks/use-tree-walker'
import { useOpenClosed, State, OpenClosedProvider } from '../../internal/open-closed'
Expand Down Expand Up @@ -492,6 +498,13 @@ let Items = forwardRefWithAs(function Items<TTag extends ElementType = typeof DE
case Keys.Tab:
event.preventDefault()
event.stopPropagation()
dispatch({ type: ActionTypes.CloseMenu })
disposables().nextFrame(() => {
focusFrom(
state.buttonRef.current!,
event.shiftKey ? FocusManagementFocus.Previous : FocusManagementFocus.Next
)
})
break

default:
Expand Down
13 changes: 11 additions & 2 deletions packages/@headlessui-react/src/utils/focus-management.ts
Expand Up @@ -129,7 +129,16 @@ export function sortByDomNode<T>(
})
}

export function focusIn(container: HTMLElement | HTMLElement[], focus: Focus, sorted = true) {
export function focusFrom(current: HTMLElement | null, focus: Focus) {
return focusIn(getFocusableElements(), focus, true, current)
}

export function focusIn(
container: HTMLElement | HTMLElement[],
focus: Focus,
sorted = true,
active: HTMLElement | null = null
) {
let ownerDocument = Array.isArray(container)
? container.length > 0
? container[0].ownerDocument
Expand All @@ -141,7 +150,7 @@ export function focusIn(container: HTMLElement | HTMLElement[], focus: Focus, so
? sortByDomNode(container)
: container
: getFocusableElements(container)
let active = ownerDocument.activeElement as HTMLElement
active = active ?? (ownerDocument.activeElement as HTMLElement)

let direction = (() => {
if (focus & (Focus.First | Focus.Next)) return Direction.Next
Expand Down
1 change: 1 addition & 0 deletions packages/@headlessui-vue/CHANGELOG.md
Expand Up @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Don’t close dialog when opened during mouse up event ([#1667](https://github.com/tailwindlabs/headlessui/pull/1667))
- Don’t close dialog when drag ends outside dialog ([#1667](https://github.com/tailwindlabs/headlessui/pull/1667))
- Fix outside clicks to close dialog when nested, unopened dialogs are present ([#1667](https://github.com/tailwindlabs/headlessui/pull/1667))
- Close `Menu` component when using `tab` key ([#1673](https://github.com/tailwindlabs/headlessui/pull/1673))

## [1.6.7] - 2022-07-12

Expand Down
16 changes: 8 additions & 8 deletions packages/@headlessui-vue/src/components/menu/menu.test.tsx
Expand Up @@ -1656,7 +1656,7 @@ describe('Keyboard interactions', () => {
})

describe('`Tab` key', () => {
it('should focus trap when we use Tab', async () => {
it('should not focus trap when we use Tab', async () => {
renderTemplate(jsx`
<Menu>
<MenuButton>Trigger</MenuButton>
Expand Down Expand Up @@ -1697,12 +1697,12 @@ describe('Keyboard interactions', () => {
// Try to tab
await press(Keys.Tab)

// Verify it is still open
assertMenuButton({ state: MenuState.Visible })
assertMenu({ state: MenuState.Visible })
// Verify it is closed
assertMenuButton({ state: MenuState.InvisibleUnmounted })
assertMenu({ state: MenuState.InvisibleUnmounted })
})

it('should focus trap when we use Shift+Tab', async () => {
it('should not focus trap when we use Shift+Tab', async () => {
renderTemplate(jsx`
<Menu>
<MenuButton>Trigger</MenuButton>
Expand Down Expand Up @@ -1743,9 +1743,9 @@ describe('Keyboard interactions', () => {
// Try to Shift+Tab
await press(shift(Keys.Tab))

// Verify it is still open
assertMenuButton({ state: MenuState.Visible })
assertMenu({ state: MenuState.Visible })
// Verify it is closed
assertMenuButton({ state: MenuState.InvisibleUnmounted })
assertMenu({ state: MenuState.InvisibleUnmounted })
})
})

Expand Down
15 changes: 14 additions & 1 deletion packages/@headlessui-vue/src/components/menu/menu.ts
Expand Up @@ -22,7 +22,13 @@ import { useTreeWalker } from '../../hooks/use-tree-walker'
import { useOpenClosedProvider, State, useOpenClosed } from '../../internal/open-closed'
import { match } from '../../utils/match'
import { useResolveButtonType } from '../../hooks/use-resolve-button-type'
import { FocusableMode, isFocusableElement, sortByDomNode } from '../../utils/focus-management'
import {
FocusableMode,
isFocusableElement,
sortByDomNode,
Focus as FocusManagementFocus,
focusFrom,
} from '../../utils/focus-management'
import { useOutsideClick } from '../../hooks/use-outside-click'

enum MenuStates {
Expand Down Expand Up @@ -413,6 +419,13 @@ export let MenuItems = defineComponent({
case Keys.Tab:
event.preventDefault()
event.stopPropagation()
api.closeMenu()
nextTick(() =>
focusFrom(
dom(api.buttonRef),
event.shiftKey ? FocusManagementFocus.Previous : FocusManagementFocus.Next
)
)
break

default:
Expand Down
31 changes: 20 additions & 11 deletions packages/@headlessui-vue/src/utils/focus-management.ts
Expand Up @@ -122,7 +122,16 @@ export function sortByDomNode<T>(
})
}

export function focusIn(container: HTMLElement | HTMLElement[], focus: Focus, sorted = true) {
export function focusFrom(current: HTMLElement | null, focus: Focus) {
return focusIn(getFocusableElements(), focus, true, current)
}

export function focusIn(
container: HTMLElement | HTMLElement[],
focus: Focus,
sorted = true,
active: HTMLElement | null = null
) {
let ownerDocument =
(Array.isArray(container)
? container.length > 0
Expand All @@ -135,7 +144,7 @@ export function focusIn(container: HTMLElement | HTMLElement[], focus: Focus, so
? sortByDomNode(container)
: container
: getFocusableElements(container)
let active = ownerDocument.activeElement as HTMLElement
active = active ?? (ownerDocument.activeElement as HTMLElement)

let direction = (() => {
if (focus & (Focus.First | Focus.Next)) return Direction.Next
Expand Down Expand Up @@ -180,15 +189,6 @@ export function focusIn(container: HTMLElement | HTMLElement[], focus: Focus, so
offset += direction
} while (next !== ownerDocument.activeElement)

// This is a little weird, but let me try and explain: There are a few scenario's
// in chrome for example where a focused `<a>` tag does not get the default focus
// styles and sometimes they do. This highly depends on whether you started by
// clicking or by using your keyboard. When you programmatically add focus `anchor.focus()`
// then the active element (document.activeElement) is this anchor, which is expected.
// However in that case the default focus styles are not applied *unless* you
// also add this tabindex.
if (!next.hasAttribute('tabindex')) next.setAttribute('tabindex', '0')

// By default if you <Tab> to a text input or a textarea, the browser will
// select all the text once the focus is inside these DOM Nodes. However,
// since we are manually moving focus this behaviour is not happening. This
Expand All @@ -201,5 +201,14 @@ export function focusIn(container: HTMLElement | HTMLElement[], focus: Focus, so
next.select()
}

// This is a little weird, but let me try and explain: There are a few scenario's
// in chrome for example where a focused `<a>` tag does not get the default focus
// styles and sometimes they do. This highly depends on whether you started by
// clicking or by using your keyboard. When you programmatically add focus `anchor.focus()`
// then the active element (document.activeElement) is this anchor, which is expected.
// However in that case the default focus styles are not applied *unless* you
// also add this tabindex.
if (!next.hasAttribute('tabindex')) next.setAttribute('tabindex', '0')

return FocusResult.Success
}