fix: Scroll GridList to focused drop indicators - #10437
Conversation
|
Looks like the PR is failing lint and the browser tests. As a reminder to your AI, you can run the browser tests with |
df4d27d to
0621269
Compare
|
Fixed the lint failure by wrapping the programmatic focus in
|
|
If you are unable to see the browser test error, here it is |
Addressed the remaining lint failure in `packages/react-aria-components/test/GridList.browser.test.tsx` by wrapping programmatic focus in React’s `act()`. === CODEX STATUS === STATUS: PASS_WITH_ENV_BLOCKED CHANGES: - 1 file changed, 2 insertions, 2 deletions. - No upstream files materialized and no rebase attempted. VERIFICATION: - PASS: `yarn lint`
0621269 to
f466349
Compare
The browser environment renders aria-labels as raw ICU templates, so [aria-label="Drag Item 0"] matched nothing and the test dereferenced null. Selects the handle by slot and asserts aria-roledescription instead, and drops act(), which browser mode does not support.
|
@snowystinger thanks for pasting the failure, that was the whole thing. Fixed in a7dc05e. The cause was not the selector being stale. In the browser environment aria-labels render as raw ICU templates: the drag handle is literally The test now selects the handle by Verified against chromium locally: 4/4 in the file. I also checked the test is not vacuous by removing the One thing worth flagging beyond this PR: since the labels come through uninterpolated, no browser test in the suite can assert a localized string today. Happy to open a separate issue if that is news. |
Closes
✅ Pull Request Checklist:
📝 Test Instructions:
Give the rendered
GridListDropIndicatorrow a relative default position so its visually hidden, absolutely positioned focus target is anchored at the indicator's location in the collection; follow the equivalent positioning pattern already used byTreeDropIndicator. Keep the change inside the existing render-props/default-style path so consumer-provided styles continue to compose normally and no new helper or API surface is introduced. Add a real-browser regression in the existing GridList browser suite because jsdom cannot validate native focus-driven scrolling: render a reorderable list taller than a constrained scrollport, start keyboard dragging, navigate to an initially off-screen drop target, and verify both focus and the scroll offset.Closes #6492
🧢 Your Project:
Not applicable to this change.