Skip to content

Commit

Permalink
Merge pull request #7131 from Sage/FE-7020
Browse files Browse the repository at this point in the history
fix(submenu): ensure submenu has expected width in Safari
  • Loading branch information
nuria1110 authored Dec 19, 2024
2 parents df6a2e5 + 03865ec commit 3f61a88
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 3f61a88

Please sign in to comment.