feat(ui): update Filter Button and remove ExpressionFilterBar#326
feat(ui): update Filter Button and remove ExpressionFilterBar#326
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR consolidates resource type filtering across the dashboard UI. It removes the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@libs/react/ui/src/components/dashboard/toolbar/filter-button.tsx`:
- Around line 24-27: The component's logic (filterOptions, selectedLabel,
selectedIndex) can break when value refers to a disabled RESOURCE_TYPE_OPTIONS
entry; add upstream validation in DashboardProvider.setResourceType to reject or
coerce disabled values, and also harden the UI: when computing selectedIndex in
filter-button.tsx, detect selectedIndex === -1 and fallback to the first enabled
option (or its id/label) so menu highlighting never compares against -1; ensure
the same validation is applied where setResourceType is called so disabled
resource types cannot become the provider's value.
🧹 Nitpick comments (2)
libs/react/ui/src/components/dashboard/toolbar/filter-button.tsx (2)
11-20: Use Button prop types to avoid dropping button-specific attributes.Line 17 uses
HTMLAttributes<HTMLButtonElement>, which omitsdisabled,type, and any customButtonprops while you still spread...propsinto<Button>. Consider usingComponentPropsWithoutRef<typeof Button>(and omitvariant/sizeif you want them fixed) so callers can pass standard button attributes safely.♻️ Proposed typing adjustment
-import type {HTMLAttributes} from 'react'; +import type {ComponentPropsWithoutRef} from 'react'; -export interface FilterButtonProps extends HTMLAttributes<HTMLButtonElement> { +type ButtonBaseProps = Omit<ComponentPropsWithoutRef<typeof Button>, 'variant' | 'size'>; +export interface FilterButtonProps extends ButtonBaseProps { value: ResourceType; onValueChange: (value: ResourceType) => void; }
101-135: Restore selection semantics for accessibility.Lines 105–135 use
DropdownMenuItemwith purely visual selection. ConsiderDropdownMenuRadioGroup/DropdownMenuRadioItem(orrole="menuitemradio"+aria-checked) so screen readers can detect the active selection.♿ Example using radio items (if supported by components/dropdown-menu)
import { DropdownMenu, DropdownMenuContent, - DropdownMenuGroup, - DropdownMenuItem, DropdownMenuLabel, + DropdownMenuRadioGroup, + DropdownMenuRadioItem, DropdownMenuTrigger, } from 'components/dropdown-menu'; -<DropdownMenuGroup> +<DropdownMenuRadioGroup + value={value} + onValueChange={(next) => handleFilterChange(next as ResourceType)} +> {filterOptions.map((option, index) => { return ( - <DropdownMenuItem + <DropdownMenuRadioItem key={option.id} - onClick={() => handleFilterChange(option.id)} + value={option.id} style={{ paddingLeft: calculatePaddingLeft(index), }} className={cn( 'relative hover:text-foreground-neutral-base', selectedIndex === index && 'text-foreground-neutral-base', )} > ... - </DropdownMenuItem> + </DropdownMenuRadioItem> ); })} -</DropdownMenuGroup> +</DropdownMenuRadioGroup>
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
libs/react/ui/src/components/dashboard/context/dashboard-context.tsxlibs/react/ui/src/components/dashboard/context/index.tslibs/react/ui/src/components/dashboard/context/types.tslibs/react/ui/src/components/dashboard/filters/expression-filter-bar.tsxlibs/react/ui/src/components/dashboard/filters/index.tslibs/react/ui/src/components/dashboard/index.tslibs/react/ui/src/components/dashboard/pages/analytics-page.tsxlibs/react/ui/src/components/dashboard/toolbar/filter-button.tsxlibs/react/ui/src/components/dashboard/toolbar/toolbar-actions.tsx
💤 Files with no reviewable changes (3)
- libs/react/ui/src/components/dashboard/pages/analytics-page.tsx
- libs/react/ui/src/components/dashboard/filters/expression-filter-bar.tsx
- libs/react/ui/src/components/dashboard/filters/index.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
We handle errors at the edge of our applications in most cases. Do not recommend to add error handling around every single function. We prefer them to bubble up and be handled at upper layers.
Files:
libs/react/ui/src/components/dashboard/context/types.tslibs/react/ui/src/components/dashboard/toolbar/toolbar-actions.tsxlibs/react/ui/src/components/dashboard/context/dashboard-context.tsxlibs/react/ui/src/components/dashboard/index.tslibs/react/ui/src/components/dashboard/toolbar/filter-button.tsxlibs/react/ui/src/components/dashboard/context/index.ts
🧬 Code graph analysis (2)
libs/react/ui/src/components/dashboard/toolbar/toolbar-actions.tsx (2)
libs/react/ui/src/components/dashboard/context/dashboard-context.tsx (1)
useDashboardContext(202-208)libs/react/ui/src/components/dashboard/toolbar/filter-button.tsx (1)
FilterButton(22-142)
libs/react/ui/src/components/dashboard/toolbar/filter-button.tsx (2)
libs/react/ui/src/components/dashboard/context/types.ts (1)
ResourceType(33-39)libs/react/ui/src/components/dashboard/context/dashboard-context.tsx (2)
RESOURCE_TYPE_OPTIONS(66-72)RESOURCE_TYPE_LABELS(57-64)
🔇 Additional comments (7)
libs/react/ui/src/components/dashboard/toolbar/toolbar-actions.tsx (1)
68-73: Good wiring to the new resourceType state.The updated context bindings and controlled
FilterButtonAPI usage look consistent.libs/react/ui/src/components/dashboard/context/types.ts (1)
33-48: Verify compatibility of the new ResourceType literals.If any persisted state, URL params, or backend contracts still use the old hyphenated identifiers, add a migration or mapping so selections keep working.
libs/react/ui/src/components/dashboard/index.ts (1)
21-21: Re-export looks good.Keeps the dashboard public API aligned with the new context metadata.
libs/react/ui/src/components/dashboard/context/dashboard-context.tsx (3)
10-17: Type import expansion is fine.The additional type imports align with the new resource type metadata.
48-72: Resource type metadata addition looks solid.Centralized constants make the UI wiring cleaner and more consistent.
98-126: Default updated to the new ResourceType format.Matches the dot-separated identifiers used elsewhere.
libs/react/ui/src/components/dashboard/context/index.ts (1)
6-12: Re-export additions are good.Keeps the context index as the single entry point for resource type metadata.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Pull request overview
This pull request refactors the filtering mechanism in the dashboard UI by replacing the ExpressionFilterBar component with an enhanced FilterButton component and standardizing resource type naming from kebab-case to dot notation.
Changes:
- Removed the standalone
ExpressionFilterBarcomponent and replaced its functionality with an updatedFilterButtondropdown - Changed resource type format from kebab-case (e.g.,
ci-pipeline) to dot notation (e.g.,ci.pipeline) - Updated the
FilterButtoncomponent to use resource type selection instead of status filtering with checkboxes - Changed keyboard shortcut from 'F' key to '⌘↓' (Cmd/Ctrl+ArrowDown)
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
toolbar-actions.tsx |
Updated to use resourceType/setResourceType from context instead of filters/setFilters |
filter-button.tsx |
Complete rewrite: changed from checkbox-based status filter to dropdown-based resource type selector with tree visualization |
analytics-page.tsx |
Removed the standalone ExpressionFilterBar component usage |
index.ts (dashboard) |
Removed exports for ExpressionFilterBar and ResourceTypeOption types |
filters/index.ts |
Deleted entire file as filter components are removed |
expression-filter-bar.tsx |
Deleted entire component file |
types.ts (context) |
Updated ResourceType union and added ResourceTypeOption interface with new dot notation values |
index.ts (context) |
Added exports for RESOURCE_TYPE_LABELS, RESOURCE_TYPE_OPTIONS, and RESOURCE_TYPES |
dashboard-context.tsx |
Added resource type constants and labels, updated default value to ci.pipeline |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
libs/react/ui/src/components/dashboard/toolbar/filter-button.tsx
Outdated
Show resolved
Hide resolved
kylengn
changed the title
2 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.