feat(discover): discover v2 swipe nav + cutover#7553
Open
DanielSinclair wants to merge 29 commits into
Open
Conversation
This was referenced Jun 3, 2026
Contributor
Author
This was referenced Jun 3, 2026
DanielSinclair
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 626e66c502
ℹ️ 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".
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".
DanielSinclair
force-pushed
the
daniel/discover-v2-sparkline-pointer
branch
from
2d9b47d to
8557323
Compare
DanielSinclair
force-pushed
the
daniel/discover-v2-swipe-nav-cutover
branch
from
626e66c to
5b69baa
Compare
DanielSinclair
force-pushed
the
daniel/discover-v2-sparkline-pointer
branch
from
8557323 to
ac49258
Compare
DanielSinclair
force-pushed
the
daniel/discover-v2-swipe-nav-cutover
branch
from
5b69baa to
e0e844e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0e844eb01
ℹ️ 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".
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".
DanielSinclair
added a commit
that referenced
this pull request
Jun 4, 2026
…library The original branch bundled cutover runtime, new V2 analytics, perps coupling, and dead exports into one ~2.7k-line change that nothing mounted, which review flagged as unscoped dead code spanning several concerns. Root cause: the card/layout island, its pager/header runtime, the analytics event schema, the liveTokens split, and source-specific price math all landed in one commit, so the branch read as multiple scopes and coupled the generic cards directly to perps unit math. Narrow #7539 to just the unmounted card/layout/section library that the upstack source renderers (#7540 token, #7541 perp, #7542 prediction) and the cutover (#7553) consume: - Move cutover runtime to #7553: delete DiscoverHeader, DiscoverSectionsPager, discoverNavigationStore, and navigation.ts; revert SmoothPager.fillHeight and the DiscoverScreenContext section-ref machinery (now byte-identical to develop); restore the legacy scrollViewRef scroll-to-top fallback, fixing the live single-scrollview regression. - Make SectionLayout callback-only via onPressSeeAll; delete getHeaderPress and the unused getInitialRenderedItemCount; compute showHeaderCaret once. - Strip new V2 analytics: event.ts and trackInteraction.ts revert to develop; the render-callback contract is now (item, width) for carousel/grid and (item) for list, with no analytics arg or DiscoverCardAnalyticsContext. - Decouple generic cards from perps: new src/framework/ui/price/formatPriceChange.ts owns the arrow glyphs and the percent-unit formatter ("5.23" => 5.23%, no 10_000 factor); MarketPill/MarketPriceChange no longer import @/features/perps. - Clean MarketDisplayItem: drop onNavigate, pressMetadata, chartMaxPoints. - Remove dead placement/surfaceId plumbing from the generic layout layer. - Add focused tests: computeSnapToOffsets, formatNormalizedPercentChange, isEventCardDisplay.
DanielSinclair
added a commit
that referenced
this pull request
Jun 4, 2026
…t + fixes Restacked onto the slimmed #7539 library and reconciled with its new source-neutral contract, then applied the review-driven token cleanups. Cascade adaptation (compile against #7539): - Drop the analytics plumbing inherited from the old contract: remove DiscoverCardAnalyticsContext, the analyticsContext renderItem arg, and the card analyticsContext props; the token descriptors now match the generic (item, width) / (item) callback signatures. - Remove navigateDiscoverDestination (deleted in #7539); the token "See All" header press uses onTapSearch() only, gated to token-owned destinations. Non-token CMS routing is deferred to the #7553 cutover. - Stop setting onNavigate/pressMetadata on the token MarketDisplayItem (those fields were removed from the type); drop the dead placement/surfaceId arguments from the renderSectionLayout call. Token-renderer fixes: - Fix a 10_000x price-change bug. The token helper divided relativeChange24h by 10_000, which only worked because the generic card multiplied it back; #7539 removed that multiply, so the divide now under-reported every token change as ~0.00%. The token API already returns percent units ("5.23" == 5.23%), so normalizeTokenPercentChange now passes the value through with a nullish + finite guard and no scaling. - Guard optional asset.price (price?.relativeChange24h/value/updatedAt); missing change becomes "0" and missing value a formatted zero. - Use Number(x ?? 0) instead of || 0 to preserve valid zero values. - Delegate token chart color to the canonical getPriceChangeColor. - Normalize token chart timestamps to seconds at the source boundary via normalizeTokenChartTimestampSeconds. - Make a forced Discover refresh bypass the module-level token-ref cache via clearTokenRefCache so it refetches inside TOKEN_REFS_STALE_TIME. - Extract aggregateLineChartFetches (keeps partial successes, records the first error, throws only if every fetch failed) and share it between the token and Hyperliquid line-chart stores.
DanielSinclair
added a commit
that referenced
this pull request
Jun 4, 2026
… + fixes Restacked onto the cleaned #7540 and reconciled with the source-neutral contract, then applied the perp cleanups and review fixes. Cascade adaptation (compile against the new contract): - Drop the analytics plumbing (DiscoverCardAnalyticsContext, the analyticsContext renderItem arg and card props); perp descriptors now match the generic (item, width) / (item) callbacks. - Remove navigateDiscoverDestination; the perp "See All" header press uses onTapSearch() gated to perp-owned destinations, mirroring the token path. Non-perp CMS routing is deferred to the #7553 cutover. - Stop setting onNavigate/pressMetadata on the perp MarketDisplayItem and drop the dead placement/surfaceId arguments to renderSectionLayout. Perp-renderer fixes: - Fix the perp price-change units. Perps store priceChange['24h'] with an extra /100 baked in (so a +5.23% move is "0.000523"), and the Hyperliquid live adapter feeds change24hPct from that same stored value. The old generic card multiplied by 10_000; #7539 removed that, so usePerpMarketDisplay now converts to percent via convertStoredPerpPriceChangeToPercent for the initial value, the live priceChangeSelector, and the pill-width input. Importing the perps util here is correct -- this is the perp source hook, not a generic card. - Restore the forced token-ref cache bypass. The Hyperliquid refresh addition had dropped clearTokenRefCache() from the rainbow branch of refreshDiscoverSurface; without it a forced refresh within TOKEN_REFS_STALE_TIME serves stale token data and the helper is dead. Re-add the cache clear and keep the force-aware Hyperliquid refetch alongside it. Keep perps_enabled fallback true and the token/perp renderers as separate source-specific code (no consolidation), per plan.
…stack runtime moved here. This restores that runtime so the cutover compiles and mounts the V2 surfaces, and wires the generic destination routing. Re-add the runtime moved out of #7539 (step 31): - discoverNavigationStore and the section-scroll refs it backs in DiscoverScreenContext, restoring active-section scroll-to-top. - navigation.ts / navigateDiscoverDestination. - DiscoverHeader, with its new V2 tab analytics stripped out -- analytics belongs to the dedicated top analytics branch; the tab-press handler keeps its navigation/scroll side effects. - SmoothPager.fillHeight, used by DiscoverSectionsPager. Wire generic CMS destination header presses now that all source renderers exist (step 31): prediction and perp section "See All" route through navigateDiscoverDestination (predictions -> Polymarket, perps -> perps screen); the token section keeps onTapSearch (token "See All" opens Discover search, and navigateDiscoverDestination is intentionally a no-op for tokens). Preserve the resolved cutover fixes (step 32): DiscoverSearch results offset by DISCOVER_HEADER_HEIGHT, pull-to-refresh state owned by DiscoverRefreshControl with no screen-level isRefreshing and no renderRefreshControl prop chain. No new V2 analytics here and event.ts stays byte-identical to develop (step 33).
These symbols are still consumed by the v1 placement stores and carousel components, which are deleted in the next PR. Removing them here breaks standalone compilation of this branch.
Reanimated can fail to resolve a host instance on Android when the Discover section Animated.ScrollView mounts with a UI-thread scroll handler. Use a plain ScrollView with a JS scroll handler on Android while keeping the animated scroll path on iOS, and narrow the stored section ref to the scrollTo surface used by the header.
Android ScrollView wraps the native scroll view by cloning the provided refreshControl and passing the scroll content as children. DiscoverRefreshControl owned the refresh state but did not forward those injected children, so switching Discover sections to a plain Android ScrollView avoided the Reanimated crash but dropped the page body. Forward the injected style and children into RefreshControl so Android keeps the ScrollView content while preserving Discover's refresh behavior.
MarketTileCard accepted an onPress prop but never forwarded it to ButtonPressAnimation, so tile cards rendered as tappable while dropping their destination handler. Pass the handler through to match the list and pill market card variants.
The floating swipe tab bar shell used pointerEvents=box-none, so only child icon pressables handled touches and gaps inside the visible bar could fall through to the Wallet screen. Let the visible tab bar container receive pointer events while preserving the existing animated pointerEvents style that disables the bar during browser tab-view transitions.
Discover uses SmoothPager with fillHeight as a full-screen tab surface, but the pager wrapper still inherited box-none hit testing. Let the existing fillHeight mode receive touches so horizontal pans can start in blank For You space without adding a new pager prop or touching DiscoverSectionsPager.
MarketTileCard rendered light-mode cards with a transparent body, so Android elevation exposed the Discover surface through Perps tiles. Give the tile body the same translucent white light surface used by the compact Perps market card while keeping the shadow host shadow-only.
DanielSinclair
force-pushed
the
graphite-base/7553
branch
from
4528216 to
dbef118
Compare
DanielSinclair
force-pushed
the
daniel/discover-v2-swipe-nav-cutover
branch
from
bd6ab9a to
f817e16
Compare
Android elevation draws the host view as the visible shadow surface. Give the light market tile shadow host the same rounded design-system white surface as the card and use the Perps-equivalent black shadow token so Android no longer shows an inner tile box while iOS keeps rendering the card body.
Market tile shadows extend past the carousel item bounds, but the carousel only accounted for vertical bleed. Add an optional horizontal bleed to the existing descriptor path and opt market tiles into the same named bleed so edge-card shadows are not clipped.
Small prediction tiles render through PolymarketEventsListItem, so apply the matching rounded shadow style through its existing style prop instead of changing the shared Polymarket component. Opt the prediction tile carousel into the same bleed so the new shadow is visible at the carousel edges.
The browser tab is rendered as a content-width column so it can shrink into back/forward controls. Its resting width was still the fixed five-tab pill width, which made four-tab layouts allocate a smaller browser column than the tab-position math expected. Use the computed tab slot width for the resting browser tab and the matching browser-control bar resize delta.
DanielSinclair
force-pushed
the
daniel/discover-v2-swipe-nav-cutover
branch
from
f817e16 to
b489343
Compare
Comment on lines
+20
to
+23
| navigate: section => { | ||
| if (get().activeSection === section) return; | ||
| set({ activeSection: section }); | ||
| }, |
Member
There was a problem hiding this comment.
Would change to:
Suggested change
| navigate: section => { | |
| if (get().activeSection === section) return; | |
| set({ activeSection: section }); | |
| }, | |
| navigate: section => | |
| set(state => { | |
| if (state.activeSection === section) return state; | |
| return { activeSection: section }; | |
| }), |
Comment on lines
28
to
+32
| export const usePerpsEnabled = createDerivedStore<boolean>( | ||
| $ => $(useRemoteConfigStore, state => state.getRemoteConfigKey('perps_enabled')) && !IS_TEST, | ||
| { fastMode: true } | ||
| $ => $(useRemoteConfigStore, state => state.getRemoteConfigKey('perps_enabled')), | ||
| { | ||
| fastMode: true, | ||
| } |
Member
There was a problem hiding this comment.
Fine to handle in a follow-up but this derived store is unnecessary, it's not deriving anything. Can just use the remote config store directly.
Comment on lines
+35
to
+48
| # Navigate the Discover v2 tabs by POSITION. Number-based IDs are stable across the | ||
| # server-driven surface, and page assertions verify the active tab content changed. | ||
| - extendedWaitUntil: | ||
| visible: | ||
| id: tab-1 | ||
| timeout: 30000 | ||
| - tapOn: | ||
| id: tab-2 | ||
| - assertVisible: | ||
| id: discover-section-page-2 | ||
| - tapOn: | ||
| id: tab-1 | ||
| - assertVisible: | ||
| id: discover-section-page-1 |
Member
There was a problem hiding this comment.
Looks like this is failing? In the CI screen recording artifact, it displays the first page correctly but doesn't switch to the second page.
Could just remove so this doesn't block merging.
Member
There was a problem hiding this comment.
This could be a bit stricter, which destinations are accepted for each section type.
I see some of this code differs in #7557, so if anything would apply this outside of this PR as a follow-up.
Might do something like:
type DestinationForRoot<Root extends DestinationRoot> = [Root, ...string[]];
type NavigableDiscoverDestination = DestinationForRoot<'perps'> | DestinationForRoot<'predictions'>;
export function navigateDiscoverDestination(destination: NavigableDiscoverDestination): void {
const [root, ...segments] = destination;
switch (root) {
case 'predictions': {
const [category, league] = segments;
if (category === 'sports') navigateToPolymarketSportsLeague(league ?? DEFAULT_SPORTS_LEAGUE_KEY);
else if (category) navigateToPolymarketCategory(category);
else navigateToPolymarket();
return;
}
case 'perps':
if (segments.length) navigateToPerpsSearch();
else navigateToPerps();
return;
}
}
export function hasDestinationRoot<Root extends DestinationRoot>(
destination: Destination,
root: Root
): destination is DestinationForRoot<Root> {
return destination?.[0] === root;
}Then can do e.g.:
const perpsDestination = hasDestinationRoot(surface.destination, 'perps') ? surface.destination : null;
const onPress = useCallback(() => {
if (perpsDestination) navigateDiscoverDestination(perpsDestination);
}, [perpsDestination]);
return renderSectionLayout({
data: perpsResult.items,
descriptor: perpDescriptor,
loading: perpsResult.isLoading,
onPress: perpsDestination && perpsResult.placement ? onPress : undefined,
placement: perpsResult.placement,
section: surface,
surfaceId,
});(Rather than the various copies of hasDestination that just check if a destination exists. Also note this is based on what seems to be the updated structure in #7557.)
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
2 participants
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.
Discover v2 was built but never mounted; this makes it the live Discover screen behind a remote flag, leaving the legacy home dormant for the cleanup PRs on top.
What changed
discover_enabled(replacingdiscover_placements_enabled), with visible tab routes derived from active-route predicates.What to test
discover_enabledon, the Discover tab shows the v2 surface; with it off, the tab is absent.