feat: add inline filters variant with expandable sections #496
Conversation
WalkthroughAdds an inline filters variant with expandable sections, debounced text filtering, ToggleGroupWithLabel, new filter label keys, search and priority filters, model/request surface updates, mocked mapper changes, and related animations and styles. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Client as TicketList.client.tsx
participant FiltersUI as Filters / FilterItem / ToggleGroup
participant Framework as framework (GetTicketListQuery)
participant MockMapper as integrations/mocked mapper
participant API as Backend/API
User->>Client: open list / interact with filters
Client->>FiltersUI: render inline filters (variant="inline")
User->>FiltersUI: type search / set priority / toggle filters
FiltersUI->>Client: emit filter change (text debounced)
Client->>Framework: send GetTicketListQuery (includes search, priority)
Framework->>MockMapper: map request/block (local/dev)
MockMapper->>API: fetch filtered tickets
API-->>Client: return tickets
Client-->>User: render updated list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
![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 @packages/ui/src/components/Filters/FilterItem.tsx:
- Line 28: debouncedSubmit created via useMemo
(debounce(TEXT_FILTER_DEBOUNCE_MS, () => submitForm())) can fire after unmount
because it isn’t cancelled; change to create/retain the debounced function in
useMemo or a ref and add a useEffect that returns a cleanup which calls
debouncedSubmit.cancel() (or the debounce lib’s equivalent) whenever
debouncedSubmit or submitForm changes and on unmount; ensure you verify debounce
returns a cancel method (or switch to lodash.debounce) and reference the
debouncedSubmit, submitForm, TEXT_FILTER_DEBOUNCE_MS, useMemo and useEffect
symbols when applying the fix.
🧹 Nitpick comments (1)
packages/integrations/mocked/src/modules/cms/mappers/blocks/cms.ticket-list.mapper.ts (1)
541-549: RedundantdetailsUrlassignments inmapTicketListBlock.The
detailsUrlproperty is already defined in each mock object (lines 178, 357, 538) with the same values being assigned in the spread. This redundancy doesn't cause bugs but could be simplified.♻️ Optional cleanup
export const mapTicketListBlock = (locale: string): CMS.Model.TicketListBlock.TicketListBlock => { switch (locale) { case 'de': - return { ...MOCK_TICKET_LIST_BLOCK_DE, detailsUrl: '/faelle/{id}' }; + return MOCK_TICKET_LIST_BLOCK_DE; case 'pl': - return { ...MOCK_TICKET_LIST_BLOCK_PL, detailsUrl: '/zgloszenia/{id}' }; + return MOCK_TICKET_LIST_BLOCK_PL; default: - return { ...MOCK_TICKET_LIST_BLOCK_EN, detailsUrl: '/cases/{id}' }; + return MOCK_TICKET_LIST_BLOCK_EN; } };
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
.changeset/dull-dancers-hide.mddiff-feature.txtdiff-name-status.txtpackages/blocks/ticket-list/src/api-harmonization/ticket-list.model.tspackages/blocks/ticket-list/src/api-harmonization/ticket-list.request.tspackages/blocks/ticket-list/src/frontend/TicketList.client.tsxpackages/framework/src/modules/cms/models/blocks/ticket-list.model.tspackages/framework/src/modules/tickets/tickets.request.tspackages/framework/src/utils/models/filters.tspackages/integrations/mocked/src/modules/cms/mappers/blocks/cms.ticket-list.mapper.tspackages/ui/src/components/Filters/FilterItem.tsxpackages/ui/src/components/Filters/Filters.tsxpackages/ui/src/components/Filters/Filters.types.tspackages/ui/src/elements/toggle-group.tsxpackages/ui/src/globals.css
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-26T11:57:00.632Z
Learnt from: marcinkrasowski
Repo: o2sdev/openselfservice PR: 411
File: packages/framework/src/modules/cms/models/blocks/product-list.model.ts:29-46
Timestamp: 2025-11-26T11:57:00.632Z
Learning: In the framework layer (packages/framework/src/modules/cms/models/blocks/*.model.ts), block classes like ProductListBlock should NOT include explicit __typename discriminators. The __typename field is added at the API harmonization layer (packages/blocks/*/src/api-harmonization/*.model.ts) where it's needed for discriminated unions. The framework maintains normalized data models without these discriminators.
Applied to files:
packages/blocks/ticket-list/src/api-harmonization/ticket-list.model.tspackages/framework/src/modules/cms/models/blocks/ticket-list.model.ts
🧬 Code graph analysis (4)
packages/ui/src/elements/toggle-group.tsx (2)
packages/ui/src/elements/label.tsx (1)
Label(17-17)packages/ui/src/lib/utils.ts (1)
cn(5-7)
packages/ui/src/components/Filters/FilterItem.tsx (4)
packages/framework/src/utils/models/filters.ts (1)
FilterItem(12-17)packages/ui/src/components/Filters/Filters.types.ts (1)
FilterItemProps(28-38)packages/ui/src/elements/toggle-group.tsx (3)
ToggleGroupItem(124-124)ToggleGroupWithLabel(124-124)ToggleGroup(124-124)packages/ui/src/lib/utils.ts (1)
cn(5-7)
packages/framework/src/modules/cms/models/blocks/ticket-list.model.ts (3)
packages/framework/src/utils/models/filters.ts (1)
Filters(1-10)packages/ui/src/components/Filters/Filters.tsx (1)
Filters(329-478)packages/blocks/ticket-list/src/api-harmonization/ticket-list.model.ts (1)
Ticket(40-57)
packages/ui/src/components/Filters/Filters.types.ts (2)
packages/framework/src/utils/models/filters.ts (1)
Filters(1-10)packages/ui/src/components/Filters/Filters.tsx (1)
Filters(329-478)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: deploy-preview
🔇 Additional comments (23)
diff-feature.txt (1)
1-70: Incomplete review context: This file lists only documentation and configuration changes, not the feature implementation.The provided
diff-feature.txtis a high-level manifest of repo-wide file operations (adds, modifications, deletions). It shows extensive documentation restructuring (Contentful, Strapi, Algolia, Zendesk, Redis, SurveyJS, theming) and package manifest updates, but does not contain the actual implementation code for the inline filters variant feature described in the PR objectives.According to the PR summary, the following implementation files should exist:
@o2s/uipackageFilterscomponent withInlineFiltersContentsubcomponent- Helper functions:
separateLeadingItems,getActiveFilterBadges,getFilterResetValue- CSS keyframe animations with staggered timing
- Updates to
FilterItemcomponent withisInlineVariantprop- New
ToggleGroupWithLabelwrapper component- Type updates for
search,priority,isLabelHiddenfields- Mocked CMS data updates (EN, DE, PL)
To conduct a comprehensive review of this feature, please provide the actual implementation files containing the above components and logic, not just the file manifest.
packages/framework/src/utils/models/filters.ts (1)
37-37: LGTM! Consistent pattern for label visibility control.The addition of
isLabelHiddentoFilterToggleGroupaligns with the existing pattern used inFilterText(line 58) and supports the new inline filter variants.packages/framework/src/modules/tickets/tickets.request.ts (1)
21-22: LGTM! Backward-compatible query extensions.The new
searchandpriorityparameters are appropriately marked as optional, maintaining backward compatibility while enabling the new filtering capabilities described in the PR.packages/blocks/ticket-list/src/api-harmonization/ticket-list.request.ts (1)
10-11: LGTM! Proper interface alignment.The addition of
searchandpriorityfields correctly alignsGetTicketListBlockQuerywith the extendedTickets.Request.GetTicketListQueryinterface from the framework.packages/ui/src/globals.css (1)
141-161: LGTM! Clean animation definitions for filter transitions.The
filterFadeInandfilterFadeOutkeyframes provide smooth transitions with appropriate opacity and translation values. The 8px vertical offset creates a subtle, polished effect for the inline filter UI..changeset/dull-dancers-hide.md (1)
1-8: LGTM!The changeset correctly documents the minor version bumps for all affected packages and provides a clear description of the feature addition.
packages/blocks/ticket-list/src/frontend/TicketList.client.tsx (2)
41-42: LGTM!The new
searchandpriorityfilter fields are correctly initialized as empty strings in the initial filter state, matching the expected request query structure.
208-214: LGTM!The inline variant configuration is properly set up with the necessary labels for expandable filter sections. The new label keys (
showMoreFilters,hideMoreFilters,noActiveFilters) align with the inline filter UI requirements.packages/blocks/ticket-list/src/api-harmonization/ticket-list.model.ts (1)
26-28: LGTM!The new label fields are correctly added as optional properties to support the inline filters UI. The type extensions align with the corresponding framework model changes.
packages/framework/src/modules/cms/models/blocks/ticket-list.model.ts (2)
12-14: LGTM!The
Filterstype is correctly extended to include the newsearchandpriorityfields alongside the existingsortandviewModeoptions. All fields are appropriately optional.
24-26: LGTM!The new label fields are consistently added to both the runtime
labelsand the CMS metadataMeta.labels, ensuring proper support for the inline filters UI across the data model.Also applies to: 56-58
packages/ui/src/components/Filters/Filters.types.ts (2)
4-11: LGTM!The new
FilterLabelsinterface consolidates all filter-related labels into a single, reusable type. All fields are appropriately optional, maintaining flexibility and backward compatibility. The interface includes comprehensive labeling for the inline filter variant, including accessibility labels.
20-20: LGTM!The type updates correctly replace inline label types with the shared
FilterLabelsinterface, improving type consistency. The newisInlineVariantflag inFilterItemPropsenables proper conditional behavior for the inline filter variant.Also applies to: 36-37
packages/ui/src/elements/toggle-group.tsx (1)
100-122: LGTM! NewToggleGroupWithLabelcomponent is well-structured.The component correctly wraps
ToggleGroupwith accessibility support viasr-onlyfor hidden labels. The props are properly spread to the underlyingToggleGroup.One minor note: the
classNameorder in thecn()call at line 116 placeslabelClassNamebefore the conditionalsr-only. This works correctly sincesr-onlywill override visibility-related styles whenisLabelHiddenis true.packages/integrations/mocked/src/modules/cms/mappers/blocks/cms.ticket-list.mapper.ts (1)
135-146: LGTM! Priority filter added consistently across all locales.The new
priorityfilter with HIGH/MEDIUM/LOW options is properly localized and maintains consistent structure across EN, DE, and PL locales.Also applies to: 314-325, 495-506
packages/ui/src/components/Filters/Filters.tsx (5)
68-73: Consider usingtoSorted()or documenting mutation safety.The
arraysEqualfunction correctly creates copies before sorting, avoiding mutation of input arrays. This is good practice.
186-207: Animation effect handles cleanup correctly.The effect properly tracks previous expansion state via ref, manages animation timing, and returns cleanup functions for timeouts. The staggered timing based on
otherItems.lengthprovides smooth visual transitions.
316-320: "Clear all filters" button only appears with 2+ active filters.This is intentional UX - when only one filter is active, users can remove it via the badge's X button. The "Clear all filters" button adds value only with multiple active filters.
306-310: Good accessibility: aria-label for filter removal button.The remove button correctly includes an accessible label that falls back to a sensible default when
labels?.removeFilterAriaLabelis not provided.
214-220: FilterDateRange reset value should be handled explicitly or excluded from badges.
getFilterResetValuehas no case forFilterDateRangeand defaults to returning an empty string. WhileFilterDateRangeis not inSUPPORTED_FILTER_TYPESand filters are excluded from the rendered UI,getActiveFilterBadgesreceives the unfiltered items array. If a date range filter with changed values appears in the data, users could remove it via the badge, resetting it to an empty string—which may not be the correct reset value for dates. Either add explicit handling forFilterDateRangeingetFilterResetValue, or ensure date range filters are excluded from badge generation.packages/ui/src/components/Filters/FilterItem.tsx (3)
126-147: Single-selection toggle group rendering logic doesn't considerisInlineVariant.For
allowMultiple: true(lines 82-103), the code checksisInlineVariantto decide betweenToggleGroupWithLabelandToggleGroup. However, for single selection (allowMultiple: false), the rendering is based solely onisLeading:
isLeading: true→ToggleGroup(no label)isLeading: false→ToggleGroupWithLabel(with label)This asymmetry may be intentional (leading items never need labels), but verify this aligns with the expected behavior for single-selection inline filters.
41-64: Dynamic corner rounding for adjacent selected items is a nice UX touch.The logic correctly rounds corners only on the outer edges of contiguous selected items, creating a visual grouping effect.
66-80: ALL option handling in multi-select is correct.When "ALL" is clicked, the selection is cleared (empty array). When other options are selected, "ALL" is filtered out. The ref-based tracking of whether "ALL" was clicked ensures correct behavior in the async flow.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui/src/components/Filters/FilterItem.tsx (1)
93-137: Inconsistent wrapping: leading single-select lacks ScrollContainer.For
allowMultiple: true, the toggle items are wrapped inScrollContainer(line 86-88), but for single-select withisLeading: true(lines 114-121), there's noScrollContainer. This could cause overflow issues if there are many options.🔧 Proposed fix for consistency
return isLeading ? ( <ToggleGroup type="single" variant="solid" value={currentValue} onValueChange={handleValueChange} > - {toggleGroupItems} + <ScrollContainer className="scroll-container flex whitespace-nowrap w-full"> + {toggleGroupItems} + </ScrollContainer> </ToggleGroup> ) : (
🧹 Nitpick comments (3)
packages/ui/src/elements/toggle-group.tsx (2)
103-107: Consider adding ref forwarding support to ToggleGroupWithLabelProps.The
ToggleGroupWithLabelPropsextendsToggleGroupPropswhich includes arefproperty, but theToggleGroupWithLabelcomponent doesn't forward a ref to the outerdivwrapper. If consumers need to reference the container element, this could be limiting.♻️ Proposed enhancement
-type ToggleGroupWithLabelProps = ToggleGroupProps & { +type ToggleGroupWithLabelProps = Omit<ToggleGroupProps, 'ref'> & { label: string | React.ReactNode; labelClassName?: string; isLabelHidden?: boolean; + ref?: React.Ref<HTMLDivElement>; };
109-125: MissinghtmlForassociation between Label and ToggleGroup.The
Labelcomponent is rendered without anhtmlForattribute linking it to the toggle group. While toggle groups don't have a single input to reference, for accessibility it's common to usearia-labelledbyon the group referencing the label's id.♻️ Proposed fix for accessibility
+import { useId } from 'react'; + const ToggleGroupWithLabel = ({ className, label, labelClassName, isLabelHidden, children, ...props }: ToggleGroupWithLabelProps) => { + const labelId = useId(); return ( <div className="grid gap-2"> - <Label className={cn(labelClassName, isLabelHidden && 'sr-only')}>{label}</Label> - <ToggleGroup {...props} className={className}> + <Label id={labelId} className={cn(labelClassName, isLabelHidden && 'sr-only')}>{label}</Label> + <ToggleGroup {...props} className={className} aria-labelledby={labelId}> {children} </ToggleGroup> </div> ); };packages/ui/src/components/Filters/FilterItem.tsx (1)
38-92: Potential stale closure issue withallWasClickedRefin onClick handler.The
allWasClickedRefpattern works but relies on theonClickfiring beforeonValueChange. This ordering is implementation-dependent on the underlying Radix toggle group. Additionally, if multiple items are clicked rapidly, the ref state could become inconsistent.Consider using the value directly in
handleValueChangeto determine if ALL was selected:♻️ Alternative approach without ref timing dependency
const handleValueChange = async (value: string[]) => { let newValue: string[]; + const wasAllClicked = value.includes('ALL') && !currentValue.includes('ALL'); - if (allWasClickedRef.current) { + if (wasAllClicked) { newValue = []; - allWasClickedRef.current = false; } else { newValue = value.filter((v) => v !== 'ALL'); } await setFieldValue(field.name, newValue); if (isLeading || isInlineVariant) { await submitForm(); } };This approach determines intent by comparing the new value array with the current value, eliminating the timing dependency.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/ui/src/components/Filters/FilterItem.tsxpackages/ui/src/elements/toggle-group.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
packages/ui/src/elements/toggle-group.tsx (3)
packages/ui/src/elements/checkbox.tsx (1)
Label(99-99)packages/ui/src/elements/label.tsx (1)
Label(17-17)packages/ui/src/lib/utils.ts (1)
cn(5-7)
packages/ui/src/components/Filters/FilterItem.tsx (3)
packages/framework/src/utils/models/filters.ts (1)
FilterItem(12-17)packages/ui/src/components/Filters/Filters.types.ts (1)
FilterItemProps(28-38)packages/ui/src/elements/toggle-group.tsx (3)
ToggleGroupItem(127-127)ToggleGroupWithLabel(127-127)ToggleGroup(127-127)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: deploy-preview
🔇 Additional comments (7)
packages/ui/src/elements/toggle-group.tsx (2)
7-7: LGTM!Import of Label component is correctly placed and necessary for the new
ToggleGroupWithLabelcomponent.
18-32: LGTM!The
toggleGroupVariantsconfiguration is well-structured with clear variant definitions. The CSS classes for handling adjacent[data-state=on]elements to remove border-radius appropriately creates a visually connected group.packages/ui/src/components/Filters/FilterItem.tsx (5)
16-17: LGTM!The debounce constant is clearly named and the 1000ms value is reasonable for text input filtering to reduce unnecessary submissions while still feeling responsive.
28-34: LGTM!Good use of
useMemofor the debounced function with proper cleanup inuseEffectto cancel pending debounced calls on unmount, preventing potential state updates on unmounted components.
193-196: LGTM!The text filter correctly uses the debounced submit, updating the field value immediately for responsive UI while debouncing the actual form submission.
150-152: LGTM!The
isInlineVariantcondition is correctly added to trigger auto-submit for FilterSelect, consistent with the FilterToggleGroup behavior.
78-89: If FilterToggleGroup type includes isLabelHidden property, pass it to ToggleGroupWithLabel.The
ToggleGroupWithLabelcomponent supportsisLabelHidden(as shown byInputWithLabelpattern forFilterTextat line 188). However, it is not currently being passed in theFilterToggleGroupcases. If theFilterToggleGrouptype definition includes this property, it should be passed similarly:isLabelHidden={item.isLabelHidden}.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui/src/components/Filters/FilterItem.tsx (1)
41-76: Multi ToggleGroup: “ALL” reset depends ononClick(pointer-only) — fix to support keyboard/a11y.
With keyboard interaction,allWasClickedRefwon’t flip, so selecting “ALL” while other values are selected may fail to clear them. Consider deriving intent from previous selection instead ofonClick.Proposed fix (remove pointer-only dependency)
- const toggleGroupItems = item.options.map((option) => { + const toggleGroupItems = item.options.map((option) => { return ( <ToggleGroupItem key={option.value} value={option.value} className="min-w-[98px] rounded-sm h-9" - onClick={() => { - allWasClickedRef.current = option.value === 'ALL'; - }} > {option.label} </ToggleGroupItem> ); }); const handleValueChange = async (value: string[]) => { let newValue: string[]; - if (allWasClickedRef.current) { - newValue = []; - allWasClickedRef.current = false; - } else { - newValue = value.filter((v) => v !== 'ALL'); - } + const hadSelections = (field.value?.length ?? 0) > 0; + const includesAll = value.includes('ALL'); + + if (includesAll && hadSelections) { + // user chose ALL to clear existing selections + newValue = []; + } else { + // coming from ALL-state (empty) or normal multi-select updates + newValue = value.filter((v) => v !== 'ALL'); + } await setFieldValue(field.name, newValue); if (isLeading || isInlineVariant) { await submitForm(); } };Follow-up: if you adopt this, you can delete
allWasClickedRef(Line 26) and dropuseReffrom the React import.</details> </blockquote></details> </blockquote></details>🧹 Nitpick comments (2)
packages/ui/src/components/Filters/FilterItem.tsx (2)
3-16: Hook/constant additions look fine; debounce delay change should be intentional.
TEXT_FILTER_DEBOUNCE_MS=1000 is a noticeable UX shift—worth double-checking it matches product expectations.
96-125: Single ToggleGroup path is clear; consider extracting shared rendering to reduce duplication.
ThetoggleGroupItems+ScrollContainer+ToggleGroupWithLabelpattern is now duplicated across single/multiple.📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/ui/src/components/Filters/FilterItem.tsx🧰 Additional context used
🧬 Code graph analysis (1)
packages/ui/src/components/Filters/FilterItem.tsx (3)
packages/framework/src/utils/models/filters.ts (1)
FilterItem(12-17)packages/ui/src/components/Filters/Filters.types.ts (1)
FilterItemProps(28-38)packages/ui/src/elements/toggle-group.tsx (2)
ToggleGroupItem(127-127)ToggleGroupWithLabel(127-127)⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: deploy-preview
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
packages/ui/src/components/Filters/FilterItem.tsx (3)
141-144: Good: Select now respectsisInlineVariantfor auto-submit.
This matches the new inline behavior without forcing submits in other contexts.
184-187: Verify intended behavior: text filter now always auto-submits (debounced), even when not leading/inline.
Toggle/select submission is gated by(isLeading || isInlineVariant), but text isn’t—if that’s unintended, gatedebouncedSubmit()similarly.Possible adjustment (if you want parity with other filter types)
onChange={async (e) => { await setFieldValue(field.name, e.target.value); - debouncedSubmit(); + if (isLeading || isInlineVariant) { + debouncedSubmit(); + } }}
28-35: Debounced submit lifecycle and API contract verified.
The[email protected]library correctly returns a callable withcancel(options?: { upcomingOnly?: boolean }): void. The TypeScript types match the runtime behavior, and your cleanup pattern in the useEffect is properly implemented.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/ui/src/components/Filters/FilterItem.tsx:
- Line 16: The debounce constant TEXT_FILTER_DEBOUNCE_MS in FilterItem.tsx is
set to 1000ms which can feel sluggish; change its value to a lower, more
responsive default (e.g., 300–500ms — set to 500ms for a conservative
improvement) and optionally expose it as a prop or configurable value on the
FilterItem component so tests and callers can override the debounce when needed;
update any references to TEXT_FILTER_DEBOUNCE_MS and relevant tests/usage to
reflect the new default.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/ui/src/components/Filters/FilterItem.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
packages/ui/src/components/Filters/FilterItem.tsx (3)
packages/framework/src/utils/models/filters.ts (1)
FilterItem(12-17)packages/ui/src/components/Filters/Filters.types.ts (1)
FilterItemProps(28-38)packages/ui/src/elements/toggle-group.tsx (2)
ToggleGroupItem(127-127)ToggleGroupWithLabel(127-127)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: deploy-preview
🔇 Additional comments (5)
packages/ui/src/components/Filters/FilterItem.tsx (5)
26-32: LGTM! Proper debounce lifecycle management.The debounced submit is correctly memoized and cleaned up on unmount, preventing potential memory leaks and stale submissions.
57-73: ALL selection logic is correct but complex.The handleValueChange function properly manages the "ALL" toggle behavior: clicking ALL when filters are active clears them, and any other selection automatically excludes ALL. The state cycle (empty → ALL displayed → specific selections → ALL clears → empty) works as intended for a multi-select toggle with an "ALL" option.
93-121: LGTM! Single selection toggle refactor is clean.The single-selection path correctly handles ALL as an empty filter state, defaults appropriately, and consistently applies auto-submit behavior for inline variants.
138-140: LGTM! Consistent inline variant behavior.The auto-submit logic for FilterSelect now correctly supports the inline variant, matching the pattern used in FilterToggleGroup.
181-184: LGTM! Text filter debounce refactored correctly.The field value updates immediately for responsive UI feedback, while form submission is properly debounced. The cleanup in useEffect ensures cancelled submissions if the component unmounts.
What does this PR do?
Related Ticket(s)
Key Changes
isLeading: true) are always visibleFilterscomponent in@o2s/uipackageInlineFiltersContentcomponent for managing inline filter state and animationsseparateLeadingItems,getActiveFilterBadges,getFilterResetValueFilterItemcomponent to support inline variant behaviorisInlineVariantprop for auto-submit behaviorToggleGroupWithLabelwrapper component for labeled toggle groupssearchandpriorityfields to ticket request typesshowMoreFilters,hideMoreFilters,noActiveFiltersisLabelHiddenoption forFilterToggleGrouppriorityfilterFilterSelecttoFilterToggleGroupwithallowMultiple: trueSummary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.