[codex] refine token routes workspace motion and hierarchy#394
Open
[codex] refine token routes workspace motion and hierarchy#394
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughSplits route rendering into paired summary and detail panels with a desktop presence/closing delay, replaces animated-collapse hooks with custom presence state, introduces per-route memoized missing-token caches, updates drag/sort strategy and sortable channel wiring, large CSS restyling (dark-mode/responsive), and adds/updates tests for accessibility, sorting, and panel presence. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Page as TokenRoutes
participant Card as RouteCard (summary)
participant Panel as DesktopDetailPanelPresence
participant Timer as Close Timer
User->>Card: click summary (open)
Card->>Page: notify toggleExpand(routeId)
Page->>Panel: mount detailPanel for routeId
Panel->>Card: mark summary as active
User->>Panel: click "收起详情" (close)
Panel->>Page: notify toggleExpand(routeId)
Page->>Timer: start DESKTOP_DETAIL_COLLAPSE_MS (180ms)
Timer-->>Panel: delay expiration
Panel->>Card: keep summary marked active while closing
Timer->>Page: unmount detailPanel, clear closing flag
Page->>Card: remove active state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
bot
added
area: web
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/web/pages/TokenRoutes.tsx (1)
1814-1886:⚠️ Potential issue | 🟠 MajorThe summary/detail split still eagerly computes detail-only props.
This loop still calls
getRouteCandidateView()plus both missing-token builders for every visible route on every parent render. That keeps unrelated channel draft/update state on anO(visibleRoutes)hot path, and the fresh missing-token arrays also invalidateRouteCard's expanded-props equality check even when the underlying hints did not change.⚡ Minimal fix for the collapsed summary path
const summaryCard = ( <RouteCard route={route} brand={routeBrandById.get(route.id) || null} expanded={false} summaryExpanded={isExpanded} @@ - candidateView={getRouteCandidateView(route.id)} + candidateView={EMPTY_ROUTE_CANDIDATE_VIEW} @@ - missingTokenSiteItems={getMissingTokenSiteItems(route.id)} - missingTokenGroupItems={getMissingTokenGroupItems(route.id)} + missingTokenSiteItems={EMPTY_MISSING_ITEMS} + missingTokenGroupItems={EMPTY_MISSING_GROUP_ITEMS} onCreateTokenForMissing={stableCreateTokenForMissing} onAddChannel={stableAddChannel} onSiteBlockModel={stableSiteBlockModel} expandedSourceGroupMap={expandedSourceGroupMap} onToggleSourceGroup={stableToggleSourceGroup}If you want the expanded path to stay memo-friendly too, cache
getMissingTokenSiteItems()/getMissingTokenGroupItems()per route the same way asgetRouteCandidateView().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/web/pages/TokenRoutes.tsx` around lines 1814 - 1886, The summary/detail split is eagerly computing detail-only values (getRouteCandidateView, getMissingTokenSiteItems, getMissingTokenGroupItems) for every route; change the code so these calls are only invoked when detailPanel is rendered (i.e., inside the isExpanded branch) and pass lightweight defaults to the summary RouteCard (keep candidateView, missingTokenSiteItems and missingTokenGroupItems undefined/null for the collapsed summary); for a stronger fix also memoize/cache per-route results of getRouteCandidateView, getMissingTokenSiteItems and getMissingTokenGroupItems (keyed by route.id) so RouteCard’s expanded prop equality isn’t invalidated unnecessarily (refer to RouteCard props candidateView, missingTokenSiteItems, missingTokenGroupItems and the helper getRouteCandidateView).
🧹 Nitpick comments (1)
src/web/pages/tokenRoutes.desktop-detail-panel.test.tsx (1)
2-2: ImportReactTestRendererexplicitly in this new test.
WebTestRenderermakes this file depend on an ambient alias again. Pull the renderer type fromreact-test-rendererhere so the test stays self-contained.♻️ Suggested change
-import { act, create, type ReactTestInstance } from 'react-test-renderer'; +import { act, create, type ReactTestInstance, type ReactTestRenderer } from 'react-test-renderer'; @@ - let root!: WebTestRenderer; + let root!: ReactTestRenderer;Based on learnings, Prefer explicit imports of
ReactTestRendererfromreact-test-rendererin new test files (e.g.,*.test.tsx) instead of relying on any globally-declaredWebTestRenderer/WebTestRenderertype aliases. Keeping the dependency explicit makes tests self-contained and avoids ambiguity when checking/maintaining TypeScript types.Also applies to: 142-142
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/web/pages/tokenRoutes.desktop-detail-panel.test.tsx` at line 2, Add an explicit import for the ReactTestRenderer type from 'react-test-renderer' in this test file so it doesn't rely on any ambient WebTestRenderer alias: update the import statement that currently pulls in act, create, and ReactTestInstance to also import the type ReactTestRenderer and use that type where needed (references: ReactTestRenderer, ReactTestInstance, act, create) to keep the test self-contained.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/web/index.css`:
- Around line 4829-4844: The .route-batch-bar is using top: 0 so it will stick
under the existing fixed topbar; change the rule for .route-batch-bar to use the
top offset variable (replace top: 0 with top: var(--topbar-height)) so the
sticky batch bar sits below the topbar (use the --topbar-height CSS custom
property as the offset and keep existing z-index/display styles).
In `@src/web/pages/token-routes/RouteCard.tsx`:
- Around line 273-277: The summary surface (the div with class "card
route-card-collapsed" that calls onToggleExpand(route.id)) is not keyboard
focusable or operable; make it accessible by either converting it to a semantic
<button> or adding tabIndex={0}, role="button", aria-expanded={summaryExpanded}
and an onKeyDown handler that triggers onToggleExpand(route.id) when Enter or
Space is pressed; ensure the existing onClick handler remains and that
aria-expanded reflects the summaryExpanded state so screen readers announce it.
---
Outside diff comments:
In `@src/web/pages/TokenRoutes.tsx`:
- Around line 1814-1886: The summary/detail split is eagerly computing
detail-only values (getRouteCandidateView, getMissingTokenSiteItems,
getMissingTokenGroupItems) for every route; change the code so these calls are
only invoked when detailPanel is rendered (i.e., inside the isExpanded branch)
and pass lightweight defaults to the summary RouteCard (keep candidateView,
missingTokenSiteItems and missingTokenGroupItems undefined/null for the
collapsed summary); for a stronger fix also memoize/cache per-route results of
getRouteCandidateView, getMissingTokenSiteItems and getMissingTokenGroupItems
(keyed by route.id) so RouteCard’s expanded prop equality isn’t invalidated
unnecessarily (refer to RouteCard props candidateView, missingTokenSiteItems,
missingTokenGroupItems and the helper getRouteCandidateView).
---
Nitpick comments:
In `@src/web/pages/tokenRoutes.desktop-detail-panel.test.tsx`:
- Line 2: Add an explicit import for the ReactTestRenderer type from
'react-test-renderer' in this test file so it doesn't rely on any ambient
WebTestRenderer alias: update the import statement that currently pulls in act,
create, and ReactTestInstance to also import the type ReactTestRenderer and use
that type where needed (references: ReactTestRenderer, ReactTestInstance, act,
create) to keep the test self-contained.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6d26086e-7f0d-4e09-80ed-5e31e169a345
📒 Files selected for processing (6)
src/web/index.csssrc/web/pages/TokenRoutes.tsxsrc/web/pages/token-routes/RouteCard.test.tsxsrc/web/pages/token-routes/RouteCard.tsxsrc/web/pages/token-routes/RouteFilterBar.tsxsrc/web/pages/tokenRoutes.desktop-detail-panel.test.tsx
Owner
Author
|
Addressed the current review feedback in
Verification rerun after the fixes:
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/web/pages/token-routes/RouteCard.tsx (1)
833-835: Consider documenting the early-return optimization.The
if (!next.expanded) return trueshort-circuit is the core memoization optimization that prevents collapsed cards from re-rendering on expanded-only state changes. A brief inline comment would help future maintainers understand this intentional behavior.📝 Suggested documentation
+ // Collapsed cards don't render channel details, so skip re-renders + // triggered by expanded-only state (channels, drafts, decision, etc.) if (!next.expanded) { return true; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/web/pages/token-routes/RouteCard.tsx` around lines 833 - 835, Add a brief inline comment above the early-return "if (!next.expanded) { return true; }" in RouteCard.tsx explaining that this short-circuit is an intentional memoization optimization to prevent collapsed cards from re-rendering when only expanded-state changes; reference the condition (next.expanded) and the intent (skip updates for collapsed cards) so future maintainers understand why the guard exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/web/pages/token-routes/RouteCard.tsx`:
- Around line 833-835: Add a brief inline comment above the early-return "if
(!next.expanded) { return true; }" in RouteCard.tsx explaining that this
short-circuit is an intentional memoization optimization to prevent collapsed
cards from re-rendering when only expanded-state changes; reference the
condition (next.expanded) and the intent (skip updates for collapsed cards) so
future maintainers understand why the guard exists.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f9be428a-aa8b-48b1-b7ac-f1aee0e21d25
📒 Files selected for processing (5)
src/web/index.csssrc/web/pages/TokenRoutes.tsxsrc/web/pages/token-routes/RouteCard.test.tsxsrc/web/pages/token-routes/RouteCard.tsxsrc/web/pages/tokenRoutes.desktop-detail-panel.test.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/web/pages/token-routes/RouteCard.test.tsx
- src/web/pages/TokenRoutes.tsx
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/web/pages/token-routes/SortableChannelRow.tsx (1)
180-185:⚠️ Potential issue | 🟠 MajorUse a theme token for the “当前生效” badge background.
color-mix(in srgb, var(--color-info) 14%, white)resolves to an almost-white pill in both themes. In dark mode that pairs the lighter--color-infotext with a very light background, which hurts contrast and breaks the new dark-surface hierarchy. You already have--color-info-softfor this.Suggested fix
- background: 'color-mix(in srgb, var(--color-info) 14%, white)', + background: 'var(--color-info-soft)',Also applies to: 387-392
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/web/pages/token-routes/SortableChannelRow.tsx` around lines 180 - 185, The badge for the "当前生效" pill uses a hard-coded color-mix background that produces poor contrast in dark mode; update the span with className "badge" in the SortableChannelRow component to use the theme token var(--color-info-soft) instead of color-mix(...), and make the same replacement for the second occurrence around the other badge (the block referenced at the other occurrence). Ensure the style object replaces background: 'color-mix(in srgb, var(--color-info) 14%, white)' with background: 'var(--color-info-soft)' so the badge respects the dark-surface hierarchy.
🧹 Nitpick comments (2)
src/web/pages/token-routes/SortableChannelRow.layering.test.tsx (1)
69-77: Make this regression test independent of styling tokens.Matching the row by
display === 'grid'andborderRadius === 16, then assertingborderLeftis absent, couples a layering test to presentation details rather than thezIndexcontract. A stable hook on the row root would keep this from breaking on the next visual refresh.Stable selector example
--- a/src/web/pages/token-routes/SortableChannelRow.tsx +++ b/src/web/pages/token-routes/SortableChannelRow.tsx @@ - <div ref={setNodeRef} style={rowStyle}> + <div ref={setNodeRef} style={rowStyle} data-testid={`sortable-channel-row-${channel.id}`}>--- a/src/web/pages/token-routes/SortableChannelRow.layering.test.tsx +++ b/src/web/pages/token-routes/SortableChannelRow.layering.test.tsx @@ - const row = root.root.find((node) => ( - node.type === 'div' - && node.props.style - && node.props.style.display === 'grid' - && node.props.style.borderRadius === 16 - )); + const row = root.root.findByProps({ 'data-testid': `sortable-channel-row-${channel.id}` }); expect(row.props.style.zIndex).toBeUndefined(); - expect(row.props.style.borderLeft).toBeUndefined();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/web/pages/token-routes/SortableChannelRow.layering.test.tsx` around lines 69 - 77, The test is brittle because it locates the row element by styling tokens (root.root.find with display === 'grid' && borderRadius === 16) and then asserts layering props; instead, change the component under test (SortableChannelRow) to expose a stable hook (e.g., a data attribute like data-testid="sortable-channel-row" or data-layer-root) and update the test to find the node by that stable attribute (replace the root.root.find predicate) and then assert that row.props.style.zIndex and row.props.style.borderLeft are undefined; reference SortableChannelRow and the test's row lookup to implement this change so future visual style changes won't break the layering contract.src/web/pages/token-routes/SortableChannelRow.tsx (1)
88-91: Scopewill-changeto active transforms only.This keeps every idle row in the list promoted all the time. On larger route buckets that can add unnecessary compositing/memory overhead and eat into the smoothness win this PR is aiming for. Gate it on an active
transforminstead.Example tweak
- willChange: 'transform', + willChange: transform ? 'transform' : undefined,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/web/pages/token-routes/SortableChannelRow.tsx` around lines 88 - 91, The inline style in SortableChannelRow.tsx currently sets willChange unconditionally which promotes every row; change the style so willChange is only set when the row is actively being transformed (use the existing isDragging flag), e.g. set willChange to 'transform' when isDragging is true and to undefined/'auto' when false; update the style object where boxShadow and willChange are defined (the block referencing isDragging) so only active drags request the GPU compositing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/web/pages/token-routes/SortableChannelRow.tsx`:
- Around line 180-185: The badge for the "当前生效" pill uses a hard-coded color-mix
background that produces poor contrast in dark mode; update the span with
className "badge" in the SortableChannelRow component to use the theme token
var(--color-info-soft) instead of color-mix(...), and make the same replacement
for the second occurrence around the other badge (the block referenced at the
other occurrence). Ensure the style object replaces background: 'color-mix(in
srgb, var(--color-info) 14%, white)' with background: 'var(--color-info-soft)'
so the badge respects the dark-surface hierarchy.
---
Nitpick comments:
In `@src/web/pages/token-routes/SortableChannelRow.layering.test.tsx`:
- Around line 69-77: The test is brittle because it locates the row element by
styling tokens (root.root.find with display === 'grid' && borderRadius === 16)
and then asserts layering props; instead, change the component under test
(SortableChannelRow) to expose a stable hook (e.g., a data attribute like
data-testid="sortable-channel-row" or data-layer-root) and update the test to
find the node by that stable attribute (replace the root.root.find predicate)
and then assert that row.props.style.zIndex and row.props.style.borderLeft are
undefined; reference SortableChannelRow and the test's row lookup to implement
this change so future visual style changes won't break the layering contract.
In `@src/web/pages/token-routes/SortableChannelRow.tsx`:
- Around line 88-91: The inline style in SortableChannelRow.tsx currently sets
willChange unconditionally which promotes every row; change the style so
willChange is only set when the row is actively being transformed (use the
existing isDragging flag), e.g. set willChange to 'transform' when isDragging is
true and to undefined/'auto' when false; update the style object where boxShadow
and willChange are defined (the block referencing isDragging) so only active
drags request the GPU compositing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a34345a1-2357-4d09-b5f9-9041c4c94db2
📒 Files selected for processing (3)
src/web/index.csssrc/web/pages/token-routes/SortableChannelRow.layering.test.tsxsrc/web/pages/token-routes/SortableChannelRow.tsx
github-actions
bot
added
size: XXL
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/web/pages/TokenRoutes.tsx (1)
1841-1959:⚠️ Potential issue | 🟡 MinorGrid layout will leave gaps and displace later summary cards without dense packing.
The detail panel's
gridColumn: '1 / -1'span (confirmed at line 145 of TokenRoutes.tsx and line 5849 of index.css) combined with the grid'srepeat(auto-fill, minmax(400px, 1fr))auto-placement will push subsequent route cards below the full-width detail panels. Withoutgrid-auto-flow: dense, the grid respects source order and does not backfill empty cells. Addgrid-auto-flow: denseto.route-card-gridor switch toauto-fitwith careful overflow handling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/web/pages/TokenRoutes.tsx` around lines 1841 - 1959, The grid placement is leaving gaps because detail panels span full width and the container uses auto-placement without backfilling; update the CSS for the grid container (selector .route-card-grid) to include grid-auto-flow: dense so summaryCard/RouteCard items can backfill gaps created by the detailPanel full-width span, or alternatively change the grid from repeat(auto-fill, minmax(400px, 1fr)) to repeat(auto-fit, minmax(400px, 1fr)) with proper overflow handling; apply this change alongside the TokenRoutes rendering of summaryCard and detailPanel (DesktopDetailPanelPresence) to ensure subsequent route cards aren’t pushed below full-width panels.
🧹 Nitpick comments (2)
src/web/pages/token-routes/RouteCard.tsx (2)
238-243: ScopewillChangeto active transforms only.Leaving
willChange: 'transform'on every channel shell all the time is a poor fit for long route lists; it can keep too many rows eligible for compositor promotion even when they are idle. Only opt in while a row is actually dragging or animating.♻️ Minimal change
- willChange: 'transform', + willChange: isDragging || translatedTransform ? 'transform' : undefined,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/web/pages/token-routes/RouteCard.tsx` around lines 238 - 243, The component currently sets willChange: 'transform' unconditionally which keeps rows on the compositor; change it to only opt-in when the row is actively being dragged or animating by making willChange conditional (e.g. set willChange to 'transform' only when isDragging is true or shellTransition exists) in the style object where translatedTransform, shellTransition and isDragging are used (inside RouteCard's inline style block) so idle rows don't remain eligible for compositor promotion.
1022-1051: Move the collapsed fast-path ahead of expanded-only callback checks.Collapsed cards do not read
onEdit,onSaveToken,onDeleteChannel,onRoutingStrategyChange, and the other expanded-only handlers, but those identities are still compared before the!next.expandedreturn. Any upstream callback churn will still invalidate memoization for the summary path and make the rerender win here fragile.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/web/pages/token-routes/RouteCard.tsx` around lines 1022 - 1051, areRouteCardPropsEqual currently compares many expanded-only callback props before checking if the card is collapsed; move the collapsed fast-path (the if (!next.expanded) return true) to the top of the function (right after the simple scalar comparisons like route/brand/expanded/compact/summaryExpanded/detailPanel) so collapsed cards bail out without comparing expanded-only handlers. Specifically, ensure that handlers such as onEdit, onSaveToken, onDeleteChannel, onRoutingStrategyChange, onCreateTokenForMissing, onAddChannel, onDelete, onToggleEnabled, onClearCooldown, onToggleChannelEnabled, onChannelDragEnd, onTokenDraftChange, onToggleExpand, onToggleSourceGroup, and onSiteBlockModel are not compared when next.expanded is false by placing the !next.expanded short-circuit before those comparisons in areRouteCardPropsEqual.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/web/pages/token-routes/RouteCard.tsx`:
- Around line 769-774: The compact-mode button rendering only checks
!exactRoute, which hides the group editor for explicitGroupRoute routes; update
the condition that renders the edit button (the block that calls onEdit(route)
and uses renderClearCooldownButton()) to allow explicitGroupRoute as well by
changing the guard to (explicitGroupRoute || !exactRoute) so explicit-group
routes keep the edit entry point in compact/mobile layouts; ensure
explicitGroupRoute is in scope where this JSX renders.
In `@src/web/pages/TokenRoutes.tsx`:
- Around line 108-139: The effect runs the "close" timeout on initial mount when
open is false; add a "hasEverOpened" flag (useRef or state) and set it true
inside the branch where open is true (e.g. in DesktopDetailPanelPresence when
open becomes true set hasEverOpened.current = true), then in the effect's false
branch only run the setIsOpen(false)/setIsClosing(true)/setTimeout(...) cleanup
sequence if hasEverOpened is true; if hasEverOpened is false, skip scheduling
the timeout and just ensure shouldRender stays false and isOpen is false to
avoid the initial unnecessary timer and state churn (keep existing RAF and timer
cleanup logic intact and refer to DESKTOP_DETAIL_COLLAPSE_MS).
---
Outside diff comments:
In `@src/web/pages/TokenRoutes.tsx`:
- Around line 1841-1959: The grid placement is leaving gaps because detail
panels span full width and the container uses auto-placement without
backfilling; update the CSS for the grid container (selector .route-card-grid)
to include grid-auto-flow: dense so summaryCard/RouteCard items can backfill
gaps created by the detailPanel full-width span, or alternatively change the
grid from repeat(auto-fill, minmax(400px, 1fr)) to repeat(auto-fit,
minmax(400px, 1fr)) with proper overflow handling; apply this change alongside
the TokenRoutes rendering of summaryCard and detailPanel
(DesktopDetailPanelPresence) to ensure subsequent route cards aren’t pushed
below full-width panels.
---
Nitpick comments:
In `@src/web/pages/token-routes/RouteCard.tsx`:
- Around line 238-243: The component currently sets willChange: 'transform'
unconditionally which keeps rows on the compositor; change it to only opt-in
when the row is actively being dragged or animating by making willChange
conditional (e.g. set willChange to 'transform' only when isDragging is true or
shellTransition exists) in the style object where translatedTransform,
shellTransition and isDragging are used (inside RouteCard's inline style block)
so idle rows don't remain eligible for compositor promotion.
- Around line 1022-1051: areRouteCardPropsEqual currently compares many
expanded-only callback props before checking if the card is collapsed; move the
collapsed fast-path (the if (!next.expanded) return true) to the top of the
function (right after the simple scalar comparisons like
route/brand/expanded/compact/summaryExpanded/detailPanel) so collapsed cards
bail out without comparing expanded-only handlers. Specifically, ensure that
handlers such as onEdit, onSaveToken, onDeleteChannel, onRoutingStrategyChange,
onCreateTokenForMissing, onAddChannel, onDelete, onToggleEnabled,
onClearCooldown, onToggleChannelEnabled, onChannelDragEnd, onTokenDraftChange,
onToggleExpand, onToggleSourceGroup, and onSiteBlockModel are not compared when
next.expanded is false by placing the !next.expanded short-circuit before those
comparisons in areRouteCardPropsEqual.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 622da77f-18f8-49ae-b292-f8f2cee2fada
📒 Files selected for processing (13)
src/web/index.csssrc/web/pages/TokenRoutes.tsxsrc/web/pages/token-routes/RouteCard.sortable-shell.test.tsxsrc/web/pages/token-routes/RouteCard.test.tsxsrc/web/pages/token-routes/RouteCard.tsxsrc/web/pages/token-routes/RouteFilterBar.test.tsxsrc/web/pages/token-routes/RouteFilterBar.tsxsrc/web/pages/token-routes/SortableChannelRow.layering.test.tsxsrc/web/pages/token-routes/SortableChannelRow.tsxsrc/web/pages/token-routes/sortingStrategies.test.tssrc/web/pages/token-routes/sortingStrategies.tssrc/web/pages/token-routes/types.tssrc/web/pages/tokenRoutes.desktop-detail-panel.test.tsx
✅ Files skipped from review due to trivial changes (2)
- src/web/pages/tokenRoutes.desktop-detail-panel.test.tsx
- src/web/pages/token-routes/RouteFilterBar.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/web/pages/token-routes/SortableChannelRow.layering.test.tsx
- src/web/pages/token-routes/RouteCard.test.tsx
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
1 participant
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.
Summary
This PR refines the
/routesworkspace so the page feels more modern and materially smoother during everyday route operations.From a user perspective, the biggest problems were desktop route cards jumping between grid and full-width layouts, summary cards re-rendering during unrelated channel edits, and a general lack of hierarchy in the toolbar/filter/card surfaces. Those issues showed up as stutter, flicker, shape changes, and a page that felt heavier than the rest of the product.
Root Cause
The main animation and smoothness issues came from three interacting patterns:
RouteCardwas memoized too loosely for this page shape, so collapsed cards still re-rendered when expanded-only channel state changed elsewhere.Fix
This change addresses both the structural smoothness problems and the visual polish together:
RouteCardmemo behavior so collapsed cards skip rerenders triggered by expanded-only channel draft/update statealltoward property-specific transitionsValidation
I verified the change with:
npm test -- src/web/pages/token-routes/RouteCard.test.tsx src/web/pages/tokenRoutes.desktop-detail-panel.test.tsx src/web/pages/tokenRoutes.group-collapse.test.tsx src/web/pages/tokenRoutes.mobile-layout.test.tsx src/web/pages/tokenRoutes.mobile.test.tsx src/web/pages/tokenRoutes.routing-strategy.test.tsxnpm run typecheck:webnpm run typecheck:web:testnpm run repo:drift-checkAll of the above passed in the isolated latest-main worktree used for this PR.
Summary by CodeRabbit
New Features
Style
Tests