feat(dashboard): new proxies page and nav restructure#1362
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughRouting now renders ChangesProxy & Provider UI + Querying
Rules — Rule Providers Section
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Browser/UI
participant QueryClient
participant API as Backend API
User->>UI: Click "Test Proxy"
activate UI
UI->>UI: set testingProxies state, disable button
UI->>API: getProxyDelay(proxyName, url, timeout)
activate API
API-->>UI: { delay: number }
deactivate API
UI->>UI: update latencyMap, clear testing state
UI->>QueryClient: (no query invalidation on success)
deactivate UI
sequenceDiagram
participant User
participant UI as Browser/UI
participant QueryClient
participant API as Backend API
User->>UI: Click "Test All" for provider
activate UI
UI->>UI: set testingProviders state, disable button
loop per provider proxy (batched)
UI->>API: getProviderProxyDelay(providerName, proxyName, url, timeout)
API-->>UI: { delay: number }
UI->>UI: update latencyMap
end
UI->>QueryClient: invalidateQueries(['providers'])
QueryClient->>API: fetch providers
API-->>QueryClient: providers data
UI->>UI: clear testingProviders, re-render
deactivate UI
sequenceDiagram
participant User
participant UI as Browser/UI
participant QueryClient
participant API as Backend API
User->>UI: Click "Update" on rule provider
activate UI
UI->>UI: set updatingRuleProviders state
UI->>API: updateRuleProvider(providerName)
API-->>UI: Success
UI->>QueryClient: invalidateQueries(['rule-providers'])
QueryClient->>API: fetch rule-providers
API-->>QueryClient: updated providers
UI->>UI: clear updating state, re-render
deactivate UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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. Review rate limit: 4/8 reviews remaining, refill in 27 minutes and 24 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clash-dashboard/src/App.tsx`:
- Line 29: Add a compatibility alias route so requests to "/providers" still
resolve to the same component: keep the existing Route with path="proxies" and
add a second Route with path="providers" that also renders <ProxyList /> (i.e.,
add a Route with path="providers" element={<ProxyList />} alongside the Route
path="proxies"). This preserves bookmarks and shared links while rolling out the
new URL.
In `@clash-dashboard/src/pages/ProxyList.tsx`:
- Around line 231-238: The testAllInProvider function double-runs provider
checks: it individually calls testProviderProxy for each proxy and then
immediately calls healthcheckProvider which repeats probes server-side; remove
the redundant call to healthcheckProvider() (or, alternatively, skip per-proxy
testProviderProxy calls and only call healthcheckProvider()) and keep the
existing queryClient.invalidateQueries({ queryKey: ['providers'] }) to refresh
histories—update testAllInProvider to pick one path (recommended: keep per-proxy
testProviderProxy calls and delete the healthcheckProvider() invocation) so no
duplicate /delay requests are sent.
- Around line 157-162: The latency cache is keyed only by proxy.name which
collides across providers; change all reads and writes to use a namespaced key
like `${provider.name}::${proxy.name}` so values are unique per provider. Update
the ProxyCard prop latency (replace latencyMap[proxy.name]) to
latencyMap[`${provider.name}::${proxy.name}`], ensure wherever latency is set
(e.g., in onTestProxy or its helper that updates latencyMap) uses the same
namespaced key, and confirm testingProxies usage remains consistent with the
namespaced key (`testingProxies.has(\`${provider.name}::${proxy.name}\`)`).
Ensure any other lookups/sets of latencyMap or related caches use the same
composite key so cards from different providers don't overwrite each other.
In `@clash-dashboard/src/pages/Rules.tsx`:
- Around line 65-73: The runRuleUpdate function currently swallows errors from
updateRuleProvider; add a catch block to handle failures, call
setUpdatingRuleProviders to clear the loading state in finally (keep existing
finally) and inside the catch use the app's user-feedback mechanism (e.g.,
toast/errorSnackbar or setState error) to surface the error message and
optionally log it; specifically wrap await updateRuleProvider(provider.name) in
try/catch (or add a .catch) and on error call your notification helper with the
error and include provider.name, then still call queryClient.invalidateQueries
on success as now and retain the finally that removes provider.name from the
Set.
- Around line 84-184: The RuleProviderRulesPanel component is defined inside
Rules which causes remounts and lost state; extract RuleProviderRulesPanel to a
top-level component (either above the Rules function or into its own file), keep
its prop signature ({ name, behavior }) and the internal state vars (matchInput,
matchTarget) and useQuery hooks (queryKey ['rule-provider-rules', name],
['rule-provider-match', name, matchTarget]) unchanged, then update the Rules
component to import/use this top-level RuleProviderRulesPanel and pass through
the same name and behavior props; if moved to a separate file, export default or
named-export it and update the import in the file.
🪄 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 Plus
Run ID: b1ecded3-cc13-4293-ad36-661724d33519
📒 Files selected for processing (7)
clash-dashboard/src/App.tsxclash-dashboard/src/components/Layout.tsxclash-dashboard/src/components/ProxyGroups.tsxclash-dashboard/src/lib/api.tsclash-dashboard/src/pages/Proxies.tsxclash-dashboard/src/pages/ProxyList.tsxclash-dashboard/src/pages/Rules.tsx
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
- Add new /proxies route (ProxyList.tsx) showing static config proxies
and provider proxies grouped by provider name
- Add per-proxy latency test button using correct API endpoints:
/proxies/{name}/delay for static, /providers/proxies/{prov}/proxies/{name}/delay for provider proxies
- Add getProviderProxyDelay() to api.ts
- Fix selector jump glitch in Proxies.tsx and ProxyGroups.tsx:
remove invalidateQueries from onSettled, keep it only in onError
- Move rule providers section from Providers.tsx to Rules.tsx
- Update nav order: Overview / Flows / Connections / Proxies / Rules / DNS / Logs / Settings
- Remove Providers nav link, add Proxies link pointing to /proxies
- Remove /providers route from App.tsx router config
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add /providers as compatibility alias route (preserves existing bookmarks) - Namespace latency cache keys as provider::proxy to avoid collisions across providers with same-named proxies - Remove redundant healthcheckProvider() from testAllInProvider (per-proxy calls already cover it) - Add error handling / console.error to runRuleUpdate in Rules.tsx - Extract RuleProviderRulesPanel to module level to prevent state loss and unnecessary unmounts on Rules re-renders Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clash-dashboard/src/pages/ProxyList.tsx`:
- Around line 178-188: The proxies/providers fetches lack explicit error
handling; update the useQuery calls for getProxies and getProxyProviders to
destructure isError, error, and refetch (e.g., const { data: proxiesData,
isLoading: proxiesLoading, isError: proxiesError, error: proxiesErrorObj,
refetch: refetchProxies } = useQuery(...)) and in the render paths where
proxiesData/providersData and loading/empty states are handled (including the
provider/proxy list blocks), show a clear error UI when isError is true that
displays the error message (from error or error.message) and a Retry button that
calls refetchProxies/refetchProviders; apply the same pattern to the other
related query usages around the provider rendering logic
(providersData/providersLoading) so API failures render as errors rather than
empty states.
In `@clash-dashboard/src/pages/Rules.tsx`:
- Around line 141-145: The 'rule-providers' useQuery (queryKey
['rule-providers'], queryFn getRuleProviders) doesn't handle failures and lets
the UI fall through to the empty-state; add error handling by leveraging
useQuery's onError and/or reading isError and error from the hook (alongside
ruleIsLoading and ruleData) and render an explicit error state when the query
fails instead of the empty-state; update the rendering logic that currently uses
ruleData/ruleIsLoading to check for ruleIsError/error and show a helpful error
message (or retry action), and apply the same change to the other occurrence of
the 'rule-providers' query referenced elsewhere in this file.
- Around line 302-306: The clickable div that calls
toggleRuleProviderExpanded(provider.name) should be made keyboard-accessible:
replace the div (or modify it) so it is focusable and actionable via keyboard by
using a semantic <button> (or adding role="button" plus tabIndex={0}) and
handling onKeyDown for Enter/Space to call
toggleRuleProviderExpanded(provider.name); also add appropriate ARIA attributes
like aria-expanded and aria-controls tied to provider.name to expose the
expanded/collapsed state to assistive tech. Ensure the element preserves the
existing className and style so layout/appearance remain unchanged.
🪄 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 Plus
Run ID: 3e7cccc8-0265-48a3-b602-82e8b9c8eaab
📒 Files selected for processing (7)
clash-dashboard/src/App.tsxclash-dashboard/src/components/Layout.tsxclash-dashboard/src/components/ProxyGroups.tsxclash-dashboard/src/lib/api.tsclash-dashboard/src/pages/Proxies.tsxclash-dashboard/src/pages/ProxyList.tsxclash-dashboard/src/pages/Rules.tsx
✅ Files skipped from review due to trivial changes (1)
- clash-dashboard/src/lib/api.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- clash-dashboard/src/components/ProxyGroups.tsx
- clash-dashboard/src/pages/Proxies.tsx
- Fix latencyMap key bug in testProviderProxy (was using proxy.name instead of namespaced key, so latency display was broken) - Add isError handling for proxies and providers queries with error UI - Add isError handling for rule-providers query with error UI - Make rule provider expand/collapse keyboard-accessible (role=button, tabIndex, onKeyDown for Enter/Space) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clash-dashboard/src/pages/ProxyList.tsx`:
- Around line 78-86: The icon-only latency button in ProxyList.tsx (the <button>
that calls onTest and renders the Activity component) lacks a reliable
accessible name; add an aria-label to the button (e.g. aria-label="Test latency"
or, if a proxy/name variable is available, aria-label={`Test latency for
${proxy.name}`}) so screen readers can announce its purpose; keep the existing
title and testing/disabled logic intact and ensure the aria-label is updated if
the button context (proxy) changes.
- Around line 231-237: The testAllInProvider function currently calls
testProviderProxy for every proxy concurrently, causing a burst of /delay
requests; modify testAllInProvider to throttle concurrency by processing proxies
in small batches (or using a concurrency limit) instead of mapping all at once.
Implement this by splitting provider.proxies (or iterating) into chunks of a
configurable batch size (e.g., 5–10) and awaiting Promise.all on each chunk
before proceeding to the next, calling testProviderProxy(provider.name, proxy)
for each item; ensure setTestingProviders and the final
queryClient.invalidateQueries(['providers']) behavior remains the same.
- Around line 293-298: Rendering of static ProxyCard instances doesn't pass the
testing state, so the card's button never shows loading; update the render call
for ProxyCard to pass the testing flag from testingProxies for the same key used
by testStaticProxy (testingProxies[`static::${proxy.name}`]) along with the
existing latency prop (latencyMap[`static::${proxy.name}`]). In other words, add
a prop (e.g., isTesting or testing) to the ProxyCard invocation and feed it
testingProxies[`static::${proxy.name}`] so ProxyCard can disable/show loading
during an in-flight /proxies/{name}/delay request; keep the existing key, proxy,
latency, and onTest props unchanged.
🪄 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 Plus
Run ID: 3c0f2141-dc1c-4857-88a4-98f74f5cec6f
📒 Files selected for processing (2)
clash-dashboard/src/pages/ProxyList.tsxclash-dashboard/src/pages/Rules.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- clash-dashboard/src/pages/Rules.tsx
- Add aria-label to icon-only latency test button for screen reader support - Wire static proxy cards into testingProxies so loading state shows correctly and prevents overlapping requests on repeated clicks - Throttle 'Test All' to run in batches of 5 instead of all concurrently to avoid request bursts on large providers - Add aria-expanded to rule provider expand/collapse toggle Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Changes
New
/proxiespage (ProxyList.tsx)Replaces the old
/providerspage. Shows:/providerskept as a compatibility alias so existing bookmarks still workPer-proxy latency testing
GET /proxies/{name}/delayGET /providers/proxies/{provider}/{name}/healthcheck{provider}::{proxy}to avoid collisions when two providers have same-named proxiesFixed selector proxy jumping glitch
Removed
invalidateQueriesfromonSettledofselectMutationinProxies.tsxandProxyGroups.tsx. The optimistic update now sticks without being overwritten by an immediate refetch.Rule providers moved to Rules page
Rule provider section (with match tester + update button) now lives at the bottom of
Rules.tsxinstead of the old Providers page.RuleProviderRulesPanelextracted to module level to prevent state loss on re-renders.Nav updated
New order: Overview / Flows / Connections / Proxies / Rules / DNS / Logs / Settings
Summary by CodeRabbit
New Features
UI Enhancements
Bug Fixes
Chores