fix: aggregate token tab auto-enable, network sheet sizing, uncreated-address value, cold-start home images, custom token keyboard dismiss, tooltip close(OK-61863, OK-61860, OK-61879, OK-61505, OK-61527, OK-61417, OK-61524) - #13218
Conversation
A hover tooltip stayed open above a freshly pushed modal (desktop home "Copy address" -> WalletAddress modal) because Tamagui tooltips only close on mouseleave, the pointer does not move when a modal appears, and the tooltip portal always paints above modals. - Tooltip (web): a mouse/pen pointerdown on the trigger closes the tooltip and keeps it closed until the pointer leaves the trigger. Touch is exempt because a tap is the only way to reveal a tooltip there. Uses pointerdown instead of onPressIn, which fires for both mousedown and touchstart. - tooltipRegistry: track open tooltips and expose closeAllTooltips(); useAppNavigation.pushModalPage calls it so shortcut- or event-driven modals never open under a lingering tooltip. - Unit tests for the registry, the open-state hook, and the navigation hook; desktop verified over CDP.
…ructorName (OK-61417)
…ate token selector (OK-61879)
weatherstar
changed the title
|
@codex review |
|
@codex security review |
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. |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
| const sheetScrollViewMaxHeight = isFitSheet | ||
| ? Math.max( | ||
| 0, | ||
| Math.floor(viewportHeight * FIT_SHEET_MAX_HEIGHT_RATIO) - | ||
| sheetHeaderHeight - | ||
| (bottom || SHEET_BOTTOM_MARGIN), | ||
| ) |
There was a problem hiding this comment.
Keyboard-open fit sheets overflow
Severity: non-severe
When a tall fit sheet opens the keyboard, sheetScrollViewMaxHeight reserves no keyboard space. The frame adds keyboardHeight separately, while the token selector auto-focuses its search field. Its header or bottom rows can leave the viewport.
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
2 participants
OK-61863 OK-61860 OK-61879 OK-61505 OK-61527 OK-61417 OK-61524
Summary
constructorName(regression guard for the "loading forever" RPC hang).Intent & Context
QA reported a batch of App-6.6.0 wallet issues, most of them in the aggregated token flows on Android:
Root Cause
onPageSelected(target)at the start of a programmatic smooth scroll. Thereact-native-collapsible-tab-viewpatch treated that as arrival and cleared itsprogrammaticPageTargetguard, so the following absoluteonPageScrollpositions (0→4) propagated every rounded transit index throughonIndexChange, and the page auto-enabled each transit network. iOS is unaffected: its offset is interpolated by|destination − current|andonPageSelectedfires at the end. (2) The library's index-propagation reaction has fixed deps, so theonIndexChangeclosure is frozen at first render;handleTabIndexChangekept reading the pre-enableallNetworksStatesnapshot and toasted again for networks it had already enabled.sheetProps={{ snapPoints: [92], snapPointsMode: 'percent' }}, so the native sheet was always 92% tall regardless of content. Switching to the defaultfitmode alone is not enough: Tamagui caps the fit-modeSheet.ScrollViewat the full screen height, so header + list overflowed the screen (header under the status bar, last rows clipped) — reproduced on the emulator with USDT's ~20 members.AggregateTokenListItemrendered the balance/value column unconditionally; rows for networks with no derived address have no home sub-token entry andCurrencymapsundefinedto'0', hence "$0.00". The old account lookup returned a barestring | undefined, so "pending" and "no address" were indistinguishable.Page scrollEnabled+keyboardDismissMode="on-drag"/keyboardShouldPersistTaps="handled").mouseleave; the pointer does not move when a modal appears and the tooltip portal always paints above modals.Design Decisions
handlePageSelectednow keeps the programmatic target alive until the pager visually reaches the page (Math.round(indexDecimal) === selectedIndex), while still syncingindeximmediately so the tab highlight stays instant. The guard is released by the reaction at visual arrival, by a non-settlingscroll state, or by the iOS fallback timer (which now also syncsindexDecimal). Landing on a different page (user interrupted the scroll) still releases the target, so swipes are never blocked.enableNetworkInAllNetworksOncereadsgetAllNetworksState()from the background at decision time and dedupes in-flight networks;handleTabIndexChangereadsaggregateTabsthrough a ref. Any nativeTabs.ContaineronIndexChangeconsumer must not rely on closure freshness.Popover(native and web files are separate), measure the sheet header viaonLayoutand passmaxHeight = floor(viewportHeight × 0.92) − headerHeight − (bottomInset || 20)toSheet.ScrollViewwhen the sheet is in fit mode. Short lists stay compact, long lists cap at the previous 92% footprint and scroll. Sheets that explicitly pass percent/constant snap points are untouched.{ accountId }so the value column is hidden only once the lookup settles without an account; while pending it stays put, so rows that do have an address never blink their balance in after the first frame (Android modals open without animation, so first-frame round trips are visible). Rows with an address on a disabled network still show "$0.00" (product accepted).pointerdown. Mouse/penpointerdownon the trigger closes the tooltip and keeps it closed until the pointer leaves; touch is exempt because a tap is the only way to reveal a tooltip there.pushModalPagecallscloseAllTooltips()via a small registry.Changes Detail
patches/react-native-collapsible-tab-view+8.0.1.patch: programmatic-target lifecycle inhandlePageSelected/handlePageScrollStateChanged/ iOS fallback; the index reaction releases the guard at visual arrival even whenindexwas already synced.packages/kit/src/views/AssetDetails/pages/TokenDetails/index.tsx: tab-change auto-enable uses the new helper + refs; contract-address popover drops the 92% snap.packages/kit/src/views/AssetDetails/pages/TokenDetails/tokenDetailsNetworkAutoEnable.ts(+ test): live-state, single-flight enable helper.packages/kit/src/views/AssetDetails/pages/TokenDetails/TokenDetailsTabToolbar.tsx: "Select network" popover drops the 92% snap.packages/components/src/actions/Popover/index.tsx,index.native.tsx: header-measuredSheet.ScrollViewcap for fit-mode sheets.packages/kit/src/views/AssetSelector/pages/AggregateTokenSelector.tsx(+ test): settled account lookup; value column hidden on networks without an address.packages/kit/src/views/Home/hooks/useHomeWalletTabSupport.ts,packages/kit/src/utils/coldStartImagePreload.ts,WalletBanner.tsx,homeWalletTabSupportUtils.ts,swrCacheUtils.ts(+ tests): cold-start snapshot and banner prewarm.packages/kit/src/views/AssetList/pages/AddCustomTokenModal.tsx(+ test): scrollable page with keyboard dismiss on drag / handled taps.packages/components/src/actions/Tooltip/*,packages/kit/src/hooks/useAppNavigation.ts(+ tests): pointerdown latch, tooltip registry, close-all on modal push.packages/kit-bg/src/.../setupBackgroundThreadRPCHandler.test.ts,setupMainThreadBackgroundRunner.test.ts: regression tests for OK-61417.Risk Assessment
Tabs.Containerconsumer (market tabs, home tabs, token details). Programmatic jumps now propagate one index change at the early AndroidonPageSelected(highlight still instant) instead of one per transit page; iOS behavior is unchanged except the fallback timer also syncsindexDecimal.Test plan
AggregateTokenSelector.test.tsx(3),tokenDetailsNetworkAutoEnable.test.ts(5), tooltip registry / open-state / navigation hook tests,AddCustomTokenModal.test.tsx,useHomeWalletTabSupport+coldStartImagePreloadtests, OK-61417 RPC rehydration tests.yarn agent:check --profile commitgreen on every commit.react-native-collapsible-tab-view@8.0.1.