feat: add market list filters and client-side column sorting - #12898
feat: add market list filters and client-side column sorting#12898erikzou wants to merge 86 commits into
Conversation
| if (!record.perpsCoin) { | ||
| return renderRedesignTokenIdentity( | ||
| record, | ||
| intl, | ||
| gtXl, | ||
| copyFrom || ECopyFrom.Homepage, | ||
| showStockSubtitle ?? true, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Token creation age reappears in lists that asked for it to be hidden
Severity: non-severe
The token age is always drawn in the redesigned name cell (renderRedesignTokenIdentity at packages/kit/src/views/Market/MarketHomeV2/components/MarketTokenList/hooks/useColumnsDesktop.tsx:464-472) even when the screen asked for it to be hidden, so lists such as the banner detail table now show an age they intentionally suppressed.
Impact: Screens that deliberately omit token age (banner detail) display it again, and the favorites table shows an age under a column still labelled just "Name".
How the hideTokenAge / header-label contract is broken
useColumnsDesktop still receives hideTokenAge and uses it to drop the standalone tokenAge column (packages/kit/src/views/Market/MarketHomeV2/components/MarketTokenList/hooks/useMarketTokenColumns/useColumnsDesktop.tsx:742), and it also uses hideTokenAge / isWatchlistMode / useStockMetadataColumns to decide whether the name header reads "Name/Token Age" or plain "Name" (same file, lines 445-452).
But the cell renderer no longer consults any of those flags: for every non-perps row it calls renderRedesignTokenIdentity(...), which unconditionally computes ageLabel = formatTokenAgeLabel(intl, record.firstTradeTime) and renders it as the first element of the subtitle row. MarketBannerDetail.tsx:220 passes hideTokenAge, and the watchlist passes isWatchlistMode; in both cases the header says "Name" while the rows now render the age, and the banner detail's explicit suppression is defeated whenever the API supplies firstTradeTime.
Threading the existing hideTokenAge/isWatchlistMode decision into renderRedesignTokenIdentity (e.g. a showTokenAge argument gated by the same condition that picks the header label) would keep the cell and the header consistent.
|
Thanks — went through every item. Four are fixed and pushed in
The remaining three need a decision rather than a patch.
|
originalix
left a comment
There was a problem hiding this comment.
Automated code review found blocking issues
Reviewed commit 7259124c63c2.
- P1 · Client-side sort is enabled for spot categories that still paginate from the backend
- P2 · Live row updates do not re-run the active client-side sort
|
Correction to my previous comment — I conceded the pagination point without measuring it, and the measurement does not support it. The backend ignores
Both lists therefore hold the complete set after one request, and So the specific failure described — "the visible order diverges from the real backend pool and shifts again as more pages load" — does not reproduce on either category today. My earlier "the substance is right" was wrong, and the code comment I changed to hedge it was accurate as originally written; I'll restore it. Two things that remain true and are worth keeping on the record:
Given that, I'd propose option 1 (keep as is). Happy to add the guard from option 2 if the backend intends to start paginating. |
originalix
left a comment
There was a problem hiding this comment.
Automated code review found blocking issues
Reviewed commit 3f79d1dbe59c.
- P1 · Live market updates do not re-run the active client-side sort
- P2 · Mobile exposes filters on spot categories that do not consume them
| // Temporarily hide the backend-injected native BTC row in the trending list. | ||
| // Plan of record (PRD P0-1/P1-1): BTC moves to the curated Majors tab once it | ||
| // ships. Flip to false at dev handoff to show the injected row again. | ||
| export const HIDE_INJECTED_BTC_ROW = true; |
There was a problem hiding this comment.
P2: [将 Trending 的 BTC 可见性下沉到服务端]
这里用编译期常量过滤后端注入的 BTC,并为了修正过滤后与 total 的差异额外维护 hasReachedEndAfterFirstPage。这会把榜单内容策略、分页补丁和客户端发版绑定在一起:切换 BTC 是否展示需要重新发版,服务端分页或 total 语义变化时也容易出现重复拉取或提前终止。建议由 Market API 根据榜单配置决定是否返回 BTC,并保证 list/total 一致,前端只消费服务端结果,删除这组硬编码过滤和分页特判。
There was a problem hiding this comment.
Agreed, and it turns out the whole thing was unnecessary — product confirmed BTC does not need hiding at all. Removed in adc8101f01 rather than moved server-side.
Gone: HIDE_INJECTED_BTC_ROW, filterInjectedBtcRow, shouldHideInjectedBtcRowForType, hasReachedEndAfterFirstPage, the two filter call sites in useMarketTokenList, and the now-dead type parameter on transformMarketTokenListResponse. Net -151 lines. None of it existed on x; this restores that behaviour.
Your point about the coupling was the right diagnosis: hasReachedEndAfterFirstPage only existed to patch the list/total mismatch the filtering created, so deleting the filter removed the pagination special case with it. Nothing is left binding list content policy to a client release.
Verified in the browser — BTC is back as the first trending row ($1.26T market cap, -- for the fields the injected row does not carry), and 425 Market tests pass.
| if (draftTimeRange !== initialTimeRange) { | ||
| onApplyTimeRange(draftTimeRange); | ||
| } | ||
| onApply(draft); |
There was a problem hiding this comment.
P2: [Confirm 无改动时会清空当前排序]
先点击 Top Volume 建立 sortState,再打开 Filters 并在不修改条件的情况下直接 Confirm,这里仍会调用 onApply(draft);而 applyConditions 在未传 options.sort 时会把排序重置为 EMPTY_SORT。结果是一次无操作确认也会让排序 chip 消失、列表恢复默认顺序。建议先比较 draft 与 initialConditions,条件未变化时不要调用 onApply;仅修改 time frame 时也应保留现有排序。
There was a problem hiding this comment.
Confirmed and fixed in 49df56ee1f.
Reproduced first: apply the Top Volume chip, open Filters, press Confirm without touching anything — the 成交量 由高到低 chip disappears and the list drops back to default order.
Took your suggestion: Confirm now compares draft against initialConditions and only calls onApply when they differ. Reused the sameConditions helper already in marketListFilterConfig (it was private, backing findActiveMarketFilterChip) rather than adding a second notion of "changed", and exported it with tests covering add / remove / retarget / key-order.
The time-frame case you raised is covered by the same shape: onApplyTimeRange is independent of onApply, so changing only the time frame no longer touches the sort.
Verified after the fix — unchanged Confirm keeps the sort chip and the row order is byte-identical before and after.
| return ( | ||
| <LazyTooltip | ||
| placement="top" | ||
| onPress={onSortPress} |
There was a problem hiding this comment.
P2: [Tooltip 表头会重复触发排序回调]
HeaderColumn 已经把同一个 handleColumnPress 绑定在外层 Column,这里又把它交给内层 LazyTooltip.Trigger。Web 点击 tooltip 表头时事件会继续冒泡到外层,因此同一次点击会执行两次 onSortTypeChange 及 dexSort 埋点。请只保留一个 press owner,例如不要在这里再次转发 onSortPress,让外层 Column 统一处理。
There was a problem hiding this comment.
Correct, and fixed in 49df56ee1f the way you suggested — the forward is gone and the outer Column is the only press owner.
Worth recording why it bubbles, since it is not obvious from the call site: LazyTooltip does not wrap the trigger, it cloneElements it and attaches onPress onto that same element (LazyTooltip.tsx:118-127), and once loaded the real Tooltip renders TMTooltip.Trigger inside the Column too (Tooltip/index.tsx:195-203). Neither stops propagation, so both handlers ran in both states. My original comment there claimed the Tooltip trigger owned the press — that was simply wrong, and the comment now records the real reason.
One correction to the impact, in case it matters for how you triage similar reports: the ordering itself was never wrong. Both invocations run off the same render’s closure, so getNextSortOrder reads the same currentSortOrder and returns the same value twice — the sort lands where the arrow says. What actually broke is exactly what you named: dexSort double-counts.
Verified after removing the forward that tooltip’d headers still sort on a single click (市值 desc → $1.26T … $287.51M) and the hover explainer still appears, so nothing depended on the forwarded handler.
originalix
left a comment
There was a problem hiding this comment.
Automated code review found blocking issues
Reviewed commit 0068eaeaead6.
- P1 · Live token updates bypass the active client-side sort
- P2 · Banner detail now persists sort keys that some layouts cannot honor
originalix
left a comment
There was a problem hiding this comment.
Automated code review found blocking issues
Reviewed commit adc8101f0182.
- P1 · Deselecting a filter tier leaves a ghost active condition behind
- P1 · Live token updates bypass the active client-side sort
- P2 · Banner detail now persists sort keys that the compact layout cannot honor
…filter # Conflicts: # packages/shared/src/locale/enum/translations.ts # packages/shared/src/locale/json/bn.json # packages/shared/src/locale/json/de.json # packages/shared/src/locale/json/en_US.json # packages/shared/src/locale/json/es.json # packages/shared/src/locale/json/fr_FR.json # packages/shared/src/locale/json/hi_IN.json # packages/shared/src/locale/json/id.json # packages/shared/src/locale/json/it_IT.json # packages/shared/src/locale/json/ja_JP.json # packages/shared/src/locale/json/ko_KR.json # packages/shared/src/locale/json/pt.json # packages/shared/src/locale/json/pt_BR.json # packages/shared/src/locale/json/ru.json # packages/shared/src/locale/json/th_TH.json # packages/shared/src/locale/json/uk_UA.json # packages/shared/src/locale/json/vi.json # packages/shared/src/locale/json/zh_CN.json # packages/shared/src/locale/json/zh_HK.json # packages/shared/src/locale/json/zh_TW.json
originalix
left a comment
There was a problem hiding this comment.
Automated code review found blocking issues
Reviewed commit 49df56ee1f93.
- P1 · Live token updates bypass the active client-side sort
- P2 · Re-selecting the current filter tier silently clears the sort
- P2 · Quick-chip matching ignores the chip's time-range anchor
- P2 · Banner detail stores sort keys that the compact layout cannot apply
originalix
left a comment
There was a problem hiding this comment.
Automated code review found blocking issues
Reviewed commit b5e2b14cccc2.
- P1 · Live price updates bypass the active client-side sort
- P2 · Re-selecting the active filter tier clears the sort
- P2 · Preset matching ignores the chip's anchored time range
- P2 · Banner detail exposes sorts that the compact list cannot replay
originalix
left a comment
There was a problem hiding this comment.
Automated code review found blocking issues
Reviewed commit bc30da45a7e7.
- P1 · Live updates bypass the active client-side sort
- P2 · Re-selecting the active filter tier clears the sort
- P2 · Preset matching ignores the chip time range
- P2 · Banner-detail layouts do not share one sort contract
- P3 · Deferred feature branches ship as dead production code
|
Latest round — two fixed in Re-selecting the active filter tier clears the sort — fixedCorrect, and the guard belongs lower than the tier popover. The no-op check now lives in Verified: with Preset matching ignores the chip's anchored time range — fixedConfirmed: Live price updates bypass the active client-side sort — already trackedSame finding Codex raised earlier in this PR; your report notes the cause under Validation gaps ( Banner detail exposes sorts the compact list cannot replay — not changing, reachability belowThe asymmetry is real: the sort lives in the persisted What makes it not worth restructuring is how it is reached. In The same yardstick applies on the other side: the Market tabs avoid this only because Happy to build the compact header out to the full sort set if you would rather close it properly — flagging the cost/benefit rather than declining outright. |
originalix
left a comment
There was a problem hiding this comment.
Automated code review found blocking issues
Reviewed commit 91c3679e0681.
- P1 · Live updates bypass the active client-side sort
- P2 · Filter changes do not reset pagination exhaustion
- P2 · Banner-detail layouts use incompatible sort contracts
| onPress={() => { | ||
| defaultLogger.dex.list.dexFilterChip({ | ||
| action: 'clearAll', | ||
| }); | ||
| applyConditions({}); | ||
| }} |
There was a problem hiding this comment.
The clear button on the market filter bar does nothing when only a sort is active
Severity: non-severe
Pressing clear-all only clears the filter conditions (applyConditions({}) at packages/kit/src/views/Market/MarketHomeV2/components/MarketFilterChipsBar/MarketFilterChipsBar.tsx:713) and leaves the active ordering untouched, so when nothing but an ordering is applied the button appears dead.
Impact: A user who sorted the trending table and then presses the clear button sees nothing change — the chip row stays expanded with the sort still applied.
Why the shared apply path silently no-ops
The chip row is rendered whenever conditionEntries.length > 0 || hasSort (MarketFilterChipsBar.tsx:549), so a header-driven sort alone expands the row and shows the clear-all ×.
applyConditions in packages/kit/src/views/Market/MarketHomeV2/components/MarketFilterChipsBar/MarketListFilterContext.tsx:65-69 deliberately returns early when the incoming conditions equal the current ones, precisely to avoid dropping the sort on a no-op re-apply. With no conditions applied, sameConditions({}, {}) is true, so the early return fires and neither setFilterState nor setSortState runs. Clear-all therefore only works when at least one filter condition exists.
The sort chip's own × calls setSortState({}) directly (MarketFilterChipsBar.tsx:677) and does work, which makes the inconsistency more visible.
Suggested fix:
onPress={() => {
defaultLogger.dex.list.dexFilterChip({
action: 'clearAll',
});
applyConditions({});
setSortState({});
}}
originalix
left a comment
There was a problem hiding this comment.
Automated code review found blocking issues
Reviewed commit eda5ecdb1e00.
- P1 · Live updates bypass the active client-side sort
- P2 · Filter changes do not reset pagination exhaustion
- P2 · Banner-detail layouts use incompatible sort contracts
|
@cursoragent review |
| if (!isWatchlistMode && !clientSort) { | ||
| // The watchlist is ordered by drag, so a header sort would silently | ||
| // overwrite the arrangement the user built by hand. It never sorts. | ||
| if (isWatchlistMode || !clientSort) { |
There was a problem hiding this comment.
🟠 P1 自选列表的表头排序被整体移除
问题
原来的守卫是 if (!isWatchlistMode && !clientSort) return undefined;,watchlist 会继续往下走并命中 columnsMap = SORTABLE_COLUMNS(liquidity / mc / v24hUSD)。本 PR 改成 if (isWatchlistMode || !clientSort) return undefined;,而 MarketWatchlistTokenList 从不传 clientSort,因此收藏页的每一列都不再返回 onSortTypeChange;HeaderColumn 里 enableSortType = !!events?.onSortTypeChange 恒为 false,排序箭头和点击响应一并消失。
注释里给出的理由("watchlist 由拖拽排序,表头排序会覆盖手工顺序")与既有实现不符:拖拽顺序只是 sortBy 为空时的默认序,useMarketWatchlistTokenList 在 !sortBy || !sortType 时按 sortIndex 排,第三次点击表头即回到该默认序,两者本来就是共存的。
影响
桌面「自选」标签上原本可点排序的 24h Vol 列(turnover → v24hUSD → SORT_MAP → turnover)现在变成纯文本,用户无法再按成交额排自选。连带 useMarketWatchlistTokenList 中 sortBy/sortType 的整个 sortedData 分支、tokenListHelpers.SORT_MAP,以及 MarketTokenListBase 里 MarketWatchlistOnlyChanged 为自选清空排序的逻辑全部变成死代码——没有任何入口能再把自选的 sortBy 置为非空。
建议
保留 watchlist 分支,只把 clientSort 分支新增到它旁边;如果确实要保护拖拽顺序,也应只在真正可拖拽时(draggable/isDraggable 为真)禁用,而不是整个 isWatchlistMode:
if (!isWatchlistMode && !clientSort) {
return undefined;
}
if (!clientSort) {
// 自选沿用原 SORTABLE_COLUMNS 分支
}Generated by Claude Code
| // survive seed -> remote refresh and polling updates. | ||
| setTransformedData((prev) => | ||
| reuseStableMarketTokenRows({ prev, next: transformed }), | ||
| reuseStableMarketTokenRows({ prev, next: visibleTokens }), |
There was a problem hiding this comment.
🟠 P1 筛选变化不复位分页,hasReachedEnd 会永久卡死列表
问题
本 PR 新增了 filterParamsKey 作为查询维度(已进入 currentQueryKey 和 swrKeys.marketHomeTokenList),但下面重置分页的 effect 依赖数组仍然是 [networkId, sortBy, sortType, type, category, timeFrame],不含 filterParamsKey:
useEffect(() => {
setCurrentPage(1);
setIsLoadingMore(false);
setHasReachedEnd(false);
}, [networkId, sortBy, sortType, type, category, timeFrame]);同时本 PR 在紧接本行下方删除了 setHasReachedEnd(false);(原本每次新的 apiResult 到达都会兜底复位)。两处叠加后,hasReachedEnd 一旦置 true 就只能靠改网络/时间窗/分类来清除。
影响
hasReachedEnd 只在 loadMore 返回空列表时置 true,而分页只在选中了具体网络时开启(hasNetworkId = Boolean(networkId)),所以场景是:在某条链的 trending 上触底一次之后,用户再改任意筛选条件(新的 filterParamsKey → 新 query → 新 apiResult),canLoadMore 里的 !hasReachedEnd 恒为 false,筛选后的列表永远停在第 1 页 20 条。60s 轮询本身也会把 transformedData 截回第 1 页(reuseStableMarketTokenRows 在长度不同时直接返回 next),过去靠这里的 setHasReachedEnd(false) 才能重新翻页,现在同样恢复不了。
建议
把 filterParamsKey 加入重置 effect 的依赖,并保留新数据到达时的兜底复位:
}, [networkId, sortBy, sortType, type, category, timeFrame, filterParamsKey]);Generated by Claude Code
| // the cell actually prints one. Stocks and the watchlist never carry an | ||
| // age, and banner detail opts out via hideTokenAge. | ||
| const showTokenAge = | ||
| !useStockMetadataColumns && !hideTokenAge && !isWatchlistMode; |
There was a problem hiding this comment.
🟡 P2 未启用 redesign 列表的 tab 会重复渲染代币年龄
问题
showTokenAge 只排除了 stock / watchlist / hideTokenAge,没有和「Name 与 Token Age 合并」这件事的真正开关 redesignColumnOrderEnabled 挂钩。而独立 tokenAge 列的生成条件是 gtXl && !isWatchlistMode && !hideTokenAge,它只会被 REDESIGN_COLUMN_ORDER(不含 tokenAge)过滤掉——也就是只有 trending 会被过滤。
x_mentioned 在 SPOT_CATEGORIES_WITH_FULL_STATS 里,所以 shouldHideSpotExtendedStats 为 false、hiddenDesktopColumns 为 undefined,tokenAge 不会被隐藏;同时它不是 trending,redesignColumnOrderEnabled 为 false,showTokenAge 却仍然为 true。
影响
在 gtXl 宽度下的 x_mentioned spot 标签上,Name 表头显示为 Name / Token Age 且单元格副行渲染 ageLabel,右侧同时还保留着独立的 Token Age 列,同一个数值在一行里出现两次,表头语义也与实际列集合不符。后续服务端再放出任何非 stock 的 full-stats 分类都会复现。
建议
把合并列的显示条件绑定到同一个开关上:
const showTokenAge =
Boolean(redesignColumnOrderEnabled) &&
!useStockMetadataColumns &&
!hideTokenAge &&
!isWatchlistMode;Generated by Claude Code
Summary
Implements PRD client-side column sorting and filter chips + Filters panel for the Market list, and unifies the row/header chrome across the four Market tabs so they stop drifting apart. Targets 6.6.0.
Ships on by default — there is no feature flag.
Filters
A chip bar plus a Filters panel over the trending list.
marketListFilterConfig.tsis the single source of truth for every dimension, so the chips, the panel, the local filter and the request params cannot drift apart.The split between server and client filtering is deliberate:
filterParamspassthrough)marketListFilterPassthrough.test.tslocks five invariants, including every server-side dimension emitting a param, so a silently dropped filter fails CI rather than quietly returning unfiltered rows.Trending is the only list wired to filters, and
isMarketTrendingListis the one predicate that decides it. Anything that shows a filter control gates on the same predicate that consumes it — the mobile toolbar previously used a lookalike condition, which would have put a live-looking Filters button on any future non-stock category that ignores it.Sorting
Header sorting is client-side across the spot, stock and perps tables.
useClientSortResultintercepts thesortBy/sortTypesetters so a header press re-orders the rows already in hand instead of re-keying the SWR query and refetching. Favorites keeps its drag ordering and does not sort.Measured 2026-08-16: the backend ignores
page/limitand answers the first request with the whole pool (trendinglist.length101 oftotal101, stocks 106 of 106), socanLoadMorenever opens and local sorting covers the complete set. This is observed behaviour rather than a documented contract — the client stays defensive (pageSize: 20,maxPages: 5), and if the backend starts honouringlimit, sorting would cover one page only.Token Age sorts through
getTokenAgeSortValuerather than the raw timestamp: the cell renders an age (now - firstTradeTime), which runs opposite to the timestamp, so sorting the timestamp inverted the arrow.Shared list chrome
marketListRedesignVisuals.tsxholds the row height, star column, icon gap and header/sort rendering that the spot and perps tables both use. This closes a set of alignment bugs — perps rows were 60px vs 68px, mobile perps padding 16px vs 20px — that caused a visible jump when switching tabs.The merged Name column gates the age on one
showTokenAgeflag shared by the header label and the cell, so a list that opts out (banner detail'shideTokenAge, the watchlist) cannot end up with a header reading "Name" above rows printing an age.Volume column naming
Turnover→Volume, and each header states its own window. Verified against the wire:stock.assetAnalysis.volume24hmatched all four displayed values exactly, and the server has no turnover field at all, onlyvolume{window}. Trending stays reactive to the selected time frame; Favorites, Banner detail and the detail-page token selector are pinned to24hbecause their time frame is not selectable. Header tooltips were removed from those three for the same reason — they promised a range the user cannot pick.Verification
yarn agent:check --profile commit— all PASSyarn jest packages/kit/src/views/Market— 428 tests / 51 suites pass, 6 test files added or extendedrpcProtocol.ts: serialisation is plainJSON.stringifyand deserialisation validates onlymethod/params-is-array /sync, so no field is strippedNotes for review
Manifest.lock14.2.0 vsPodfile.lock18.21.0) and the build fails inRNPurchases.m. Everything was verified on web including narrow widths, but theMobileLayout.native.tsxpath has not been seen on real hardware.yarn i18n:pullsyncs the whole current Lokalise state, so the locale diff is larger than this feature: of the 70 newen_USkeys only 14 aremarket.*. This matches how locale changes land in other PRs here (fix: show asset symbol in aggregate network selector title (OK-60136) #12868, a one-line title fix, carried 2326 lines across 20 locale files).dexmarket_turnoveris still used inviews/Home/pages/PerpsContainer.tsx:682andviews/UniversalSearch/components/MarketTableHeader.tsx:45. Their data windows were not verified, so the label was not changed.5m/1h/4h/24hrange labels remain untranslated literals inTimeRangeSelector, by request.orderedDatais sorted before the live price overlay is applied and does not re-sort on a tick, so a price-sorted list shows fresh values in the previous poll's order until the next refresh. Handed to the team rather than patched, becauseorderedDataalso feeds the WS subscription range and reordering the two touches that calculation.