Skip to content

Commit cc0bba9

Browse files
authored
fix: Properly hide separator in Menu when next node is loader/null (#10609)
* fix section rendering separator even after last item in Menu due to loader * add stories and add width setting to menu async docs * fix lint
1 parent 90ff0c6 commit cc0bba9

5 files changed

Lines changed: 152 additions & 51 deletions

File tree

packages/@react-spectrum/s2/chromatic/Menu.stories.tsx

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@ import {
2020
} from '../stories/Menu.stories';
2121
import {Button} from '../src/Button';
2222
import {expect} from '@storybook/jest';
23-
import {Menu, MenuItem, MenuTrigger} from '../src/Menu';
23+
import {Header, Heading} from '../src/Content';
24+
import {Menu, MenuItem, MenuSection, MenuTrigger} from '../src/Menu';
2425
import type {Meta, StoryObj} from '@storybook/react';
2526
import NewIcon from '../s2wf-icons/S2_Icon_New_20_N.svg';
2627
import {userEvent, within} from 'storybook/test';
@@ -149,3 +150,68 @@ export const WithLoadMore: Story = {
149150
await within(menu).findByRole('progressbar', {hidden: true});
150151
}
151152
};
153+
154+
export const WithSections: Story = {
155+
render: () => (
156+
<MenuTrigger>
157+
<Button aria-label="Actions">
158+
<NewIcon />
159+
</Button>
160+
<Menu aria-label="Test">
161+
<MenuSection>
162+
<Header>
163+
<Heading>Section 1</Heading>
164+
</Header>
165+
<MenuItem>Cut</MenuItem>
166+
<MenuItem>Copy</MenuItem>
167+
</MenuSection>
168+
<MenuSection>
169+
<Header>
170+
<Heading>Section 2</Heading>
171+
</Header>
172+
<MenuItem>Paste</MenuItem>
173+
<MenuItem>Delete</MenuItem>
174+
</MenuSection>
175+
</Menu>
176+
</MenuTrigger>
177+
),
178+
play: async ({canvasElement}) => {
179+
await userEvent.tab();
180+
await userEvent.keyboard('{ArrowDown}');
181+
let body = canvasElement.ownerDocument.body;
182+
await within(body).findByRole('menu');
183+
}
184+
};
185+
186+
export const WithSectionsAndLoadMore: Story = {
187+
render: () => (
188+
<MenuTrigger>
189+
<Button aria-label="Actions">
190+
<NewIcon />
191+
</Button>
192+
<Menu aria-label="Test" loadingState="loadingMore">
193+
<MenuSection>
194+
<Header>
195+
<Heading>Section 1</Heading>
196+
</Header>
197+
<MenuItem>Cut</MenuItem>
198+
<MenuItem>Copy</MenuItem>
199+
</MenuSection>
200+
<MenuSection>
201+
<Header>
202+
<Heading>Section 2</Heading>
203+
</Header>
204+
<MenuItem>Paste</MenuItem>
205+
<MenuItem>Delete</MenuItem>
206+
</MenuSection>
207+
</Menu>
208+
</MenuTrigger>
209+
),
210+
play: async ({canvasElement}) => {
211+
await userEvent.tab();
212+
await userEvent.keyboard('{ArrowDown}');
213+
let body = canvasElement.ownerDocument.body;
214+
let menu = await within(body).findByRole('menu');
215+
await within(menu).findByRole('progressbar', {hidden: true});
216+
}
217+
};

packages/@react-spectrum/s2/src/ComboBox.tsx

Lines changed: 2 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ import {
3535
SpectrumLabelableProps
3636
} from '@react-types/shared';
3737
import {AvatarContext} from './Avatar';
38-
import {BaseCollection, CollectionNode} from 'react-aria/private/collections/BaseCollection';
3938
import {baseColor, centerPadding, focusRing, space, style} from '../style' with {type: 'macro'};
4039
import {Button, ButtonRenderProps} from 'react-aria-components/Button';
4140
import {centerBaseline} from './CenterBaseline';
@@ -86,6 +85,7 @@ import {HeaderContext, HeadingContext, Text, TextContext} from './Content';
8685
import {IconContext} from './Icon';
8786
import {InputContext, InputProps} from 'react-aria-components/Input';
8887
import intlMessages from '../intl/*.json';
88+
import {isSeparatorHidden, SeparatorNode} from './separator-utils';
8989
import {ListLayout} from 'react-stately/useVirtualizerState';
9090
import {mergeRefs} from 'react-aria/mergeRefs';
9191
import {Node} from '@react-types/shared';
@@ -822,24 +822,6 @@ const ComboboxInner = forwardRef(function ComboboxInner(
822822
);
823823
});
824824

825-
class SeparatorNode extends CollectionNode<any> {
826-
static readonly type = 'separator';
827-
828-
filter(
829-
collection: BaseCollection<any>,
830-
newCollection: BaseCollection<any>
831-
): CollectionNode<any> | null {
832-
let prevItem = newCollection.getItem(this.prevKey!);
833-
if (prevItem && prevItem.type !== 'separator') {
834-
let clone = this.clone();
835-
newCollection.addDescendants(clone, collection);
836-
return clone;
837-
}
838-
839-
return null;
840-
}
841-
}
842-
843825
export const Divider = /*#__PURE__*/ createLeafComponent(
844826
SeparatorNode,
845827
function Divider(
@@ -849,13 +831,7 @@ export const Divider = /*#__PURE__*/ createLeafComponent(
849831
) {
850832
let listState = useContext(ListStateContext)!;
851833

852-
let nextNode = node.nextKey != null && listState.collection.getItem(node.nextKey);
853-
if (
854-
node.prevKey == null ||
855-
!nextNode ||
856-
nextNode.type === 'separator' ||
857-
(nextNode.type === 'loader' && nextNode.nextKey == null)
858-
) {
834+
if (isSeparatorHidden(node, listState.collection)) {
859835
return null;
860836
}
861837

packages/@react-spectrum/s2/src/Menu.tsx

Lines changed: 35 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,16 @@ import {
2323
MenuTriggerProps as AriaMenuTriggerProps,
2424
SubmenuTrigger as AriaSubmenuTrigger,
2525
SubmenuTriggerProps as AriaSubmenuTriggerProps,
26-
MenuItemRenderProps
26+
MenuItemRenderProps,
27+
MenuStateContext
2728
} from 'react-aria-components/Menu';
2829
import {
2930
AsyncLoadable,
3031
DOMRef,
3132
DOMRefValue,
3233
GlobalDOMAttributes,
3334
LoadingState,
35+
Node,
3436
PressEvent
3537
} from '@react-types/shared';
3638
import {
@@ -57,6 +59,7 @@ import {
5759
} from './style-utils' with {type: 'macro'};
5860
import {
5961
createContext,
62+
ForwardedRef,
6063
forwardRef,
6164
JSX,
6265
ReactElement,
@@ -65,6 +68,7 @@ import {
6568
useRef,
6669
useState
6770
} from 'react';
71+
import {createLeafComponent} from 'react-aria/CollectionBuilder';
6872
import {divider} from './Divider';
6973
import {edgeToText} from '../style/spectrum-theme' with {type: 'macro'};
7074
import {forwardRefType} from './types';
@@ -74,6 +78,7 @@ import {ImageContext} from './Image';
7478
import InfoCircleIcon from '../s2wf-icons/S2_Icon_InfoCircle_20_N.svg'; // chevron right removed??
7579
import {InPopoverContext, Popover, PopoverContext} from './Popover';
7680
import intlMessages from '../intl/*.json';
81+
import {isSeparatorHidden, SeparatorNode} from './separator-utils';
7782
import LinkOutIcon from '../ui-icons/LinkOut';
7883
import {mergeStyles} from '../style/runtime';
7984
import {Placement} from 'react-aria/useOverlayPosition';
@@ -585,29 +590,35 @@ export const Menu = /*#__PURE__*/ (forwardRef as forwardRefType)(function Menu<T
585590
return content;
586591
});
587592

588-
export function Divider(props: SeparatorProps): ReactNode {
589-
return (
590-
<Separator
591-
{...props}
592-
className={mergeStyles(
593-
divider({
594-
size: 'M',
595-
orientation: 'horizontal',
596-
isStaticColor: false
597-
}),
598-
style({
599-
display: {
600-
default: 'grid',
601-
':last-child': 'none'
602-
},
603-
gridColumnStart: 2,
604-
gridColumnEnd: -2,
605-
marginY: size(5) // height of the menu separator is 12px, and the divider is 2px
606-
})
607-
)}
608-
/>
609-
);
610-
}
593+
export const Divider = /*#__PURE__*/ createLeafComponent(
594+
SeparatorNode,
595+
function Divider(props: SeparatorProps, ref: ForwardedRef<HTMLElement>, node: Node<unknown>) {
596+
let state = useContext(MenuStateContext)!;
597+
598+
if (isSeparatorHidden(node, state.collection)) {
599+
return null;
600+
}
601+
602+
return (
603+
<Separator
604+
{...props}
605+
ref={ref}
606+
className={mergeStyles(
607+
divider({
608+
size: 'M',
609+
orientation: 'horizontal',
610+
isStaticColor: false
611+
}),
612+
style({
613+
gridColumnStart: 2,
614+
gridColumnEnd: -2,
615+
marginY: size(5) // height of the menu separator is 12px, and the divider is 2px
616+
})
617+
)}
618+
/>
619+
);
620+
}
621+
);
611622

612623
export interface MenuSectionProps<T> extends Omit<
613624
AriaMenuSectionProps<T>,
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/*
2+
* Copyright 2026 Adobe. All rights reserved.
3+
* This file is licensed to you under the Apache License, Version 2.0 (the "License");
4+
* you may not use this file except in compliance with the License. You may obtain a copy
5+
* of the License at http://www.apache.org/licenses/LICENSE-2.0
6+
*
7+
* Unless required by applicable law or agreed to in writing, software distributed under
8+
* the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
9+
* OF ANY KIND, either express or implied. See the License for the specific language
10+
* governing permissions and limitations under the License.
11+
*/
12+
13+
import {BaseCollection, CollectionNode} from 'react-aria/private/collections/BaseCollection';
14+
import {Collection, Node} from '@react-types/shared';
15+
16+
export class SeparatorNode extends CollectionNode<any> {
17+
static readonly type = 'separator';
18+
19+
filter(
20+
collection: BaseCollection<any>,
21+
newCollection: BaseCollection<any>
22+
): CollectionNode<any> | null {
23+
let prevItem = newCollection.getItem(this.prevKey!);
24+
if (prevItem && prevItem.type !== 'separator') {
25+
let clone = this.clone();
26+
newCollection.addDescendants(clone, collection);
27+
return clone;
28+
}
29+
30+
return null;
31+
}
32+
}
33+
34+
// given a collection check if the separator should be hidden if the next node is a loader or is end of list/menu
35+
export function isSeparatorHidden(
36+
node: Node<unknown>,
37+
collection: Collection<Node<unknown>>
38+
): boolean {
39+
let nextNode = node.nextKey != null && collection.getItem(node.nextKey);
40+
return (
41+
node.prevKey == null ||
42+
!nextNode ||
43+
nextNode.type === 'separator' ||
44+
(nextNode.type === 'loader' && nextNode.nextKey == null)
45+
);
46+
}

packages/dev/s2-docs/pages/s2/Menu.mdx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,7 @@ Use the `loadingState` and `onLoadMore` props to enable async loading and infini
292292
import {MenuTrigger, Menu, MenuItem} from '@react-spectrum/s2/Menu';
293293
import {ActionButton} from '@react-spectrum/s2/ActionButton';
294294
import {useAsyncList} from '@react-spectrum/s2/useAsyncList';
295+
import {style} from '@react-spectrum/s2/style' with {type: 'macro'};
295296

296297
interface Character {
297298
name: string
@@ -318,6 +319,7 @@ function Example() {
318319
<MenuTrigger>
319320
<ActionButton>Select Character</ActionButton>
320321
<Menu
322+
styles={style({width: 150})}
321323
aria-label="Star Wars Characters"
322324
items={list.items}
323325
/*- begin highlight -*/

0 commit comments

Comments
 (0)