Skip to content

FIX: Drawer - Restore only the scroll offset of selector grids - #1275

Closed
HugoDataAnalyst wants to merge 3 commits into
WatWowMap:mainfrom
HugoDataAnalyst:fix/drawer-grid-restore
Closed

HugoDataAnalyst wants to merge 3 commits into
WatWowMap:mainfrom
HugoDataAnalyst:fix/drawer-grid-restore

Conversation

@HugoDataAnalyst

@HugoDataAnalyst HugoDataAnalyst commented Oct 30, 2026 •

Copy link
Copy Markdown

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 SelectorList restored its position twice (8950dd5): Virtuoso's restoreStateFrom snapshot rendered the saved range, and useDrawerScrollMemory then set the scroller's scrollTop in 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 prettier on the changed files
  • Manual: Tappables scrolled down, close / reopen the drawer repeatedly, including after the available rewards changed: tiles always render (previously blank until toggled)
  • Manual: open / resize the window / close / reopen, tiles always render

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-30T19:02:21.249971Z 9aebdbe PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@HugoDataAnalyst HugoDataAnalyst changed the title FIX: Drawer - only restore a selector grid's state while its item count is unchanged FIX: Drawer - Restore only the scroll offset of selector grids Oct 30, 2026
@HugoDataAnalyst

Copy link
Copy Markdown
Author

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 useDrawerScrollMemory's scrollTop RAF both restored the position, and when they disagreed Virtuoso rendered an off-screen range.

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.

@Mygod Mygod closed this in 3c493b6 Oct 31, 2026
github-actions Bot pushed a commit that referenced this pull request Oct 31, 2026
## [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)
@github-actions

Copy link
Copy Markdown

🎉 This issue has been resolved in version 1.56.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant