refactor: clean frontend routing flow and remove redundant UI/test noise#689
Open
CMI-James wants to merge 5 commits into
Open
refactor: clean frontend routing flow and remove redundant UI/test noise#689CMI-James wants to merge 5 commits into
CMI-James wants to merge 5 commits into
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR consolidates frontend navigation from internal route state to URL-derived routing, refines component visibility conditions, removes redundant state management, adds service robustness, and migrates test infrastructure from Jest to Vitest while updating test assertions. ChangesFrontend cleanup and stabilization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
frontend/tests/components/AnalyticsRangeSwitcher.test.tsx (1)
134-147:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFallback test no longer validates fallback behavior.
On Line 137, passing
ranges={mockRanges}means the test"uses default ranges when none provided"does not cover the "none provided" path anymore. Either remove the prop in this test or rename/split the case.Suggested minimal fix
- it('uses default ranges when none provided', () => { + it('renders provided ranges', () => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/tests/components/AnalyticsRangeSwitcher.test.tsx` around lines 134 - 147, The test "uses default ranges when none provided" is passing ranges={mockRanges} so it doesn't exercise the fallback path; update the test for AnalyticsRangeSwitcher by removing the ranges prop (leave selectedId="24h" and onChange={mockOnChange}) so the component uses its default ranges, or alternatively rename the test to reflect that it uses provided ranges and add a separate test that omits ranges to assert default labels ("24 Hours", "7 Days", "90 Days"); adjust references to mockRanges and mockOnChange accordingly.
🧹 Nitpick comments (2)
frontend/src/App.tsx (1)
53-53: ⚡ Quick winUse ROUTE_PATHS constants for consistency.
Commands "go-lobby" and "go-profile" use hardcoded paths
"/"and"/profile"instead of the centralizedROUTE_PATHSconstants. This creates inconsistency with other commands and could cause maintenance issues if paths change.♻️ Proposed fix to use ROUTE_PATHS constants
{ id: "go-lobby", label: "Go to Lobby", description: "Open the game lobby", - action: () => navigate("/"), + action: () => navigate(ROUTE_PATHS.lobby), },{ id: "go-profile", label: "Go to Profile Settings", description: "Open the profile settings page", - action: () => navigate("/profile"), + action: () => navigate(ROUTE_PATHS.profile), },Also applies to: 65-65
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/App.tsx` at line 53, Replace the hardcoded route strings used in the command actions with the centralized ROUTE_PATHS constants: update the "go-lobby" action (currently calling navigate("/") ) to call navigate(ROUTE_PATHS.LOBBY) and update the "go-profile" action (currently calling navigate("/profile")) to call navigate(ROUTE_PATHS.PROFILE); also ensure ROUTE_PATHS is imported into the App component if it's not already.frontend/src/hooks/v1/useWalletStatus.ts (1)
62-68: ⚡ Quick winConsider adding
lastSucceededAtto the default state for explicitness.The constant omits
lastSucceededAt, which is accessed on line 151. While this works (the property is optional inWalletSessionRefreshState), explicitly including it improves clarity and documents the full shape.♻️ Proposed enhancement for explicitness
const DEFAULT_REFRESH_STATE: WalletSessionRefreshState = { phase: WalletSessionRefreshPhase.IDLE, trigger: "manual", attempt: 0, maxAttempts: 1, terminal: false, + lastSucceededAt: undefined, };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/hooks/v1/useWalletStatus.ts` around lines 62 - 68, DEFAULT_REFRESH_STATE omits the optional lastSucceededAt property which is later accessed; update the DEFAULT_REFRESH_STATE object in useWalletStatus.ts (the WalletSessionRefreshState default) to include lastSucceededAt: null (or an appropriate initial Date | null) so the full shape is explicit and consumers referencing lastSucceededAt won't rely on its absence.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@frontend/tests/components/AnalyticsRangeSwitcher.test.tsx`:
- Around line 134-147: The test "uses default ranges when none provided" is
passing ranges={mockRanges} so it doesn't exercise the fallback path; update the
test for AnalyticsRangeSwitcher by removing the ranges prop (leave
selectedId="24h" and onChange={mockOnChange}) so the component uses its default
ranges, or alternatively rename the test to reflect that it uses provided ranges
and add a separate test that omits ranges to assert default labels ("24 Hours",
"7 Days", "90 Days"); adjust references to mockRanges and mockOnChange
accordingly.
---
Nitpick comments:
In `@frontend/src/App.tsx`:
- Line 53: Replace the hardcoded route strings used in the command actions with
the centralized ROUTE_PATHS constants: update the "go-lobby" action (currently
calling navigate("/") ) to call navigate(ROUTE_PATHS.LOBBY) and update the
"go-profile" action (currently calling navigate("/profile")) to call
navigate(ROUTE_PATHS.PROFILE); also ensure ROUTE_PATHS is imported into the App
component if it's not already.
In `@frontend/src/hooks/v1/useWalletStatus.ts`:
- Around line 62-68: DEFAULT_REFRESH_STATE omits the optional lastSucceededAt
property which is later accessed; update the DEFAULT_REFRESH_STATE object in
useWalletStatus.ts (the WalletSessionRefreshState default) to include
lastSucceededAt: null (or an appropriate initial Date | null) so the full shape
is explicit and consumers referencing lastSucceededAt won't rely on its absence.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: efa10e5b-52fd-4758-8e5d-bf3a751b877e
📒 Files selected for processing (17)
frontend/src/App.tsxfrontend/src/components/CollapsibleSection.tsxfrontend/src/components/HoldingsBreakDowncard.tsxfrontend/src/components/v1/AccountSwitcher.tsxfrontend/src/components/v1/QueueHealthWidget.tsxfrontend/src/hooks/v1/useWalletStatus.tsfrontend/src/pages/AnalyticsDashboard.tsxfrontend/src/pages/Portfolio.tsxfrontend/src/utils/v1/index.tsfrontend/tests/components/AnalyticsRangeSwitcher.test.tsxfrontend/tests/components/AuditSnapshotCard.test.tsxfrontend/tests/components/Portfolio.test.tsxfrontend/tests/components/QueueHealthWidget.test.tsxfrontend/tests/components/QuickPivotLinks.test.tsxfrontend/tests/components/v1/CampaignRewardsSpotlightCard.test.tsxfrontend/tests/components/v1/CommandPalette.test.tsxfrontend/tests/unit/AccountSwitcher.test.tsx
💤 Files with no reviewable changes (2)
- frontend/src/utils/v1/index.ts
- frontend/tests/unit/AccountSwitcher.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.
Description
Implemented a full frontend cleanup pass to remove routing/state noise, reduce redundant UI behavior, and stabilize brittle tests while preserving user-facing behavior.
Closes #688
Changes proposed
What were you told to do?
Perform a full cleanup of the website codebase by pulling latest changes, removing redundant/noisy frontend patterns, and restoring stable behavior.
What did I do?
Routing and app-shell cleanup
UI and component cleanup
Test and quality stabilization
Check List (Check all the applicable boxes)
Screenshots / Testing Evidence
npm run --prefix frontend typecheckpassed.npm run --prefix frontend lintpassed.npm run --prefix frontend testpassed (107 files, 1722 tests).npm run --prefix frontend buildpassed.Summary by CodeRabbit
Bug Fixes
Improvements
Tests
Chores