Skip to content

Commit

Permalink
3003 autocomplete when in dialog intercepts escape keypresses and cli…
Browse files Browse the repository at this point in the history
…ck outside (#3087)

* trying to bring back branch

* does not mount AutocompleteOverlay if the menu is hidden - fixes issues with closing Dialog

* fixes stuff that got broken when we stopped rendering the Autocomplete Overlay instead of visually hiding it

* fix: VisallyHidden now aria-VisallyHidden

---------

Co-authored-by: Mike Perrotti <mperrotti@github.com>
  • Loading branch information
green6erry and mperrotti authored Jul 24, 2023
1 parent ddf8ebf commit c736e8e
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 23 deletions.
8 changes: 6 additions & 2 deletions src/Autocomplete/AutocompleteOverlay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import Overlay, {OverlayProps} from '../Overlay'
import {ComponentProps} from '../utils/types'
import {AutocompleteContext} from './AutocompleteContext'
import {useRefObjectAsForwardedRef} from '../hooks/useRefObjectAsForwardedRef'
import VisuallyHidden from '../_VisuallyHidden'

type AutocompleteOverlayInternalProps = {
/**
Expand Down Expand Up @@ -49,7 +50,7 @@ function AutocompleteOverlay({
return null
}

return (
return showMenu ? (
<Overlay
returnFocusRef={inputRef}
preventFocusOnOpen={true}
Expand All @@ -58,14 +59,17 @@ function AutocompleteOverlay({
ref={floatingElementRef as React.RefObject<HTMLDivElement>}
top={position?.top}
left={position?.left}
visibility={showMenu ? 'visible' : 'hidden'}
sx={{
overflow: 'auto',
}}
{...overlayProps}
>
{children}
</Overlay>
) : (
// HACK: This ensures AutocompleteMenu is still mounted when closing the menu and all of the hooks inside of it are still called.
// A better way to do this would be to move the hooks to AutocompleteOverlay or somewhere that won't get unmounted.
<VisuallyHidden aria-hidden="true">{children}</VisuallyHidden>
)
}

Expand Down
30 changes: 16 additions & 14 deletions src/__tests__/Autocomplete.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -272,28 +272,30 @@ describe('Autocomplete', () => {
<LabelledAutocomplete menuProps={{items: mockItems, selectedItemIds: [], selectionVariant: 'multiple'}} />,
)
const inputNode = container.querySelector('#autocompleteInput')
const itemToClickNode = getByText(mockItems[1].text)

expect(inputNode?.getAttribute('aria-expanded')).not.toBe('true')
inputNode && fireEvent.focus(inputNode)
expect(inputNode?.getAttribute('aria-expanded')).toBe('true')
fireEvent.click(itemToClickNode)
inputNode && (await user.type(inputNode, '{enter}'))
expect(inputNode?.getAttribute('aria-expanded')).toBe('true')
if (inputNode) {
expect(inputNode.getAttribute('aria-expanded')).not.toBe('true')
await user.click(inputNode)
expect(inputNode.getAttribute('aria-expanded')).toBe('true')
await user.click(getByText(mockItems[1].text))
expect(inputNode.getAttribute('aria-expanded')).toBe('true')
}
})

it('closes the menu when clicking an item in the menu if selectionVariant=single', () => {
it('closes the menu when clicking an item in the menu if selectionVariant=single', async () => {
const user = userEvent.setup()
const {getByText, container} = HTMLRender(
<LabelledAutocomplete menuProps={{items: mockItems, selectedItemIds: [], selectionVariant: 'single'}} />,
)
const inputNode = container.querySelector('#autocompleteInput')
const itemToClickNode = getByText(mockItems[1].text)

expect(inputNode?.getAttribute('aria-expanded')).not.toBe('true')
inputNode && fireEvent.focus(inputNode)
expect(inputNode?.getAttribute('aria-expanded')).toBe('true')
fireEvent.click(itemToClickNode)
expect(inputNode?.getAttribute('aria-expanded')).not.toBe('true')
if (inputNode) {
expect(inputNode.getAttribute('aria-expanded')).not.toBe('true')
await user.click(inputNode)
expect(inputNode.getAttribute('aria-expanded')).toBe('true')
await user.click(getByText(mockItems[1].text))
expect(inputNode.getAttribute('aria-expanded')).not.toBe('true')
}
})

it('calls handleAddItem with new item data when passing addNewItem', () => {
Expand Down
15 changes: 10 additions & 5 deletions src/hooks/useDialog.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {useCallback, useEffect} from 'react'
import {useOnEscapePress} from './useOnEscapePress'

const noop = () => null

Expand Down Expand Up @@ -103,19 +104,23 @@ function useDialog({
case 'Tab':
handleTab(event)
break
case 'Escape':
onDismiss()
event.stopPropagation()
break
}
},
[handleTab, onDismiss],
[handleTab],
)

const getDialogProps = () => {
return {onKeyDown}
}

useOnEscapePress(
(event: KeyboardEvent) => {
onDismiss()
event.preventDefault()
},
[onDismiss],
)

return {getDialogProps}
}

Expand Down
4 changes: 2 additions & 2 deletions src/stories/Autocomplete.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -769,8 +769,8 @@ export const InADialog = (args: FormControlArgs<AutocompleteArgs>) => {

return (
<>
<Button onClick={() => setIsDialogOpen(!isDialogOpen)}>Show dialog</Button>
<Dialog id="dialog-with-autocomplete" isOpen={isDialogOpen}>
<Button onClick={() => setIsDialogOpen(true)}>Show dialog</Button>
<Dialog id="dialog-with-autocomplete" isOpen={isDialogOpen} onDismiss={() => setIsDialogOpen(false)}>
<div ref={outerContainerRef}>
<Box as="form" sx={{p: 3}}>
{mounted ? (
Expand Down

0 comments on commit c736e8e

Please sign in to comment.