Skip to content

Commit

Permalink
fix(submenu): ensure submenu has expected width in Safari
Browse files Browse the repository at this point in the history
Fixes issue where a MenuItem within a submenu with overflow would get cut off and render with a
scroll bar.

fix #7112
  • Loading branch information
nuria1110 committed Dec 17, 2024
1 parent d1a7d79 commit a6ea2e2
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 8 deletions.
5 changes: 5 additions & 0 deletions src/components/menu/__internal__/submenu/submenu.style.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,13 @@ const StyledSubmenu = styled.ul<StyledSubmenuProps>`
${submenuMaxWidth &&
css`
width: max-content;
max-width: ${submenuMaxWidth};
li {
max-width: ${submenuMaxWidth};
}
&&& {
a,
button,
Expand Down
20 changes: 20 additions & 0 deletions src/components/menu/__internal__/submenu/submenu.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -489,3 +489,23 @@ test("should render the menu with a max-height set when the `maxHeight` prop is
maxHeight: "80px",
});
});

test("should override submenu children's `maxWidth` if `submenuMaxWidth` is set", async () => {
const user = userEvent.setup();
render(
<MenuContext.Provider value={menuContextValues}>
<Submenu title="title" submenuMaxWidth="300px">
<MenuItem maxWidth="400px">Apple</MenuItem>
<MenuItem minWidth="400px">Banana</MenuItem>
</Submenu>
</MenuContext.Provider>,
);
const menuItem = screen.getByRole("button", { name: "title" });
await user.hover(menuItem);
const submenu = screen.getByRole("list");
const submenuChildren = screen.getAllByRole("listitem");

expect(submenu).toHaveStyle({ maxWidth: "300px" });
expect(submenuChildren[0]).toHaveStyle({ maxWidth: "300px" });
expect(submenuChildren[1]).toHaveStyle({ maxWidth: "300px" });
});
3 changes: 2 additions & 1 deletion src/components/menu/menu-item/menu-item.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ export const MenuItem = ({
updateFocusId: updateSubmenuFocusId,
handleKeyDown: handleSubmenuKeyDown,
shiftTabPressed,
submenuHasMaxWidth,
} = submenuContext;
const ref = useRef<HTMLAnchorElement & HTMLButtonElement & HTMLDivElement>(
null,
Expand Down Expand Up @@ -355,7 +356,7 @@ export const MenuItem = ({
{...elementProps}
menuItemVariant={variant}
ariaLabel={ariaLabel}
maxWidth={maxWidth}
maxWidth={!submenuHasMaxWidth ? itemMaxWidth : undefined}
inFullscreenView={inFullscreenView}
asPassiveItem={asPassiveItem}
placeholderTabIndex={asPassiveItem}
Expand Down
6 changes: 6 additions & 0 deletions src/components/menu/menu-test.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,12 @@ export const LongLabelsStory = () => {
<MenuItem>Child B</MenuItem>
<MenuItem>Child C</MenuItem>
</MenuItem>
<MenuItem submenu="Parent Menu C with overflow" submenuMaxWidth="300px">
<MenuItem minWidth="max-content">
Child with a very long label that should wrap onto the next line and
not get cut off
</MenuItem>
</MenuItem>
</Menu>
);
};
Expand Down
6 changes: 3 additions & 3 deletions src/components/menu/menu.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ import {
The example below has set the `showDropdownArrow` to false for the MenuItem with a submenu which means no dropdown arrow
is rendered.

<Canvas of={MenuStories.NoDropdwonArrowOnSubmenuStory} />
<Canvas of={MenuStories.NoDropdownArrowOnSubmenuStory} />

### Split submenu into separate component

Expand All @@ -112,11 +112,11 @@ Note that only one `ScrollableBlock` can be used within a single submenu.
This is an example of using the `parent` prop of `ScrollableBlock` to render a scrollable sublist of a parent item. The `parentVariant`
prop can be used to give it a variant that's different from that used in the `ScrollableBlock`.

Note that the result shown here, for those interacting with the page without assisistive technology, is the same as that which
Note that the result shown here, for those interacting with the page without assistive technology, is the same as that which
would be produced by leaving out the `parent` prop and putting the `Search` component inside a separate `MenuItem` just before the
`ScrollableBlock`. However the rendered HTML would be different, with the `ScrollableBlock` becoming a sublist inside its own list
item that is not connected to the Search - in this example there is a clear semantic relationship between the search input and the
scrollable list so the `parent` prop should be used to ensure screenreaders make the relationship clear to their users.
scrollable list so the `parent` prop should be used to ensure screen readers make the relationship clear to their users.

<Canvas of={MenuStories.ScrollableSubmenuWithParent} />

Expand Down
8 changes: 4 additions & 4 deletions src/components/menu/menu.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ export const WithIconStory: Story = () => {
WithIconStory.storyName = "With Icon";
WithIconStory.parameters = { chromatic: { disableSnapshot: true } };

export const NoDropdwonArrowOnSubmenuStory: Story = () => {
export const NoDropdownArrowOnSubmenuStory: Story = () => {
return (
<Box minHeight="150px">
<Menu>
Expand All @@ -344,8 +344,8 @@ export const NoDropdwonArrowOnSubmenuStory: Story = () => {
</Box>
);
};
NoDropdwonArrowOnSubmenuStory.storyName = "No Dropdwon Arrow on Submenu";
NoDropdwonArrowOnSubmenuStory.parameters = {
NoDropdownArrowOnSubmenuStory.storyName = "No Dropdown Arrow on Submenu";
NoDropdownArrowOnSubmenuStory.parameters = {
chromatic: { disableSnapshot: true },
};

Expand Down Expand Up @@ -395,7 +395,7 @@ export const SubmenuIconAndTextAlignment: Story = () => {
</Box>
);
};
SubmenuIconAndTextAlignment.storyName = "Submeu Icon and Text Alignment";
SubmenuIconAndTextAlignment.storyName = "Submenu Icon and Text Alignment";
SubmenuIconAndTextAlignment.parameters = {
chromatic: { disableSnapshot: true },
};
Expand Down

0 comments on commit a6ea2e2

Please sign in to comment.