Skip to content

fix: keep Tabs scroll extent in sync after tab switches and async refreshes (OK-57257 OK-57977)#12530

Open
huhuanming wants to merge 1 commit into
release/v6.5.0from
codex/fix-defi-scroll-extent
Open

fix: keep Tabs scroll extent in sync after tab switches and async refreshes (OK-57257 OK-57977)#12530
huhuanming wants to merge 1 commit into
release/v6.5.0from
codex/fix-defi-scroll-extent

Conversation

@huhuanming

@huhuanming huhuanming commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

OK-57257 OK-57977


Summary

  • fix stale scroll extents in the shared web/desktop Tabs.Container
  • cover tab round trips, account/network-driven content changes, and DeFi data restoration after an app relaunch
  • replace the one-time direct-child observer snapshot with live structural tracking for every Tabs.ScrollView
  • remove the DeFi-only height selectors, cached values, and delayed retries
  • keep virtualized Tabs.List pages on their existing explicit-height contract

Reported problems

OK-57257 — DeFi cannot reach the bottom after switching tabs

The issue video starts on a tall DeFi page, switches to the shorter NFT tab, and then returns to DeFi. Wheel scrolling stops at an older, shorter extent, leaving the lower protocol sections and Support Hub cards unreachable.

The layout transition is tall -> short -> tall. Restoring the DeFi scene can increase its live scrollHeight without rebuilding the shared horizontal Tabs list at the same height.

OK-57977 — the active DeFi page cannot reach the bottom after an account/network change

The Jira video shows the same symptom without requiring a tab change:

  1. Stay on DeFi.
  2. Change the account or network context (the video includes Arbitrum -> Ethereum).
  3. A different, often taller, set of positions replaces the current content.
  4. Wheel scrolling stops before the final Support Hub cards.
  5. A later tab/network change can temporarily restore the correct bottom.

This is a different trigger from OK-57257, but the same shared layout failure: the focused tab receives taller async content while the outer Tabs list retains an earlier height.

QA retest — relaunch while DeFi is active

QA reported an additional stable path after testing the first version of this PR:

  1. Immediately after updating, DeFi/NFT tab switching and scrolling work.
  2. Leave the app on DeFi.
  3. Exit and relaunch the desktop app.
  4. DeFi data restores asynchronously without a subsequent tab focus cycle.
  5. The DeFi list once again stops before the real bottom.

The attached logs show the Home UI initializing first, followed several seconds later by the supported-protocol and position responses under the configured slow-4G profile. This makes the first DeFi measurement an initial/skeleton measurement; later data changes the DOM while DeFi remains focused.

Why the first shared fix was still incomplete

The first revision correctly moved the workaround out of DeFiContainer and made the registered Tabs.ScrollView root the live measurement source. It also attached ResizeObserver to the root and its direct content nodes.

However, those direct nodes were captured only once when the observer was attached. On relaunch or another async refresh, React can add or replace content after that snapshot:

  • the old node remains observed even after it is detached
  • the new node is not a ResizeObserver target
  • the Tabs.ScrollView border box can remain pinned by its parent layout
  • only the root's scrollHeight changes, which is not itself a ResizeObserver notification
  • no tab focus change occurs to re-run observer attachment

This explains QA's sequence precisely: switching tabs after an update re-attached the observer and temporarily fixed the page; relaunching into a fresh async DeFi load recreated the stale snapshot.

Root cause and relationship between the issues

Path Layout event Why the old extent survives
OK-57257 tall DeFi -> short NFT -> restored DeFi focused content grows after the shared list was sized for a shorter scene
OK-57977 account/network changes while DeFi stays focused focused content is replaced/grows without a focus cycle
QA relaunch initial/skeleton DeFi -> restored positions observer targets are snapshotted before async structure settles

All three paths increase a focused tab's live content extent while the shared horizontal list still owns an older numeric height. The failure belongs to Tabs.Container, not DeFi margin/padding and not a background-service scroll state.

Fix

For registered Tabs.ScrollView pages, Tabs.Container now combines two observers with separate responsibilities:

  1. ResizeObserver watches the root and its current direct content nodes for intrinsic size changes such as image, font, and layout resolution.
  2. MutationObserver watches structural childList changes in the focused ScrollView subtree.
  3. After a structural change, the container recomputes the live direct-child set, disconnects stale targets, and observes newly inserted/replaced nodes.
  4. It then re-reads the registered root's current scrollHeight and writes that live value to the shared horizontal list.
  5. Both observers are disconnected when the focused measurement source changes or the container unmounts.

The mutation observer intentionally excludes attribute/style observation, so updating the list height cannot feed back into the observer. The fix is generic to every flow-content tab using Tabs.ScrollView; business pages do not need selectors, focus-specific timers, or height reports.

The previous DeFi-specific registration is removed. In particular, it no longer caches a sampled DeFi height that can override later live DOM measurements.

Stable reproduction script

Run against the desktop client on CDP port 9222:

REGRESSION=1 node scenarios/regression.mjs tabs-scroll-extent-desktop

The scenario now validates three phases:

  1. Enter DeFi at most once and scroll to the bottom before any DeFi -> NFT -> DeFi round trip. This catches the relaunch/initial-load failure instead of masking it by re-attaching the observer.
  2. Repeat DeFi -> NFT -> DeFi with real wheel input for the original OK-57257 transition.
  3. Pin the ScrollView border box and insert a new direct content node. This deterministically models a skeleton/data node replacement and asserts that both the shared list height and wheel extent grow.

The CDP connection also waits for the OneKey renderer to finish booting, so the scenario can be started together with the app instead of racing the initial page load.

Before/after evidence

Before this revision

On the first shared-fix implementation, adding a 160 px direct child produced:

  • ScrollView border height: 5178 px
  • ScrollView live scrollHeight: 5338 px
  • shared Tabs list height: 5178 px
  • unmeasured/unreachable content: exactly 160 px

The new node was absent from the one-time ResizeObserver target snapshot.

After this revision

With the deterministic 96 px probe:

  • shared Tabs list: 5178 -> 5274 px
  • wheel max extent: 5043 -> 5139 px
  • final wheel position: 5139 / 5139
  • unmeasured content: 0 px

The structural observer detects the insertion, refreshes the target set, and measures the live root extent.

App relaunch validation

Validation used the real release-based Electron client with desktop network throttling enabled (slow4g, 562.5 ms latency).

After a full process exit and restart (isSoftRestart: false), the scenario waited for DeFi's async content, then checked the bottom before any tab round trip:

  • initial load: scrollTop = maxScrollTop = 5043
  • shared list / ScrollView: 5178 / 5178 px
  • hidden content: 0 px
  • unmeasured content: 0 px
  • result: coldStartClipped=false

The subsequent DeFi -> NFT -> DeFi check and direct-node growth probe also passed. A separate full run repeated the tab round trip 8 times; all 8 were clean.

QA acceptance

Please recheck all three routes with wheel input:

  1. DeFi -> NFT -> DeFi on a page taller than the viewport.
  2. Stay on DeFi and switch between accounts/networks with substantially different position heights.
  3. Leave DeFi active, exit/relaunch the app, wait for positions to restore, and scroll to the final Support Hub card without first switching tabs.

Repeat long -> short -> long data transitions where possible. None of the routes should require a recovery tab switch.

Checks

  • based directly on current release/v6.5.0 at 2ebdd41fdd
  • exactly 1 PR commit (c4772776fe) and 4 changed files
  • node --check scenarios/regression.mjs
  • JS oxlint --deny-warnings: 0 warnings, 0 errors
  • type-aware TS oxlint --deny-warnings: 0 warnings, 0 errors
  • full tsgo -p ./tsconfig.json --noEmit: passed
  • git diff --check: passed
  • Electron/CDP regression, full process restart + slow4g: REGRESSION PASS
  • Electron/CDP DeFi -> NFT -> DeFi, 8 rounds: 8/8 clean

yarn agent:check --profile commit cannot spawn its child commands correctly on this Windows checkout (exitCode: null in 0-3 ms). yarn lint:staged also relies on Unix-only grep/xargs. The corresponding oxlint, syntax, diff, and tsgo checks above were run directly and passed.

@huhuanming
huhuanming force-pushed the codex/fix-defi-scroll-extent branch from aef493d to 10223a7 Compare July 17, 2026 03:29
@huhuanming huhuanming changed the title fix: keep DeFi scroll extent in sync fix: restore shared Tabs scroll extent tracking Jul 17, 2026
@huhuanming huhuanming changed the title fix: restore shared Tabs scroll extent tracking fix: keep Tabs scroll extent in sync after tab and account/network changes Jul 17, 2026
@huhuanming
huhuanming marked this pull request as ready for review July 17, 2026 06:09
@huhuanming
huhuanming enabled auto-merge (squash) July 17, 2026 06:18
@huhuanming
huhuanming force-pushed the codex/fix-defi-scroll-extent branch from 232f949 to 84c4b39 Compare July 17, 2026 06:56
@huhuanming
huhuanming changed the base branch from x to release/v6.5.0 July 17, 2026 06:59
@huhuanming
huhuanming force-pushed the codex/fix-defi-scroll-extent branch from 84c4b39 to ab8ee16 Compare July 17, 2026 07:00
@huhuanming huhuanming added the release-ready This PR should be included in the next release label Jul 17, 2026 — with ChatGPT Codex Connector
@huhuanming huhuanming removed the release-ready This PR should be included in the next release label Jul 17, 2026
@huhuanming huhuanming changed the title fix: keep Tabs scroll extent in sync after tab and account/network changes fix: keep Tabs scroll extent in sync after tab and account/network changes(OK-57257 OK-57977) Jul 17, 2026
@huhuanming
huhuanming force-pushed the codex/fix-defi-scroll-extent branch from ab8ee16 to c477277 Compare July 17, 2026 12:55
@huhuanming huhuanming changed the title fix: keep Tabs scroll extent in sync after tab and account/network changes(OK-57257 OK-57977) fix: keep Tabs scroll extent in sync after tab switches and async refreshes (OK-57257 OK-57977) Jul 17, 2026
@huhuanming huhuanming added release-ready This PR should be included in the next release and removed release-ready This PR should be included in the next release labels Jul 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant