Skip to content

Conversation

@fairlighteth
Copy link
Contributor

@fairlighteth fairlighteth commented Nov 21, 2025

Summary

Fixes #6156

  • Removed Limit and Advanced Orders from the lazy/Suspense route list; they now render directly, so any downstream Suspense doesn’t bubble to the route-level and trigger a full-page loader.
  • Wrapped only OrdersTableWidget in Limit and TWAP/Advanced Orders with a local <Suspense> and the existing loader. Table data fetches can suspend without unmounting the surrounding page.
Screen.Recording.2025-11-24.at.16.20.27.mov

Technical context

Previously, when token lists toggled, OrdersTableWidget could suspend while reloading balances/allowances. Because the page was under a route-level <Suspense>, that suspension replaced the entire page with a global loader, causing the visible blink. Moving these routes out of the lazy Suspense and localizing the fallback to the table confines the loader to the table area.

To Test

  1. Limit Orders
  • Open Limit → Manage Lists; toggle lists on/off.
  • Expect only the orders table to show a loader; the rest of the page should stay rendered (no blink).
  1. TWAP / Advanced Orders
  • Open Advanced Orders → Manage Lists; toggle lists on/off.
  • Expect the same localized loader on the table while the page remains mounted.

Summary by CodeRabbit

  • Improvements
    • Optimized route handling for limit and advanced orders functionality.
    • Added loading indicators while orders tables retrieve and display data.

✏️ Tip: You can customize this high-level summary in your review settings.

@vercel
Copy link

vercel bot commented Nov 21, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Updated (UTC)
cowfi Ready Ready Preview Dec 3, 2025 9:42am
explorer-dev Ready Ready Preview Dec 3, 2025 9:42am
swap-dev Ready Ready Preview Dec 3, 2025 9:42am
widget-configurator Ready Ready Preview Dec 3, 2025 9:42am
2 Skipped Deployments
Project Deployment Preview Updated (UTC)
cosmos Ignored Ignored Dec 3, 2025 9:42am
sdk-tools Ignored Ignored Preview Dec 3, 2025 9:42am

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 21, 2025

Walkthrough

Routes for Limit Orders and Advanced Orders are converted from lazy-loaded to eagerly-loaded components, and both Advanced Orders and Limit Orders pages now wrap OrdersTableWidget in React.Suspense boundaries with Loading fallbacks to control component initialization.

Changes

Cohort / File(s) Summary
Route Configuration
apps/cowswap-frontend/src/modules/application/containers/App/RoutesApp.tsx
Removed LIMIT_ORDER and ADVANCED_ORDERS from lazy routes array; added explicit Route entries to render these components eagerly instead of lazy-loaded.
Page Components with Suspense
apps/cowswap-frontend/src/pages/AdvancedOrders/index.tsx, apps/cowswap-frontend/src/pages/LimitOrders/RegularLimitOrders.tsx
Both pages now wrap OrdersTableWidget in React.Suspense boundaries with Loading fallback components to defer rendering until data is ready.

Sequence Diagram

sequenceDiagram
    participant Router as Router
    participant LazyRoute as Lazy Route<br/>(Before)
    participant EagerRoute as Eager Route<br/>(After)
    participant Page as Page Component
    participant Suspense as Suspense Boundary
    participant Table as OrdersTableWidget

    alt Before: Lazy Loading
        Router->>LazyRoute: Navigation
        LazyRoute-->>Router: Load Component (async)
        LazyRoute->>Page: Mount page
        Page->>Table: Render immediately
        Note over Table: May cause visual jank
    end

    alt After: Eager Loading + Suspense
        Router->>EagerRoute: Navigation
        EagerRoute->>Page: Mount page (already loaded)
        Page->>Suspense: Render with Suspense
        Suspense->>Suspense: Show Loading fallback
        Suspense->>Table: Render when ready
        Note over Suspense: Controlled loading state
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Route configuration changes: Verify eager loading doesn't impact initial bundle size or performance negatively; ensure both routes render correctly without lazy-loading wrapper.
  • Suspense integration: Confirm Loading component displays appropriately and timing doesn't introduce unexpected delays; validate that the pattern is correctly applied in both Advanced Orders and Limit Orders pages.
  • Issue validation: Cross-check that these changes actually resolve the "blinking" behavior reported in linked issue #6156 by preventing unnecessary component unmount/remount cycles.

Possibly related PRs

  • Fix/5379 page name not updating #5579: Modifies the same AdvancedOrders page component—one adds a PageTitle while this PR wraps the orders table in Suspense, so coordination between these changes may be relevant.

Suggested reviewers

  • elena-zh
  • alfetopito
  • limitofzero

Poem

🐰 Routes now eager, no lazy delays,
Suspense wraps tables in Loading's embrace,
No more blinks when tokens shift place,
Components load smooth, at their own pace,
A steadier app brings a smile to your face!

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ⚠️ Warning The PR title 'fix: token list management bug' is misleading. The actual changes focus on refactoring route structure and adding Suspense boundaries to prevent full-page suspension during token list toggling, not fixing a general token list management bug. Update the title to accurately reflect the main change, such as 'refactor: prevent full-page suspension when toggling token lists' or 'fix: confine suspension to orders table to prevent page blink'.
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The PR description is comprehensive, including the issue fix reference, clear explanation of changes, technical context, and detailed testing steps aligned with the template.
Linked Issues check ✅ Passed The PR changes directly address issue #6156 by removing route-level Suspense and adding localized Suspense boundaries, preventing full-page blinking when toggling token lists.
Out of Scope Changes check ✅ Passed All code changes are scoped to fixing the Suspense behavior for Limit Orders and Advanced Orders routes; no unrelated modifications were introduced.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/token-list-toggle

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@fairlighteth fairlighteth requested a review from a team November 24, 2025 16:21
@fairlighteth fairlighteth changed the title refactor: improve token list management and enhance auto-import funct… refactor: improve token list management Nov 24, 2025
@shoom3301
Copy link
Collaborator

@fairlighteth just a small note about the commit message. This is a fix, not refactor, since it changes the app behaviour (and actually fixes a bug)

Copy link
Contributor

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for fixing this issue :)

@fairlighteth fairlighteth merged commit 36f30f6 into develop Dec 3, 2025
15 checks passed
@fairlighteth fairlighteth deleted the fix/token-list-toggle branch December 3, 2025 10:15
@github-actions github-actions bot locked and limited conversation to collaborators Dec 3, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The app blinks when switch OFF a token list

4 participants