fix(client): hide no-pools callout on the Pools settings null state#95
fix(client): hide no-pools callout on the Pools settings null state#95rongxin-liu merged 3 commits intomainfrom
Conversation
The global "No mining pools configured" banner is rendered above every authenticated route in Proto OS. On the Pools settings page itself, the page already shows its own null-state card with the same CTA, so the banner stacks on top of it and duplicates the empty-state prompt. Centralize the visibility logic in a pathname-aware helper and route both layout consumers (App, KpiLayout) through it so the banner is suppressed only when the Pools page is in its null state. The banner still renders on the Pools page when pools are configured but offline, since that's a real connectivity error rather than a null state. Closes #94 Made-with: Cursor
🔐 Codex Security Review
Review SummaryOverall Risk: LOW FindingsNo concrete findings in the reviewed diff. The change is limited to ProtoOS frontend banner-routing logic and associated tests, and I did not identify a security, correctness, or reliability regression in the changed hunks. Notes
Generated by Codex Security Review | |
There was a problem hiding this comment.
Pull request overview
This PR fixes a ProtoOS UI duplication where the global “No mining pools configured.” banner appeared on the Pools settings page even when that page is already showing its own null-state card and CTA.
Changes:
- Introduced a centralized, pathname-aware helper (
getNoPoolsCalloutState) to compute when the NoPools callout should be shown. - Updated the two layout-level callout renderers (
App,KpiLayout) to use the shared helper and suppress the banner only on/settings/mining-poolswhen pools are truly unconfigured. - Added unit tests for the helper plus App-level integration tests for the key visibility scenarios.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| client/src/protoOS/features/kpis/components/KpiLayout/KpiLayout.tsx | Uses the shared helper (including pathname) to decide whether to render the NoPools callout. |
| client/src/protoOS/components/NoPoolsCallout/utility.ts | New helper that centralizes pool live/configured detection and mining-pools-route suppression logic. |
| client/src/protoOS/components/NoPoolsCallout/utility.test.ts | Adds a pathname + pool-state matrix to verify helper behavior. |
| client/src/protoOS/components/App/App.tsx | Routes App-level callout rendering through the shared helper and suppresses on Pools null state. |
| client/src/protoOS/components/App/App.test.tsx | Adds integration tests validating the banner suppression/visibility across routes and pool states. |
mcharles-square
left a comment
There was a problem hiding this comment.
call out on using matched path seems valid
Derive the Pools settings route match from shared route metadata so the NoPools callout suppression stays aligned with router changes. Made-with: Cursor
Summary
getNoPoolsCalloutState) and routed both layout-level consumers (App,KpiLayout) through it. The helper now derives the Pools settings match fromsettingsRouteMetadata.miningPools.pathviamatchPath, including the embedded/miners/:id/...route, so it stays aligned with router changes. The banner is suppressed only when the Pools page is in its null state; if pools are configured but every one is offline, the banner stays (that's a real connectivity error, not a null state).Test plan
npx vitest run src/protoOS/components/App/App.test.tsx src/protoOS/components/NoPoolsCallout/utility.test.ts— added a utility-level matrix and three App-level integration tests; 29/29 passing.npx tsc --noEmitfromclient/.npx prettier --checkon the touched files.npx eslint --max-warnings 0on the touched files.Made with Cursor