Skip to content

Commit

Permalink
fix(popover): isLazy mounting for Popover (#5623)
Browse files Browse the repository at this point in the history
* fix(useAnimationState): update hidden logic for presenting an animation

* chore(Popover): harden unit tests for lazy popover content
  • Loading branch information
Brennvo authored Feb 28, 2022
1 parent 4f111e0 commit 5cd5cff
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 5 deletions.
7 changes: 7 additions & 0 deletions .changeset/tough-kiwis-knock.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@chakra-ui/hooks": patch
"@chakra-ui/popover": patch
---

Fixed an issue where the prop `isLazy` did not work as expected.
This was achieved by updating the hook `useAnimationState`.
2 changes: 1 addition & 1 deletion packages/hooks/src/use-animation-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export function useAnimationState(props: UseAnimationStateProps) {
() => ref.current,
)

const hidden = isOpen ? false : !mounted && once
const hidden = isOpen ? false : !mounted

return {
present: !hidden,
Expand Down
102 changes: 98 additions & 4 deletions packages/popover/tests/popover.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,27 +84,121 @@ test("can close the popover by pressing escape", async () => {
// utils.getByRole("dialog", { hidden: true })
})

test("load content lazily", async () => {
const utils = render(<Component isLazy lazyBehavior="keepMounted" />)
type LazyPopoverContentProps = {
mockFn: () => Promise<any>
}

const LazyPopoverContent = (props: LazyPopoverContentProps) => {
const { mockFn } = props
React.useEffect(() => {
mockFn()
}, [])
return <p data-testid="lazy-content">Lazy content</p>
}

const LazyPopoverComponent = (
props: UsePopoverProps & LazyPopoverContentProps,
) => {
const {
getTriggerProps,
getPopoverProps,
getPopoverPositionerProps,
onClose,
} = usePopover(props)

return (
<div>
<button type="button" {...getTriggerProps()}>
Open
</button>
<div {...getPopoverPositionerProps()}>
<div
{...getPopoverProps({
children: (
<div data-testid="content" tabIndex={0}>
<LazyPopoverContent mockFn={props.mockFn} />
</div>
),
})}
/>
</div>
<button type="button" onClick={onClose}>
Close
</button>
</div>
)
}

test("loads content lazily and unmounts the component from the DOM", async () => {
const mock = jest.fn()
const utils = render(<LazyPopoverComponent isLazy mockFn={mock} />)

// by default, content should not be visible
let content = screen.queryByTestId("content")
expect(content).not.toBeInTheDocument()
expect(mock).toHaveBeenCalledTimes(0)

// open the popover
fireEvent.click(utils.getByText(/open/i))

const dialog = await utils.findByRole("dialog")
content = screen.queryByTestId("content")

// content should now be visible
// content should now be in the DOM
expect(content).toBeInTheDocument()
expect(content).toBeVisible()
expect(mock).toHaveBeenCalledTimes(1)

// close the popover with escape
fireEvent.keyDown(dialog, { key: "Escape" })
expect(content).not.toBeInTheDocument()
expect(content).not.toBeVisible()

// ensure that when popover reopens, it also
// gets remounted
fireEvent.click(utils.getByText(/open/i))
await utils.findByRole("dialog")

content = screen.queryByTestId("content")
expect(content).toBeInTheDocument()
expect(content).toBeVisible()
expect(mock).toHaveBeenCalledTimes(2)
})

test("loads content lazily and persists the component in the DOM", async () => {
const mock = jest.fn()
const utils = render(
<LazyPopoverComponent isLazy lazyBehavior="keepMounted" mockFn={mock} />,
)

// by default, content should not be visible
let content = screen.queryByTestId("content")
expect(content).not.toBeInTheDocument()
expect(mock).toHaveBeenCalledTimes(0)

// open the popover
fireEvent.click(utils.getByText(/open/i))

const dialog = await utils.findByRole("dialog")
content = screen.queryByTestId("content")

// content should now be in the DOM
expect(content).toBeInTheDocument()
expect(mock).toHaveBeenCalledTimes(1)

// close the popover with escape
fireEvent.keyDown(dialog, { key: "Escape" })
expect(content).toBeInTheDocument()
expect(content).not.toBeVisible()

// ensure that when popover reopens, it is
// not remounting
fireEvent.click(utils.getByText(/open/i))
await utils.findByRole("dialog")

// content should still be visible
content = screen.queryByTestId("content")
expect(content).toBeInTheDocument()
expect(mock).toHaveBeenCalledTimes(1)
})

// For testing focus interaction, use another component with a focusable element inside.
Expand Down

1 comment on commit 5cd5cff

@vercel
Copy link

@vercel vercel bot commented on 5cd5cff Feb 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.