Skip to content

Commit

Permalink
fixes an empty menu being rendered when the menu is closed
Browse files Browse the repository at this point in the history
  • Loading branch information
wesgro committed Aug 2, 2024
1 parent bfcf578 commit f9dab63
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 25 deletions.
34 changes: 17 additions & 17 deletions src/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,24 +38,24 @@ const AriaMenuButtonButton: React.FC<
const [isOpen, setIsOpen] = React.useState(false);
React.useEffect(() => {
const managerRef = menuManager.current;
if (innerRef.current) {
managerRef.button = {
element: innerRef.current,
functions: {
focus: () => {
innerRef.current?.focus();
},
setState: (state) => {
flushSync(() => {
if (managerRef) {
managerRef.isOpen = state.menuOpen;
setIsOpen(state.menuOpen);
}
});
},

managerRef.button = {
element: innerRef.current,
functions: {
focus: () => {
innerRef.current?.focus();
},
};
}
setState: (state) => {
flushSync(() => {
if (managerRef) {
managerRef.isOpen = state.menuOpen;
setIsOpen(state.menuOpen);
}
});
},
},
};

return () => {
managerRef.destroy();
};
Expand Down
32 changes: 32 additions & 0 deletions src/Menu.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -297,3 +297,35 @@ test("allows the user to pass an `id` to the `wrapper` and that `id` is rendered
);
expect(screen.getByTestId("wrapper")).toHaveAttribute("id", "foo");
});

test("it should not render a menu if the menu is not open", async () => {
render(
<Wrapper>
<Button>Select a word</Button>
<Menu role="menu">
<ul>
{OPTIONS.map((item) => (
<MenuItem key={item} text={item} value={item}>
{item}
</MenuItem>
))}
</ul>
</Menu>
</Wrapper>,
);
expect(screen.queryByRole("menu")).not.toBeInTheDocument();
});

test("when the menu opens the button should be aria-expanded=true", async () => {
render(<Stage />);
expect(screen.getByRole("button", { name: "Select a word" })).toHaveAttribute(
"aria-expanded",
"false",
);
const button = screen.getByRole("button", { name: "Select a word" });
await userEvent.click(button);
expect(screen.getByRole("button", { name: "Select a word" })).toHaveAttribute(
"aria-expanded",
"true",
);
});
7 changes: 3 additions & 4 deletions src/Menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,7 @@ const AriaMenuButtonMenu: React.FC<
const listenerCleanupRef = React.useRef<() => void | undefined>();

React.useEffect(() => {
if (!el) {
return;
}
const Manager = menuManagerRef.current;

Manager.menu = {
element: el,
functions: {
Expand Down Expand Up @@ -100,6 +96,9 @@ const AriaMenuButtonMenu: React.FC<
[forwardedRef, setEl],
);

if (menuManagerRef.current.isOpen === false) {
return;
}
return (
<Tag
role={isOpen ? "menu" : "presentation"}
Expand Down
9 changes: 5 additions & 4 deletions src/createManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,13 +127,14 @@ class Manager implements ManagerInterface {
}

public update() {
this.menu?.functions.setState({ isOpen: this.isOpen });
this.button?.functions.setState({ menuOpen: this.isOpen });
this.options.onMenuToggle?.({ isOpen: this.isOpen });
this.menu?.functions.setState({ isOpen: this._isOpen });
this.button?.functions.setState({ menuOpen: this._isOpen });
this.options.onMenuToggle?.({ isOpen: this._isOpen });
}

public openMenu(openOptions?: { focusMenu?: boolean }) {
if (this.isOpen) return;
if (this._isOpen) return;

openOptions = openOptions || {};
if (openOptions.focusMenu === undefined) {
openOptions.focusMenu = true;
Expand Down

0 comments on commit f9dab63

Please sign in to comment.