Fold custom views into Fleet top-level tab nav (#398)#478
Conversation
github-actions
Bot
added
javascript
Replace the secondary ViewsBar with a right-aligned "My views" dropdown in the Fleet top tab row. Saved views now capture the active section tab (sites/buildings/racks/miners) alongside filters + sort, so clicking a view restores its full state. - Drop built-in views (`All miners` / `Needs attention` / `Offline`); v1 saved-view records migrate to `tab: "miners"` on read so existing users keep their views. - Rename `useMinerViews` -> `useFleetViews`; `summarizeFilters` becomes tab-aware so racks-tab views can render building/site labels. - Move racks-tab filters (`zone`, `issues`) and the grid/list segmented control (`display`) into URL params so views can capture them. Child pages publish filter metadata up via the FleetLayout outlet context. - Add an "Include display mode" toggle in the view modal mirroring "Include sort order". - Mobile docks the views dropdown beside the Fleet heading; desktop keeps it in the TabStrip's trailing slot. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR re-homes ProtoFleet “saved views” into the Fleet top-level navigation by replacing the old ViewsBar with a FleetViewTabs dropdown in FleetLayout. It also evolves the saved-view schema so each view captures the owning Fleet tab and expands URL-driven state (notably racks filters and display mode) so views can fully restore UI state.
Changes:
- Introduces tab-scoped saved views (
tab: FleetTabId) with v1 localStorage migration totab="miners"and removes built-in views. - Adds
FleetViewTabs(dropdown + kebab actions) and wires it intoFleetLayout, removingViewsBarfrom the miners list. - Moves racks
zone/issuesfilters and the grid/listdisplaymode into URL params and updates view summarization/modal UX accordingly.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| client/src/shared/components/Tab/TabStrip.tsx | Adds an active override to TabStripItem active styling. |
| client/src/protoFleet/features/fleetManagement/views/viewSummary.ts | Adds tab-aware filter summaries and new display-mode summary/diff helpers. |
| client/src/protoFleet/features/fleetManagement/views/viewSummary.test.ts | Updates filter-summary tests for tab-aware API and new label sources. |
| client/src/protoFleet/features/fleetManagement/views/useFleetViews.ts | Renames/generalizes miner views hook to fleet views; persists tab with views. |
| client/src/protoFleet/features/fleetManagement/views/useFleetViews.test.tsx | Updates hook tests for tab-scoped views and v1 migration behavior. |
| client/src/protoFleet/features/fleetManagement/views/savedViews.ts | Bumps schema v2, adds FleetTabId, per-tab param whitelists, removes built-ins. |
| client/src/protoFleet/features/fleetManagement/views/savedViews.test.ts | Updates tests for v2 schema, per-tab canonicalization, and migration. |
| client/src/protoFleet/features/fleetManagement/pages/RacksPage.tsx | Drives zone/issues and display mode via URL so views can capture/restore them; publishes filter label context. |
| client/src/protoFleet/features/fleetManagement/components/ViewsBar/ViewsBar.tsx | Deletes the legacy secondary saved-views tab bar. |
| client/src/protoFleet/features/fleetManagement/components/ViewsBar/ViewsBar.test.tsx | Removes tests for the deleted ViewsBar. |
| client/src/protoFleet/features/fleetManagement/components/ViewsBar/index.ts | Removes ViewsBar re-export. |
| client/src/protoFleet/features/fleetManagement/components/MinerList/MinerList.tsx | Removes mounting of ViewsBar/views hook from miners page. |
| client/src/protoFleet/features/fleetManagement/components/FleetViewTabs/ViewModal.tsx | Extends view modal to support “include display mode” with display diffs. |
| client/src/protoFleet/features/fleetManagement/components/FleetViewTabs/index.ts | Exports FleetViewTabs. |
| client/src/protoFleet/features/fleetManagement/components/FleetViewTabs/FleetViewTabs.tsx | Adds the new dropdown-based views selector + kebab actions. |
| client/src/protoFleet/features/fleetManagement/components/FleetLayout/outletContext.ts | Adds a publish API for child tabs to provide filter label metadata; adds optional outlet hook. |
| client/src/protoFleet/features/fleetManagement/components/FleetLayout/index.ts | Re-exports useOptionalFleetOutletContext. |
| client/src/protoFleet/features/fleetManagement/components/FleetLayout/FleetLayout.tsx | Wires FleetViewTabs into the top nav (desktop trailing slot + mobile header). |
| client/src/protoFleet/features/fleetManagement/components/FleetLayout/FleetLayout.test.tsx | Updates store mock to provide useUsername. |
| client/src/protoFleet/features/fleetManagement/components/Fleet/Fleet.tsx | Publishes group/rack label sources up to FleetLayout for modal summaries. |
| client/src/protoFleet/features/fleetManagement/components/Fleet/Fleet.test.tsx | Updates FleetLayout outlet context mock with publishViewFilterContext. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c49290f4bd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
🔐 Codex Security Review
Review SummaryOverall Risk: NONE FindingsNo findings identified in the latest diff. NotesReviewed Generated by Codex Security Review | |
The minersFiltersViews spec was written against the old per-tab ViewsBar
UI. This PR's switch to the FleetViewTabs dropdown left every saved-view
helper pointing at obsolete testids, causing all 5 saved-view tests to
timeout on `locator.click`.
Rewrites the page-object helpers against the new selectors
(`fleet-view-tabs-{trigger,views-popover,kebab,kebab-popover,
delete-dialog}`, action rows, and the empty-state `+ New view` button),
adds `clickClearActiveView` for the "no active view" flow, and uses
`:visible` filters so the mobile + desktop twin-mounts don't trip strict
mode.
Spec changes:
- Replace the now-removed "All miners" built-in view switch with
`clickClearActiveView` in the update test.
- Drop the "deleted from the views bar" test: the new UI only exposes
delete via the active-view kebab, so the inactive-delete path no
longer exists. The active-delete path is already covered.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 203a04ba90
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Substantive fixes:
- RacksPage: drop comma-split on zone/issues URL parsing — zone labels
may legitimately contain commas (e.g. "DC1, Row A"). Read uses
getAll() only; writes use repeated keys.
- RacksPage: remove the effect that auto-wrote `display=<stored>` into
the URL on absence. The effect re-added `display=` immediately after
activating a view that intentionally omitted it via the "Include
display mode" toggle, leaving the view permanently dirty.
- savedViews: drop `sort`/`dir` from the racks whitelist. Rack sort is
still local to useDeviceSetListState, so capturing those keys made
saved views appear to restore sort while the list ignored them.
- FleetViewTabs/ViewModal: add `intent: "update" | "rename"` discriminator
to the update mode. Rename now skips updateUserViewParams entirely, so
renaming a dirty view no longer silently overwrites the saved filters
with the dirty state. Rename modal renders as a name-only edit
("Rename view" title, "Rename" submit).
- FleetViewTabs: gate the dropdown `+ New view` row on
`canSaveCurrentTab` (was already gated on the empty-state button).
- RacksPage: collapse the building-key and zones/issues effects into one
`filterFetchKey` effect so navigations that touch multiple filters
fire one refetch instead of two.
Comment / API cleanups:
- TabStrip: `active` prop only overrides when explicitly `true`; passing
`false` (or omitting it) falls back to `id === activeId`. Avoids the
footgun where `active={someBool}` could force an item inactive even
when the context marked it active.
- Drop stale comments referring to zone/issues as component-local on
RacksPage. Tighten the JSDoc on the views trigger to describe the
text-style button it actually renders. Update the `handleClearActiveView`
comment to match the full-reset behavior.
Tests:
- Add unit coverage for summarizeDisplay, diffDisplaySummaries, and
stripDisplayFromSearchParams.
- Update the racks canonicalization assertion now that sort is no
longer whitelisted on that tab.
- Update the rename e2e to expect the new "Rename view" modal title and
"Rename" submit button.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b718e4126f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The previous `selectedZones.join(",")` / `selectedIssues.join(",")`
fold made distinct selections collide once values contained the
delimiter — e.g. `["DC1,Row A", "B"]` and `["DC1", "Row A,B"]` both
serialized to `"DC1,Row A,B"`. Because this key is the only refetch
trigger after URL-driven zone/issue changes, switching between
colliding saved views (or chip selections) would update the URL and
chips while skipping `resetAndFetch`, leaving the rack list filtered
by the previous selection.
Encode the tuple as JSON instead so the delimiter character can't
appear ambiguously in any value.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1429589c91
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Rack sort lived only in useDeviceSetListState's internal state, so whitelisting `sort`/`dir` on the racks tab would have made saved views appear to capture sort while the visible list ignored them. The prior fix dropped sort from the racks whitelist; this commit promotes sort to the URL and re-adds the whitelist entries so saved rack views actually round-trip sort. Wiring: - useDeviceSetListState now accepts an optional `initialSort` callback used to seed sortConfig at mount, so a deep-link or saved-view activation lands with the right ordering on the very first fetch. - RacksPage reads `?sort=&dir=` (validated against SORTABLE_COLUMNS) and passes a capture-once initializer to the hook. - handleRackSort — shared by the grid dropdown and the list column headers — writes the URL; a sync effect propagates external URL changes (saved view activation, deep link) into the hook by calling handleSort when urlSort differs from currentSort. Same effect no-ops when the page itself drove the change, so there's no double-fetch. - summarizeSort and summarizeDisplay are now tab-aware. Only tabs that own the param (miners + racks for sort, racks for display) surface a summary. This prevents the New/Update modal from offering an "Include sort"/"Include display" toggle for cross-tab URL residue that the tab's canonicalization whitelist would silently strip. - savedViews: re-add SORT_KEYS to the racks whitelist. Docstring updated to describe the URL ownership now that sort is shared by grid + list mechanisms. Tests: - summarizeSort gains rack + cross-tab cases. - summarizeDisplay gains miners/buildings/sites null cases. - savedViews canonicalization for racks now preserves sort. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Include site in miner view keys
When a miner view is activated from a rack URL scoped by site=..., buildUrlForView treats site as unrelated to miners because it is absent from this whitelist, so it carries the rack site filter into /fleet/miners. The miners URL parser does consume site= into filter.siteIds, which means an unfiltered miner view can open site-filtered while still looking clean because canonicalization also ignores that key; include site in the miner-owned params (and its summary) so it is stripped or saved like the other miner filters.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Summary
Folds saved views into the Fleet top-level tab nav: the secondary
ViewsBaris gone, and a right-aligned "My views" dropdown lives in the same tab strip as Sites/Buildings/Racks/Miners. A saved view now captures the active section tab plus that tab's filter+sort state (and the racks grid/list toggle), so clicking a view restores its full destination — not just a set of filters on the current page. Built-in views (All miners/Needs attention/Offline) are dropped; user-created views from v1 migrate totab: "miners"on read.Resolves #398.
How it works
Storage shape (v2). A
SavedViewis now{ id, name, tab, searchParams, createdAt }.tabis aFleetTabId(sites | buildings | racks | miners).searchParamsis a canonicalized URL query — only the keys whitelisted for that tab survivecanonicalizeSearchParams, so transient or unrelated query state never leaks into a saved view. Persistence lives inlocalStorageunderproto-fleet-miner-views:<username>.normalizeSavedViewsRecordreads v1 records (notab) and assigns themtab: "miners", which preserves every existing user view.One hook, one component.
useMinerViewsis renamed touseFleetViewsand is mounted once at theFleetLayoutlevel.FleetViewTabs(replacingViewsBar) renders the right-aligned trigger, the views dropdown, the kebab actions (Reset / Update / Rename / Delete), and the create/updateViewModal. It compares the URL's canonicalized params against the active view's saved params to compute the "dirty" state, and tints the trigger texttext-intent-warning-50when they diverge.Activating a view. Clicking a view computes a new URL via
buildUrlForView: the view's canonical params are merged with any current URL params that are not in the active tab's filter/sort whitelist, thenview=<id>is appended.FleetViewTabsthennavigate()s to/fleet/<view.tab>?<params>. Becausetabis part of the view, switching from Miners to a racks-tab view changes both the route and the filter set in one navigation.Per-tab filter context. The saved-view modal needs human-readable labels for
building=/site=/group=/rack=filter ids, butFleetViewTabslives above the tab pages. To solve this without prop-drilling,FleetLayoutexposespublishViewFilterContext(...)on its outlet context. Each tab pushes its known sources (Miners publishesavailableGroups+availableRacks; Racks publishesavailableBuildings+availableSites). Each call is a partial merge — a tab can only overwrite the keys it owns — and the merged context flows intoFleetViewTabsasfilterContext.summarizeFiltersthen dispatches ontabto render the right entries.Racks-tab URL migration. Zone and issues filters and the grid/list segmented control on
RacksPagewere previously component-local state (zone/issues) and Zustand-only (display). They now live in the URL (?zone=,?issues=,?display=) so they can be captured into a view. The grid/list setter still mirrors into Zustand so the operator's preference survives navigating away from a?display=...URL; the URL is the source of truth, Zustand is the fallback. AnInclude display modetoggle is added toViewModalalongside the existingInclude sort ordertoggle.Responsive placement.
TabStripgains an optionaltrailingslot. On desktop (laptop:breakpoint and up),FleetViewTabsrenders into that slot, right-aligned across from the section tabs. On mobile it docks next to the<h1>Fleet</h1>heading. Both mounts use the same component; visibility-class gates keep only one interactive at a time.Diagrams
Components and data flow
Activating a saved view
sequenceDiagram participant U as Operator participant V as FleetViewTabs participant H as useFleetViews participant R as Router U->>V: Click view row V->>H: findView(id) H-->>V: SavedView { tab, searchParams } V->>V: buildUrlForView(view, currentParams) Note right of V: keeps non-filter URL keys, drops other tabs' filter keys, appends view=id V->>R: navigate /fleet/{tab}?{params} R-->>U: Section tab + filters + sort + display restoredDirty-state evaluation on every URL change
Areas of the code involved
views/savedViews.tsSavedViewgainstab. Per-tab whitelist (FILTER_AND_SORT_KEYS_BY_TAB) gates canonicalization. v1 records normalize totab: "miners".buildUrlForViewpreserves unrelated URL keys.views/useFleetViews.ts(wasuseMinerViews.ts)addUserViewnow requirestab.updateUserViewParamsre-canonicalizes against the view's own tab.reorderUserViewsadded.FleetViewTabs; rename ripples through imports.views/viewSummary.tssummarizeFiltersdispatches ontab(miners vs racks). AddssummarizeDisplay,diffDisplaySummaries, andFilterSummaryContextwithavailableBuildings/availableSites.components/FleetViewTabs/FleetViewTabs.tsxViewsBar. Renders trigger + views popover + kebab popover, computes dirty state, owns the create/update modal flow.components/FleetViewTabs/ViewModal.tsxViewsBar/. AddsInclude display modetoggle (mirrorsInclude sort order) and rendersDisplaySummarydiffs.components/FleetLayout/FleetLayout.tsxuseFleetViewsandFleetViewTabs. OwnsviewFilterContextstate and thepublishViewFilterContextcallback. Mounts the views control twice (mobile heading row + desktopTabStriptrailing slot) gated bylaptop:classes.components/FleetLayout/outletContext.tsFleetOutletContext, includingpublishViewFilterContext, plus auseOptionalFleetOutletContextfor pages that also mount outside the Fleet shell.FleetLayout.tsxso non-Fleet routes can import the type without pulling the shell in.pages/RacksPage.tsxzone,issues, anddisplayare now URL-backed.setRacksViewModemirrors to both URL and Zustand. Publishes buildings + sites up via outlet context.display.components/Fleet/Fleet.tsxavailableGroups+availableRacksup via outlet context.components/MinerList/MinerList.tsxViewsBarmount and the per-pageuseMinerViewscall; views are now chrome-level.shared/components/Tab/TabStrip.tsxtrailingslot and a per-itemactiveprop override.activeoverride lets a view tab read as active alongside its section tab.components/ViewsBar/*ViewsBar.tsx,ViewsBar.test.tsx,index.ts.FleetViewTabs.views/useMinerViews.ts+ testsuseFleetViews.ts/useFleetViews.test.tsx.views/andFleetLayout/savedViews.test.ts,viewSummary.test.ts,useFleetViews.test.tsx, plus minor updates toFleet.test.tsxandFleetLayout.test.tsx.Key technical decisions & trade-offs
normalizeSavedViewsRecord. No write-on-read step; no version-bumping ceremony. Existing v1 records keep working until their next mutation, at which point they are persisted as v2. Chosen over an explicit migration pass that would have to run somewhere on app boot.buildUrlForView. Saved views can never accidentally capture unrelated URL state (e.g. a transient modal-open param), and switching tabs while activating a view strips the source tab's filter keys cleanly. Chosen over a deny-list approach.All miners/Needs attention/Offlineare no longer special-cased — they're just bookmarks the operator can recreate. Simpler model; the trade-off is that fresh users see an empty views dropdown.display. URL is the source of truth so a saved view can dictate grid vs. list; Zustand keeps the operator's last choice across non-?display=...navigations. The alternative (URL-only) regressed the "remembered density" UX.TabStrip's layout. Keeps the responsive surface visually right but means the component is mounted twice in the DOM; only one is interactive at a time thanks tolaptop:visibility classes.TabStripItem.activeprop override is a small but worth flagging — it lets two items in the same strip read as active. Used to keep a section tab and its scoped view both visibly active. Chosen over forking the component.Testing & validation
just lint— clean.client/: 2731 unit tests pass vianpm test -- --run. Tests updated for tab-aware canonicalization, v1→v2 migration, theFleetViewTabsflows, and the new display-summary diffing.tsc --noEmit— clean./fleet/minersand/fleet/racksis recommended since the top nav was restructured — confirm that creating a view on each tab, activating it from the other tab, dirtying it, and resetting it all behave as described.