Conversation
Replaces the per-option chip row with one bifurcated chip per filter category:
left half is the category title, right half summarizes the selection
("Hashing" or "3 statuses") and opens an inline edit popover. Standalone filter
dropdowns are removed in favor of a single "+ Add Filter" trigger anchored at
the end of the row, which freezes its popover position so the popover doesn't
shift when chips render. On phone/tablet the nested side-panel collapses into
a drilldown that swaps the parent popover. Applied across MinerList, GroupsPage
and RacksPage via a new shared FilterChipsBar.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Updates the ProtoFleet client filter UX to better communicate AND-across / OR-within semantics by switching to one “bifurcated” chip per filter category and consolidating filter selection under a single “+ Add Filter” trigger (with responsive mobile drilldown behavior).
Changes:
- Add
freezePositionpopover behavior to prevent layout shifts (chips appearing) from dragging an open popover. - Introduce shared filter-chip UI (
FilterChip,FilterChipsBar) and update list pages (Miners/Groups/Racks) to use the new pattern. - Add/adjust unit and e2e tests for the new chip + nested filter interactions, including mobile/tablet drilldown.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| client/src/shared/components/Popover/usePopoverPosition.ts | Adds freezePosition support to stop tracking trigger coordinates after initial measurement. |
| client/src/shared/components/Popover/Popover.tsx | Adds freezePosition prop and forces portal-fixed render mode when enabled. |
| client/src/shared/components/List/List.test.tsx | Updates assertions for new chip test ids (category-level instead of per-option). |
| client/src/shared/components/List/Filters/types.ts | Adds pluralTitle for correct pluralization and prefixIcon for nested-filter triggers. |
| client/src/shared/components/List/Filters/NestedSubmenu.tsx | Refactors option row into reusable CheckboxOptionRow. |
| client/src/shared/components/List/Filters/NestedDropdownFilter.tsx | Adds prefix-icon trigger behavior, desktop category-row refactor, and mobile/tablet drilldown mode. |
| client/src/shared/components/List/Filters/NestedDropdownFilter.test.tsx | Adds viewport mocking + mobile drilldown coverage. |
| client/src/shared/components/List/Filters/Filters.tsx | Replaces per-option active pills with one FilterChip per dropdown category and supports keeping an “open” chip mounted. |
| client/src/shared/components/List/Filters/Filters.test.tsx | Updates/adds tests for bifurcated chips, plural summaries, clear behavior, and inline editing popover. |
| client/src/shared/components/List/Filters/FilterChipsBar.tsx | New shared “chips + Add Filter trigger” component. |
| client/src/shared/components/List/Filters/FilterChip.tsx | New bifurcated chip with inline-edit popover for a filter category. |
| client/src/protoFleet/features/rackManagement/pages/RacksPage.tsx | Migrates rack filters to FilterChipsBar and updates header layout. |
| client/src/protoFleet/features/groupManagement/pages/GroupsPage.tsx | Migrates group filters to FilterChipsBar and updates header layout. |
| client/src/protoFleet/features/fleetManagement/components/MinerList/MinerList.tsx | Updates filter metadata: plural titles + nested trigger becomes “Add Filter” with plus icon. |
| client/e2eTests/protoFleet/pages/racks.ts | Updates rack e2e flows to use “Add Filter” trigger and chip editing popover. |
| client/e2eTests/protoFleet/pages/miners.ts | Updates miner e2e flows to use nested “Add Filter” trigger and model submenu selection. |
Comments suppressed due to low confidence (1)
client/src/protoFleet/features/rackManagement/pages/RacksPage.tsx:144
handleClearFiltersmutatesselectedZonesRef.currentbutselectedZonesRefisn’t included in theuseCallbackdependency array. This is inconsistent with the rest of the file and will be flagged byreact-hooks/exhaustive-deps.
Include selectedZonesRef in the dependencies (or explicitly disable the rule with a comment if omission is intentional).
const handleClearFilters = useCallback(() => {
setSelectedZones([]);
selectedZonesRef.current = [];
setSelectedIssues([]);
selectedIssuesRef.current = [];
resetAndFetch();
}, [resetAndFetch, selectedIssuesRef]);
🔐 Codex Security Review
Review SummaryOverall Risk: LOW Findings[LOW] Server-backed filter editing now refetches on every checkbox toggle
NotesFrontend-only diff in scope. I did not find auth, SQLi, gRPC, command-injection, cryptostealing/pool-hijack, protobuf wire-format, or infrastructure issues in the reviewed hunks. I could not run the client Vitest suite in this environment because Generated by Codex Security Review | |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b9918b348
ℹ️ 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".
- Filters.tsx: clear openChipFilterValue on initialActiveFilters resync so back/forward or sibling URL writes don't leave a stale empty chip mounted. - usePopoverPosition: keep visibleViewport updates flowing while a frozen popover is open so the layout effect can re-clamp the anchor on viewport resize / zoom / mobile-chrome collapse. Trigger rect stays frozen. - RacksPage: include selectedZonesRef in handleFilterChange and handleClearFilters useCallback deps for parity with selectedIssuesRef. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Triaged the bot review feedback in 41abded: Fixed
Acknowledged, not fixing in this PR
|
- validateActiveFilter / validateActiveFilterNotVisible: scope to the chip's
edit button (`active-filter-{key}-edit`). The outer chip wrapper also carries
an `active-filter-*` testid, which previously matched twice and tripped
Playwright's strict mode.
- openModelSubmenu (miners) / openZoneSubmenu (racks): wait for either the
desktop portaled side submenu or the mobile drilldown back button so the
helpers work across viewports.
- dismissAddFilterPopover: click the page heading instead of (1, 1) to fire
useClickOutside without risking a hit on an interactive element near the
viewport corner.
- applyZoneFilter (racks): clear any existing zone chip first, then re-add
via the Add Filter trigger. Avoids racy editing inside the chip's popover.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The previous dismiss clicked `h1`, but Miners and Activity pages don't render one — Playwright timed out waiting for the locator. Toggling the same trigger that opened the popover is more reliable: it's never covered by its own popover and works on every page. Also rewrote `applyZoneFilter` and `toggleAllZoneFilters` to read the live checkbox state from the Add Filter submenu and toggle to match the desired set, instead of clearing the existing chip and re-adding. The two-step clear+re-add path was racing with `resetAndFetch` and occasionally left the list unfiltered. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Summary
Screen.Recording.2026-05-04.at.12.01.27.PM.mov
Test plan
🤖 Generated with Claude Code