Repository navigation
Conversation
🦋 Changeset detectedLatest commit: 4ecea72 The changes in this PR will be included in the next version bump. This PR includes changesets to release 85 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
lord-0011
changed the title
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3567
📝 Description
scrollToHighlightedItemscrolled the highlighted item into view withscrollIntoView(itemEl, { rootEl: contentEl, block: "nearest" }), which calls the nativeelement.scrollIntoView(). The native method scrolls every scrollable ancestor, including the document, so an open combobox with a highlighted item could scroll the whole page on mount even when the item was already visible in its own list.This PR switches both
scrollToHighlightedItemactions in the combobox machine toscrollToNearest(itemEl, { rootEl: contentEl })from@zag-js/dom-query(added in #3542 for the same problem in TOC). It only scrolls the content element, and does nothing when the item is already visible.⛳️ Current behavior (updates)
An inline combobox rendered below the fold with
openanddefaultHighlightedValuescrolls the document on mount (in the new example page,window.scrollYgoes from 0 to 463) without any user input.🚀 New behavior
scrollToIndexFnpath is untouched.Changes:
packages/machines/combobox/src/combobox.machine.ts: usescrollToNearestinstead ofscrollIntoViewin bothscrollToHighlightedItemactions.examples/next-ts/pages/combobox/highlighted-on-mount.tsxand route entry, following the pattern of fix(toc): only auto-scroll the toc's own scroll container #3542.window.scrollYand the content'sscrollTopstay 0).@zag-js/combobox.Note on the example page: the example app's layout sets
overflow: hiddenonbody/.page, so the document can't scroll there. The page overrides that (html, body, .page, .page main { height: auto; overflow: visible }) so it reproduces the issue the way the reporter saw it, with the document as the scroll container. Happy to change this if you'd prefer a different setup.💣 Is this a breaking change (Yes/No):
No.
📝 Additional Information
Testing:
main(Expected: 0, Received: 463forwindow.scrollY) and passes with this change.pnpm e2e-react -- e2e/combobox.e2e.ts: 54/54 pass.pnpm typecheck,pnpm lintand Prettier pass.Select, Listbox, Menu and Cascade Select use the same
scrollIntoView(itemEl, { rootEl: contentEl, block: "nearest" })pattern. I kept this PR to the combobox as reported; happy to open a follow-up for those if you'd like.AI disclosure: I used an AI coding assistant to help prepare this change. I reviewed the diff line by line, and confirmed the new test fails without the fix and passes with it.