test: add regression for global aria-posinset across sections by gonzoblasco · Pull Request #10554 · adobe/react-spectrum

gonzoblasco · 2026-09-02T14:28:04Z

What

Test-only PR: adds a regression test that locks in global aria-posinset across sections in a virtualized menu.

Why

From the discussion in #9556: aria-posinset must be consistent with aria-setsize (the global item count) for sectioned collections. The current code already produces the correct global position (1..4 across two sections, matching aria-setsize=4), but nothing guards it. This test pins the behavior so a future refactor of the index computation cannot silently reintroduce per-section restart.

Notes

Passes against current main (verified locally, 4/4 in useMenu.test.tsx).
No production code changes.

Locks in that aria-posinset is global (1..4) across sections in a virtualized menu and matches aria-setsize, instead of restarting per group. Passes against current main; guards the behavior from the discussion in adobe#9556.

yihuiliao · 2026-09-02T22:32:55Z

Thanks for the PR! You'll need to sign the CLA and then close/re-open the PR to rerun the check

snowystinger

The test isn't rendering the sections

while that shouldn't affect the posinset, it's incomplete

The previous helper flattened sections into plain items, so the DOM never contained the sectioned menu structure the test claims to cover. Render each section as a labelled group (role=presentation wrapper + role=group with aria-labelledby heading), mirroring the real sectioned menu, and assert the groups are present before checking the global posinset values.

gonzoblasco · 2026-09-02T23:06:47Z

Fixed in 1af6cf2 - the helper now renders the real sectioned structure instead of flattening.

Each section renders as a role=presentation wrapper with a labelled group inside (role=group + aria-labelledby pointing at the section heading), mirroring how the sectioned menu is built. The test now asserts the two groups are present and each is labelled by its heading before checking the global posinset values, so the DOM shape is locked in as part of the regression.

Verified: 4/4 in useMenu.test.tsx, full lint (format, types, oxlint, constraints) clean.

Render each section through the real useMenuSection hook instead of hand-rolling the group structure, so any future logic added to the hook (per-section count restart, references) is exercised by this test.

snowystinger · 2026-09-02T23:44:39Z

+    // sectioned menu structure, so this test locks the real DOM shape.
+    let groups = getAllByRole2('group');
+    expect(groups).toHaveLength(2);
+    // Each group is labelled by its section heading, which is present in the DOM.


Suggested change

// Each group is labelled by its section heading, which is present in the DOM.

snowystinger · 2026-09-02T23:44:49Z

+    // The sections must actually render as groups, mirroring the real
+    // sectioned menu structure, so this test locks the real DOM shape.


Suggested change

// The sections must actually render as groups, mirroring the real

// sectioned menu structure, so this test locks the real DOM shape.

snowystinger · 2026-09-02T23:45:14Z

+    // aria-posinset should be global (1..4) and match aria-setsize, not restart
+    // per section (which would report 1..2 for both groups).


Suggested change

// aria-posinset should be global (1..4) and match aria-setsize, not restart

// per section (which would report 1..2 for both groups).

This comment is the name of the test

snowystinger · 2026-09-02T23:49:25Z

+  let {itemProps, headingProps, groupProps} = useMenuSection({
+    heading: node.rendered,
+    'aria-label': node['aria-label']
+  });


I think you meant this?

Suggested change

let {itemProps, headingProps, groupProps} = useMenuSection({

heading: node.rendered,

'aria-label': node['aria-label']

});

let {itemProps, headingProps, groupProps} = useMenuSection({

heading: node.props.title,

'aria-label': node['aria-label']

});

though not sure what the aria-label there is for? it could just be omitted i think?

Applied in 6b65027 - the heading now uses node.props.title (matching how Section renders it) and the redundant aria-label is dropped, since with a heading present useMenuSection labels the group via aria-labelledby.

Verified: 4/4 in useMenu.test.tsx, full lint clean.

The aria-label is the fallback accessible name for sections without a heading. useMenuSection labels the group via aria-labelledby when a heading is present, and falls back to aria-label when the section has no heading (the heading is optional in the ARIA menu pattern). Without it, an unheaded section group would have no accessible name at all.

I kept it and added a test that locks that branch: a Section with aria-label and no title renders a group with aria-label and no aria-labelledby. So the accessible-name contract of sectioned menus is covered on both paths - labelled-by-heading and labelled-by-aria-label.

Verified: 5/5 in useMenu.test.tsx, full lint clean.

Use node.props.title for the section heading (matching how Section renders it) and drop the redundant aria-label - with a heading present, useMenuSection labels the group via aria-labelledby.

useMenuSection labels a section group via aria-labelledby when a heading is present, and falls back to aria-label when the section has no heading. The new test locks that fallback branch so the accessible name contract of sectioned menus stays covered.

snowystinger

you might find this an easier test to write at the RAC level instead of the hook level

yihuiliao · 2026-09-03T00:13:50Z

+  return (
+    <div {...itemProps}>
+      {node.props.title && <span {...headingProps}>{node.props.title}</span>}
+      <div {...groupProps}>


i know that in our RAC implementation, headings + menu items are siblings. as in, items are not grouped in a wrapper separate from the heading. and as @snowystinger said, probably easier to do this as the RAC level

snowystinger senalo en 3919829010 que el span del heading debe seguir renderizando node.rendered, no node.props.title - matching how the real MenuSection component in @adobe/react-spectrum does it. The hook already uses node.rendered for the heading prop, this restores the same source for the visible span. Also explored migrating the test to react-aria-components level with a real Virtualizer (per snowystinger's suggestion in 5096476622) - found that the Virtualizer path exposes different posinset behavior (resets per section, presumably a separate issue from the original adobe#10487 hook contract this PR locks in). Keeping the test at the hook level where the original contract was verified.

gonzoblasco · 2026-09-03T00:31:19Z

Both points addressed in a0bde03:

Span rendering (3919829010): the heading span now renders node.rendered, matching the real MenuSection component. The hook also uses node.rendered, so both the ARIA labelling and the visible content come from the same source - matching the production code path in @adobe/react-spectrum.
RAC-level migration (5096476622): I explored moving the test to react-aria-components with Virtualizer, as you suggested. Found something worth flagging separately: with a real wrapper, aria-posinset resets per section in the rendered output (item 1 of section 2 reports posinset=2, not the global continuation). That looks like a separate contract issue in the Virtualizer path - the hook-level contract from fix(react-aria): make aria-posinset global across sections in listbox and menu #10487 (item.index is global, posinset matches setsize) holds at the hook level but does not appear to propagate through the Virtualizer layout.

Since that is a different contract and a different fix, I kept the test at the hook level where the original #10487 contract was verified, and left the Virtualizer-level finding as a separate observation rather than mixing it into this PR. Happy to open a follow-up issue for the Virtualizer behavior if it is in scope.

Verified: 5/5 in useMenu.test.tsx, full lint clean.

gonzoblasco mentioned this pull request Sep 2, 2026

fix(react-aria): make aria-posinset global across sections in listbox and menu #10487

Closed

Merge branch 'main' into test/posinset-sections-regression

dda41ce

fix lint

0bddc1d

yihuiliao previously approved these changes Sep 2, 2026

View reviewed changes

yihuiliao added the no testing Does not require manual testing during testing session label Sep 2, 2026

snowystinger reviewed Sep 2, 2026

View reviewed changes

gonzoblasco dismissed yihuiliao’s stale review via 1af6cf2 September 2, 2026 23:06

gonzoblasco requested review from snowystinger and yihuiliao September 2, 2026 23:07

Merge branch 'main' into test/posinset-sections-regression

07939d1

snowystinger reviewed Sep 2, 2026

View reviewed changes

Comment thread packages/react-aria/test/menu/useMenu.test.tsx Outdated

gonzoblasco and others added 2 commits September 2, 2026 20:17

test: use useMenuSection hook in posinset regression

ce5868b

Render each section through the real useMenuSection hook instead of hand-rolling the group structure, so any future logic added to the hook (per-section count restart, references) is exercised by this test.

Merge branch 'main' into test/posinset-sections-regression

3fc8128

gonzoblasco requested a review from snowystinger September 2, 2026 23:21

snowystinger reviewed Sep 3, 2026

View reviewed changes

test: use section title prop for heading in posinset regression

6b65027

Use node.props.title for the section heading (matching how Section renders it) and drop the redundant aria-label - with a heading present, useMenuSection labels the group via aria-labelledby.

snowystinger reviewed Sep 3, 2026

View reviewed changes

Comment thread packages/react-aria/test/menu/useMenu.test.tsx Outdated

snowystinger reviewed Sep 3, 2026

View reviewed changes

yihuiliao reviewed Sep 3, 2026

View reviewed changes

gonzoblasco requested review from snowystinger and yihuiliao September 3, 2026 00:32

snowystinger approved these changes Sep 3, 2026

View reviewed changes

yihuiliao approved these changes Sep 3, 2026

View reviewed changes

Navigation Menu

Uh oh!

test: add regression for global aria-posinset across sections - #10554

test: add regression for global aria-posinset across sections#10554
gonzoblasco wants to merge 10 commits into
adobe:mainfrom
gonzoblasco:test/posinset-sections-regression

gonzoblasco commented Sep 2, 2026

Uh oh!

yihuiliao commented Sep 2, 2026

Uh oh!

snowystinger left a comment

Uh oh!

gonzoblasco commented Sep 2, 2026

Uh oh!

Uh oh!

snowystinger Sep 2, 2026

Uh oh!

snowystinger Sep 2, 2026

Uh oh!

snowystinger Sep 2, 2026

Uh oh!

snowystinger Sep 2, 2026

Uh oh!

gonzoblasco Sep 3, 2026

Uh oh!

gonzoblasco Sep 3, 2026 •

edited

Loading

Uh oh!

Uh oh!

snowystinger left a comment

Uh oh!

yihuiliao Sep 3, 2026

Uh oh!

gonzoblasco commented Sep 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		// The sections must actually render as groups, mirroring the real
		// sectioned menu structure, so this test locks the real DOM shape.

		// aria-posinset should be global (1..4) and match aria-setsize, not restart
		// per section (which would report 1..2 for both groups).

Uh oh!

Conversation

gonzoblasco commented Sep 2, 2026

What

Why

Notes

Uh oh!

yihuiliao commented Sep 2, 2026

Uh oh!

snowystinger left a comment

Choose a reason for hiding this comment

Uh oh!

gonzoblasco commented Sep 2, 2026

Uh oh!

Uh oh!

snowystinger Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

snowystinger Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

snowystinger Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

snowystinger Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

gonzoblasco Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

gonzoblasco Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

snowystinger left a comment

Choose a reason for hiding this comment

Uh oh!

yihuiliao Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

gonzoblasco commented Sep 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gonzoblasco Sep 3, 2026 •

edited

Loading