Repository navigation
FIX: Drawer - Restore only the scroll offset of selector grids - #1275
HugoDataAnalyst wants to merge 3 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9aebdbe4c3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // read once per mount, the grid only applies it when it mounts | ||
| const restoreStateFrom = React.useMemo( | ||
| () => getDrawerGridState(listScrollKey), | ||
| () => getDrawerGridState(listScrollKey, items.length), |
There was a problem hiding this comment.
Gate stale scroll offsets when the item count changes
When the count changes but the old offset is still within the new list's scroll range (for example, tappables grows by one), this call returns null, yet useDrawerScrollMemory(listScrollKey, shouldPersistGridState) immediately below still restores scrollTopMemory on the Virtuoso scroller in its RAF (useScrollMemory.js:46-56). The changed grid therefore reopens at the stale offset rather than at the top, so the count guard disables only one of the two restoration mechanisms; gate or clear the scroll-top entry on the same mismatch.
Useful? React with 👍 / 👎.
|
Updated the approach in 342ad90, so the earlier commits here are superseded. Gating the snapshot on the item count (9aebdbe, 8f5cebe) fixed a list whose items changed, but the grid still came up blank when it had been scrolled and the list was unchanged. That pointed at the restore itself: the snapshot and The grids now restore only the scroll offset and let Virtuoso work out the range from it, which covers both cases, so the snapshot and the count check are gone. Tested on a live instance: no more blank tiles across close / reopen / resize, scrolled or not. Happy to squash into one commit if you prefer. |
## [1.56.1](v1.56.0...v1.56.1) (2026-10-31) ### Bug Fixes * **drawer:** stop scrolled selector grids from reopening blank ([3c493b6](3c493b6)), closes [#1275](#1275)
|
🎉 This issue has been resolved in version 1.56.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
Reopening the drawer could leave a selector grid blank: the "Set All" row and the space for the grid were there, but no tiles were drawn until the section was toggled off and on. It happened when the grid had been scrolled.
The drawer unmounts its content when closed, so each reopen mounts a new grid, and
SelectorListrestored its position twice (8950dd5): Virtuoso'srestoreStateFromsnapshot rendered the saved range, anduseDrawerScrollMemorythen set the scroller'sscrollTopin a RAF. When the two disagreed, the grid drew a range that wasn't on screen.The grids now restore only the scroll offset, like the scan area table already does. Virtuoso derives the rendered range from the scroller's real position, so it can't render an off-screen window, and a list whose items changed in between (tappable rewards come and go) no longer restores a stale snapshot either. The grid state snapshot (
getDrawerGridState/setDrawerGridState) is removed as unused.Testing
yarn lint,yarn prettieron the changed files