feat(component): advanced search form#734
Merged
anandmindfire merged 4 commits intomindfiredigital:developmentfrom Apr 17, 2026
Merged
feat(component): advanced search form#734anandmindfire merged 4 commits intomindfiredigital:developmentfrom
anandmindfire merged 4 commits intomindfiredigital:developmentfrom
Conversation
Contributor
WalkthroughReplaces the legacy SearchProvider composition with a new, declarative AdvancedSearchForm feature: a config-driven root component, typed per-filter APIs, compound subcomponents (Filters, Actions, SavedSearches, FacetedHints, ResultsCount), client-side filtering/export, Storybook stories, docs, tests, demo, and registry entry. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Form as AdvancedSearchFormRoot
participant Context as SearchContext
participant Store as ResultsEngine
participant Persist as SavedSearchStore
participant Exporter as ExportHandler
User->>Form: change filter / click Apply / click Export / save/delete saved search
Form->>Context: setActiveFilters(filters)
Note over Form,Context: debounce if autoApply enabled
Context->>Store: applyFilters(activeFilters)
Store-->>Context: filteredResults + count
Context-->>Form: render results + count
alt Save search
Form->>Persist: add/delete(savedSearch)
Persist-->>Form: updated saved list
end
alt Export
Form->>Exporter: export(filteredResults, format)
Exporter-->>User: download file
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/docs/src/components/Demo/AdvancedSearchFormDemo.tsx`:
- Around line 958-962: The faceted chip handler always wraps the selected value
in an array causing mismatch with SearchSelectFilter-backed facets like "role"
and "department"; update the onSelect handler in SearchFacetedHints to call
handleFilterChange with a scalar value for select-type facets and an array for
multi-select facets by checking the category/filter type (use facetedCategories
or the filters map to detect select filters) so that SearchSelectFilter-backed
categories receive value (not [value]) and multi-value facets keep an array.
- Around line 452-543: The generated snippet returned by generateCode contains
undeclared/unused React symbols and wrong reset behavior: declare/import
useState at top of the snippet and include minimal mock data and option arrays
(mockUsers, roleOptions, departmentOptions, statusOptions) inside the generated
AdvancedSearchDemo example so it is copy-pasteable, ensure AdvancedSearchDemo
defines handleSearch that updates filteredResults via setFilteredResults, and
change the SearchActions onReset handler to both clear active filters
(setActiveFilters({})) and restore filteredResults to the full list
(setFilteredResults(mockUsers)); reference generateCode, AdvancedSearchDemo,
handleSearch, setActiveFilters, and setFilteredResults when locating where to
add these declarations and the onReset change.
In
`@apps/docs/versioned_docs/version-1.0.3/templates/pages/forms/advanced-search-form.mdx`:
- Line 23: The page still documents deprecated components (SearchProvider,
SearchHeader, SearchPagination, etc.) while the install command and template
name use the new compound API (advanced-search-form); update the page to match
the new API by replacing references and props tables for the old components with
documentation for the new compound component(s) exported by
advanced-search-form, add accurate prop tables and usage examples for the new
compound API, and remove any leftover mentions of
SearchProvider/SearchHeader/SearchPagination so the install instruction and
rendered docs are consistent.
In
`@apps/storybook/src/templates/pages/form-pages/advanced-search-form/advanced-search-form.stories.tsx`:
- Around line 421-425: The onSelect handler in SearchFacetedHints is always
storing facet values as arrays (setActiveFilters({ ...activeFilters,
[categoryId]: [value] })), which breaks controlled state for select-style
filters like the "role" driven by SearchSelectFilter that expects a scalar
string; change the handler to detect the category's filter type (use
facetedCategoriesData.find(c => c.id === categoryId)?.type or similar) and store
a scalar string for select/single-value types and an array for multi-value
types, and apply the same conditional fix to the corresponding onRemove/update
handler referenced around lines 544-546 so select filters remain scalar while
multi-select facets stay arrays.
In
`@packages/registry/templates/pages/advanced-search-form/advanced-search-form.test.tsx`:
- Around line 758-784: The current tests only assert visible text; update the
Accessibility tests to assert programmatic associations and keyboard
reachability: for form labels replace getByText checks with label-based queries
(getByLabelText / getByRole('combobox'|'textbox' etc.) against
AdvancedSearchForm fields to ensure labels are associated via
htmlFor/aria-labelledby, and add keyboard interaction assertions using
userEvent.tab and userEvent.keyboard (or fireEvent.keyDown) to verify custom
triggers in AdvancedSearchForm.Filters are focusable and operable via keyboard
(Enter/Space) and that focus moves as expected. Ensure you reference
AdvancedSearchForm and AdvancedSearchForm.Filters in the updated tests and
assert aria attributes (aria-label/aria-labelledby/role) and focus state after
keyboard events.
In `@packages/registry/templates/pages/advanced-search-form/index.tsx`:
- Around line 480-496: Replace the clickable div used as the custom filter
trigger with a real <button> element so it is keyboard-focusable and operable;
update the element around the selectedLabels display (the onClick={() =>
setIsOpen(!isOpen)} block) to use a button, add type="button", and include
aria-expanded={isOpen} plus any needed aria-controls; retain the same className
logic (cn(..., selectedLabels.length > 0 && "ring-2 ring-blue-500")) and
preserve Typography children; apply the same change to the other trigger
instance mentioned (the block around lines 691-705) so both multi-select and
date-picker triggers are accessible.
- Around line 970-976: handleFilterChange is causing duplicate searches because
it calls onSearch immediately while a top-level effect also schedules onSearch
when activeFilters changes; remove the immediate onSearch call from
handleFilterChange (and the analogous code at the other occurrence) and let the
existing effect that watches activeFilters perform the search when autoApply is
true, or alternatively add a simple guard in that effect to ignore the update if
it was already applied (compare previous filters vs newFilters) so each filter
change only triggers one onSearch; reference handleFilterChange and the root
useEffect that watches activeFilters (also fix the duplicated block at the other
occurrence).
- Around line 330-354: The visible text rendered by the Typography component is
not programmatically associated with the input (so screen readers won't announce
it); update the rendering so the label is a real HTML label element tied to the
input using htmlFor={filter.id} (or wrap the input in the label) for the text
produced by Typography (reference: Typography, filter.id, input, handleChange,
localValue), and for range/numeric filters (the other blocks around lines
382-410 and 866-910) render distinct "Min" and "Max" visible labels each bound
to their corresponding inputs (unique ids) and add appropriate aria-label or
aria-labelledby attributes where needed to ensure each control is properly
announced.
- Around line 155-159: AdvancedSearchForm currently provides savedSearches via
context but does not expose the deletion handler, so SearchSavedSearches only
works when onDelete is passed directly; update AdvancedSearchForm to include the
onDeleteSearch handler in the same context object it already provides (mirror
how savedSearches/onSaveSearch are passed) and update SearchSavedSearches to
consume that onDeleteSearch from context instead of relying on a prop. Locate
the AdvancedSearchForm context provider and add the onDelete/onDeleteSearch
function to the context value, and update SearchSavedSearches to call that
context function (ensuring the handler signature matches SavedSearch) — also
apply the same change to the other similar component block (the one referenced
around the other SearchSavedSearches usage).
- Around line 32-35: The form never restores configured per-filter defaults
because the root state only hydrates from initialFilters and Reset clears to {},
so merge each filter's defaultValue into the initial root filters state and make
Reset revert to that merged defaults state instead of {}: in AdvancedSearchForm
(packages/.../advanced-search-form/index.tsx) locate where initialFilters is
used to build the root state and update the initializer to iterate filters'
defaultValue (handle string, string[], date-range, and numeric-range shapes) and
merge those values into the starting filters object; update the Reset handler
(resetFilters/onReset) to replace state with that merged defaults object (not an
empty object) and ensure controlled inputs read from the root state so
CLI-configured defaults appear on first render and after reset.
- Around line 826-857: The local state variables localMin and localMax are only
initialized from value and never updated when value changes externally; add a
React.useEffect that watches value.min and value.max (and possibly
activeFilters) and calls setLocalMin(value.min?.toString() || "") and
setLocalMax(value.max?.toString() || "") to keep the inputs in sync, while
preserving the existing debounceTimer/useEffect and handleChange/onChange
behavior so external resets or saved searches update the displayed inputs
immediately.
- Around line 236-244: formatDate and parseDate currently round-trip via UTC
(toISOString and new Date("YYYY-MM-DD")) which shifts calendar-only dates across
timezones; change formatDate to build a "YYYY-MM-DD" string from the Date's
local year/month/day (e.g., use date.getFullYear(), date.getMonth()+1,
date.getDate() with zero-padding) and change parseDate to parse the "YYYY-MM-DD"
string manually (split into year, month, day and construct with new Date(year,
month-1, day)) so the values remain local-calendar dates and return null for
invalid inputs; update the functions named formatDate and parseDate accordingly.
- Around line 1288-1297: handleSelect currently always writes the selected value
as an array ([value]) which breaks scalar filters; update handleSelect to
inspect the existing activeFilters[categoryId] (use Array.isArray or typeof) and
preserve its shape when setting the new value (i.e., set an array if the
existing filter is an array, otherwise set a scalar), then call setActiveFilters
and autoApply/onSearch as before; reference handleSelect, activeFilters,
setActiveFilters, autoApply, and onSearch to locate the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f3b3e2bc-9125-4fad-81a8-1eadb349f5ec
📒 Files selected for processing (8)
apps/docs/src/components/Demo/AdvancedSearchFormDemo.tsxapps/docs/src/components/UI/advanced-search-form/index.tsxapps/docs/versioned_docs/version-1.0.3/templates/pages/forms/advanced-search-form.mdxapps/storybook/src/templates/pages/form-pages/advanced-search-form/advanced-search-form.stories.tsxapps/storybook/src/templates/pages/form-pages/advanced-search-form/index.tsxpackages/registry/registry.jsonpackages/registry/templates/pages/advanced-search-form/advanced-search-form.test.tsxpackages/registry/templates/pages/advanced-search-form/index.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
packages/registry/registry.json
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Register components in
packages/registry/components/directory by creating a component subdirectory and adding an entry topackages/registry/registry.json
Files:
packages/registry/registry.json
🧠 Learnings (7)
📚 Learning: 2026-03-18T09:43:15.140Z
Learnt from: CR
Repo: mindfiredigital/ignix-ui PR: 0
File: README.md:0-0
Timestamp: 2026-03-18T09:43:15.140Z
Learning: Applies to registry.json : Update registry.json when adding new components to the codebase
Applied to files:
packages/registry/registry.json
📚 Learning: 2026-03-18T09:43:08.260Z
Learnt from: CR
Repo: mindfiredigital/ignix-ui PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2026-03-18T09:43:08.260Z
Learning: Applies to packages/registry/registry.json : Register components in `packages/registry/components/` directory by creating a component subdirectory and adding an entry to `packages/registry/registry.json`
Applied to files:
packages/registry/registry.json
📚 Learning: 2026-03-18T09:43:08.260Z
Learnt from: CR
Repo: mindfiredigital/ignix-ui PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2026-03-18T09:43:08.260Z
Learning: Applies to apps/storybook/src/components/**/*.{test,spec}.{tsx,ts} : Write unit tests for new components using Vitest, located alongside the component source code
Applied to files:
packages/registry/templates/pages/advanced-search-form/advanced-search-form.test.tsx
📚 Learning: 2026-03-18T09:43:15.140Z
Learnt from: CR
Repo: mindfiredigital/ignix-ui PR: 0
File: README.md:0-0
Timestamp: 2026-03-18T09:43:15.140Z
Learning: Applies to src/**/*.test.tsx : Create component test files alongside implementation files with .test.tsx extension
Applied to files:
packages/registry/templates/pages/advanced-search-form/advanced-search-form.test.tsx
📚 Learning: 2026-03-18T09:43:15.140Z
Learnt from: CR
Repo: mindfiredigital/ignix-ui PR: 0
File: README.md:0-0
Timestamp: 2026-03-18T09:43:15.140Z
Learning: Applies to src/**/*.tsx : Ensure components are accessible by default and follow accessibility best practices
Applied to files:
packages/registry/templates/pages/advanced-search-form/advanced-search-form.test.tsx
📚 Learning: 2026-03-18T09:43:08.260Z
Learnt from: CR
Repo: mindfiredigital/ignix-ui PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2026-03-18T09:43:08.260Z
Learning: Applies to apps/storybook/src/components/**/*.stories.{tsx,ts} : Create a Story File named `[component-name].stories.tsx` in the same directory as the component file with argTypes, controls, and example stories
Applied to files:
apps/storybook/src/templates/pages/form-pages/advanced-search-form/advanced-search-form.stories.tsx
📚 Learning: 2026-03-18T09:43:08.260Z
Learnt from: CR
Repo: mindfiredigital/ignix-ui PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2026-03-18T09:43:08.260Z
Learning: Applies to apps/storybook/src/components/**/*.{tsx,ts,jsx,js} : Create Component Files in `apps/storybook/src/components/` directory with a component subdirectory for each component
Applied to files:
apps/storybook/src/templates/pages/form-pages/advanced-search-form/advanced-search-form.stories.tsx
🔇 Additional comments (2)
packages/registry/templates/pages/advanced-search-form/index.tsx (1)
163-169: Good provider guard.Failing fast in
useSearch()makes compound-component misuse obvious and avoids harder-to-debug undefined-context crashes later.packages/registry/registry.json (1)
25-43: Registry entry is in place.Good addition of the
advanced-search-formkey and template path; that keeps the CLI/docs name aligned with the new implementation. Based on learnings: Update registry.json when adding new components to the codebase.
Comment on lines
+452
to
+543
| const generateCode = (): string => { | ||
| return `import { | ||
| AdvancedSearchForm, | ||
| SearchFilters, | ||
| SearchResults, | ||
| SearchPagination, | ||
| ThemeToggle, | ||
| } from '@ignix-ui/advanced-search'; | ||
| SearchActions, | ||
| SearchSavedSearches, | ||
| SearchFacetedHints, | ||
| SearchResultsCount, | ||
| SearchTextFilter, | ||
| SearchSelectFilter, | ||
| SearchMultiSelectFilter, | ||
| SearchDateRangeFilter, | ||
| SearchNumericRangeFilter | ||
| } from '@ignix-ui/advanced-search-form'; | ||
|
|
||
| export default function AdvancedSearchDemo() { | ||
| return ( | ||
| <div className="min-h-screen bg-background"> | ||
| <div className="container max-w-7xl mx-auto px-4 py-8"> | ||
| <SearchProvider | ||
| variant="${theme}" | ||
| cardVariant="${cardVariant}" | ||
| pageSize={${pageSize}} | ||
| debounceMs={${debounceMs}} | ||
| theme="${theme}" | ||
| ${preset !== 'none' ? `initialFilters={${JSON.stringify(initialFilters, null, 4)}}` : ''} | ||
| > | ||
| ${showTitle ? '<SearchHeader />' : ''} | ||
| ${showTabs ? '<SearchTabs />' : ''} | ||
| <div className="flex flex-col lg:flex-row gap-6 mt-6"> | ||
| <div className="flex-1 space-y-4"> | ||
| <SearchFilters expanded={${filtersExpanded}} /> | ||
| <SearchResults | ||
| showAvatar={${displaySettings.showAvatar}} | ||
| showEmail={${displaySettings.showEmail}} | ||
| showTags={${displaySettings.showTags}} | ||
| showStatus={${displaySettings.showStatus}} | ||
| showRole={${displaySettings.showRole}} | ||
| showDate={${displaySettings.showDate}} | ||
| /> | ||
| ${showPagination ? '<SearchPagination />' : ''} | ||
| </div> | ||
| </div> | ||
| <ThemeToggle /> | ||
| </SearchProvider> | ||
| function AdvancedSearchDemo() { | ||
| const [activeFilters, setActiveFilters] = useState({}); | ||
| const [filteredResults, setFilteredResults] = useState(mockUsers); | ||
|
|
||
| const handleSearch = () => { | ||
| let results = [...mockUsers]; | ||
|
|
||
| if (activeFilters.name) { | ||
| results = results.filter(u => | ||
| u.name.toLowerCase().includes(activeFilters.name.toLowerCase()) | ||
| ); | ||
| } | ||
| if (activeFilters.role) { | ||
| results = results.filter(u => | ||
| u.role.toLowerCase() === activeFilters.role.toLowerCase() | ||
| ); | ||
| } | ||
| if (activeFilters.department) { | ||
| results = results.filter(u => | ||
| u.department.toLowerCase() === activeFilters.department.toLowerCase() | ||
| ); | ||
| } | ||
| if (activeFilters.status && Array.isArray(activeFilters.status)) { | ||
| results = results.filter(u => | ||
| activeFilters.status.includes(u.status.toLowerCase()) | ||
| ); | ||
| } | ||
|
|
||
| setFilteredResults(results); | ||
| }; | ||
|
|
||
| return ( | ||
| <AdvancedSearchForm variant="${theme}" layout="stacked"> | ||
| <div className="space-y-6"> | ||
| <SearchFilters> | ||
| <div className="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 gap-4"> | ||
| <SearchSelectFilter | ||
| filter={{ id: "role", label: "Role", type: "select", options: roleOptions }} | ||
| value={activeFilters.role || ""} | ||
| onChange={(val) => setActiveFilters({ ...activeFilters, role: val })} | ||
| /> | ||
| <SearchSelectFilter | ||
| filter={{ id: "department", label: "Department", type: "select", options: departmentOptions }} | ||
| value={activeFilters.department || ""} | ||
| onChange={(val) => setActiveFilters({ ...activeFilters, department: val })} | ||
| /> | ||
| <SearchMultiSelectFilter | ||
| filter={{ id: "status", label: "Status", type: "multi-select", options: statusOptions }} | ||
| value={activeFilters.status || []} | ||
| onChange={(val) => setActiveFilters({ ...activeFilters, status: val })} | ||
| /> | ||
| </div> | ||
| </SearchFilters> | ||
|
|
||
| <SearchActions | ||
| onApply={handleSearch} | ||
| onReset={() => setActiveFilters({})} | ||
| showApply={true} | ||
| showReset={true} | ||
| /> | ||
|
|
||
| <SearchResultsCount count={filteredResults.length} total={mockUsers.length} /> | ||
|
|
||
| <div className="space-y-2"> | ||
| {filteredResults.map(user => ( | ||
| <div key={user.id} className="p-4 border rounded-lg"> | ||
| <p className="font-semibold">{user.name}</p> | ||
| <p className="text-sm text-gray-600"> | ||
| {user.role} • {user.department} • {user.status} | ||
| </p> | ||
| </div> | ||
| ))} | ||
| </div> | ||
| ); | ||
| } | ||
| `; | ||
| </div> | ||
| </AdvancedSearchForm> | ||
| ); | ||
| }`; |
Contributor
There was a problem hiding this comment.
Make the code tab copy-pasteable.
The generated snippet uses useState, mockUsers, roleOptions, departmentOptions, and statusOptions without importing or declaring them, and its Reset handler only clears filters instead of restoring filteredResults. Right now the code tab does not compile and diverges from the live demo behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/docs/src/components/Demo/AdvancedSearchFormDemo.tsx` around lines 452 -
543, The generated snippet returned by generateCode contains undeclared/unused
React symbols and wrong reset behavior: declare/import useState at top of the
snippet and include minimal mock data and option arrays (mockUsers, roleOptions,
departmentOptions, statusOptions) inside the generated AdvancedSearchDemo
example so it is copy-pasteable, ensure AdvancedSearchDemo defines handleSearch
that updates filteredResults via setFilteredResults, and change the
SearchActions onReset handler to both clear active filters
(setActiveFilters({})) and restore filteredResults to the full list
(setFilteredResults(mockUsers)); reference generateCode, AdvancedSearchDemo,
handleSearch, setActiveFilters, and setFilteredResults when locating where to
add these declarations and the onReset change.
Comment on lines
+958
to
+962
| <SearchFacetedHints | ||
| categories={facetedCategories} | ||
| onSelect={(categoryId, value): void => { | ||
| handleFilterChange(categoryId, [value]); | ||
| }} |
Contributor
There was a problem hiding this comment.
Keep facet state shape aligned with the filter type.
role and department are backed by SearchSelectFilter, but this handler always stores [value]. Clicking those facet chips puts the demo into an invalid state and the selects/results logic drift apart.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/docs/src/components/Demo/AdvancedSearchFormDemo.tsx` around lines 958 -
962, The faceted chip handler always wraps the selected value in an array
causing mismatch with SearchSelectFilter-backed facets like "role" and
"department"; update the onSelect handler in SearchFacetedHints to call
handleFilterChange with a scalar value for select-type facets and an array for
multi-select facets by checking the category/filter type (use facetedCategories
or the filters map to detect select filters) so that SearchSelectFilter-backed
categories receive value (not [value]) and multi-value facets keep an array.
Comment on lines
+421
to
+425
| <SearchFacetedHints | ||
| categories={facetedCategoriesData} | ||
| onSelect={(categoryId: string, value: string) => { | ||
| setActiveFilters({ ...activeFilters, [categoryId]: [value] }); | ||
| }} |
Contributor
There was a problem hiding this comment.
Keep facet values consistent with the underlying filter type.
Both handlers store [value] for every category, but role is driven by a SearchSelectFilter that expects a scalar string. Clicking a role facet will put these stories into an invalid controlled state.
Also applies to: 544-546
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/storybook/src/templates/pages/form-pages/advanced-search-form/advanced-search-form.stories.tsx`
around lines 421 - 425, The onSelect handler in SearchFacetedHints is always
storing facet values as arrays (setActiveFilters({ ...activeFilters,
[categoryId]: [value] })), which breaks controlled state for select-style
filters like the "role" driven by SearchSelectFilter that expects a scalar
string; change the handler to detect the category's filter type (use
facetedCategoriesData.find(c => c.id === categoryId)?.type or similar) and store
a scalar string for select/single-value types and an array for multi-value
types, and apply the same conditional fix to the corresponding onRemove/update
handler referenced around lines 544-546 so select filters remain scalar while
multi-select facets stay arrays.
Comment on lines
+758
to
+784
| describe('Accessibility', () => { | ||
| it('has proper ARIA attributes', () => { | ||
| render( | ||
| <AdvancedSearchForm filters={allFilters} ariaLabel="Custom search label"> | ||
| <AdvancedSearchForm.Filters /> | ||
| </AdvancedSearchForm> | ||
| ); | ||
|
|
||
| const searchElement = screen.getByRole('search'); | ||
| expect(searchElement).toHaveAttribute('aria-label', 'Custom search label'); | ||
| }); | ||
|
|
||
| it('has proper form labels', () => { | ||
| render( | ||
| <AdvancedSearchForm filters={allFilters}> | ||
| <AdvancedSearchForm.Filters /> | ||
| </AdvancedSearchForm> | ||
| ); | ||
|
|
||
| // Use getByText since the labels are rendered as divs with the text | ||
| expect(screen.getByText('Name')).toBeInTheDocument(); | ||
| expect(screen.getByText('Status')).toBeInTheDocument(); | ||
| expect(screen.getByText('Categories')).toBeInTheDocument(); | ||
| expect(screen.getByText('Created Date')).toBeInTheDocument(); | ||
| expect(screen.getByText('Features')).toBeInTheDocument(); | ||
| expect(screen.getByText('Price')).toBeInTheDocument(); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Assert accessible wiring, not just visible text.
These tests still pass if labels are not programmatically associated or if the custom triggers are not keyboard reachable. Adding assertions around label lookup and keyboard interaction here would catch the accessibility regressions in the current implementation. Based on learnings: Ensure components are accessible by default and follow accessibility best practices.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/registry/templates/pages/advanced-search-form/advanced-search-form.test.tsx`
around lines 758 - 784, The current tests only assert visible text; update the
Accessibility tests to assert programmatic associations and keyboard
reachability: for form labels replace getByText checks with label-based queries
(getByLabelText / getByRole('combobox'|'textbox' etc.) against
AdvancedSearchForm fields to ensure labels are associated via
htmlFor/aria-labelledby, and add keyboard interaction assertions using
userEvent.tab and userEvent.keyboard (or fireEvent.keyDown) to verify custom
triggers in AdvancedSearchForm.Filters are focusable and operable via keyboard
(Enter/Space) and that focus moves as expected. Ensure you reference
AdvancedSearchForm and AdvancedSearchForm.Filters in the updated tests and
assert aria attributes (aria-label/aria-labelledby/role) and focus state after
keyboard events.
Comment on lines
+330
to
+354
| <Typography | ||
| variant="label" | ||
| weight="medium" | ||
| as="label" | ||
| color={theme === "dark" ? "secondary" : "default"} | ||
| className={isMobile ? "text-xs" : ""} | ||
| > | ||
| {filter.label} | ||
| {filter.required && <span className="text-red-500 ml-1">*</span>} | ||
| </Typography> | ||
| <input | ||
| id={filter.id} | ||
| type="text" | ||
| value={localValue} | ||
| onChange={handleChange} | ||
| placeholder={filter.placeholder} | ||
| className={cn( | ||
| "px-3 py-2 rounded-lg border focus:outline-none focus:ring-2 transition-all", | ||
| isMobile ? "text-sm px-2 py-1.5" : "text-base", | ||
| theme === "dark" | ||
| ? "bg-gray-800 border-gray-700 text-white focus:ring-blue-500 focus:border-blue-500" | ||
| : "bg-white border-gray-300 text-gray-900 focus:ring-blue-500 focus:border-blue-500", | ||
| className | ||
| )} | ||
| /> |
Contributor
There was a problem hiding this comment.
Associate these visible labels with the actual controls.
These filters currently render adjacent text, not programmatic labels. Screen readers will not announce a proper label, and the numeric inputs are indistinguishable because they rely on placeholders alone. Add explicit label/control wiring and separate Min/Max labels.
Also applies to: 382-410, 866-910
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/registry/templates/pages/advanced-search-form/index.tsx` around
lines 330 - 354, The visible text rendered by the Typography component is not
programmatically associated with the input (so screen readers won't announce
it); update the rendering so the label is a real HTML label element tied to the
input using htmlFor={filter.id} (or wrap the input in the label) for the text
produced by Typography (reference: Typography, filter.id, input, handleChange,
localValue), and for range/numeric filters (the other blocks around lines
382-410 and 866-910) render distinct "Min" and "Max" visible labels each bound
to their corresponding inputs (unique ids) and add appropriate aria-label or
aria-labelledby attributes where needed to ensure each control is properly
announced.
Comment on lines
+480
to
+496
| <div | ||
| onClick={(): void => setIsOpen(!isOpen)} | ||
| className={cn( | ||
| "px-3 py-2 rounded-lg border cursor-pointer focus:outline-none focus:ring-2 transition-all", | ||
| isMobile ? "text-sm px-2 py-1.5" : "text-base", | ||
| theme === "dark" | ||
| ? "bg-gray-800 border-gray-700 text-white focus:ring-blue-500" | ||
| : "bg-white border-gray-300 text-gray-900 focus:ring-blue-500", | ||
| selectedLabels.length > 0 && "ring-2 ring-blue-500" | ||
| )} | ||
| > | ||
| <Typography variant="body-small" color="inherit"> | ||
| {selectedLabels.length > 0 | ||
| ? `${selectedLabels.length} selected` | ||
| : "Select options..."} | ||
| </Typography> | ||
| </div> |
Contributor
There was a problem hiding this comment.
Use real buttons for the custom filter triggers.
These clickable <div>s are not tabbable, not keyboard-activatable, and do not expose expanded state. That blocks keyboard-only users from opening the multi-select and date picker.
Also applies to: 691-705
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/registry/templates/pages/advanced-search-form/index.tsx` around
lines 480 - 496, Replace the clickable div used as the custom filter trigger
with a real <button> element so it is keyboard-focusable and operable; update
the element around the selectedLabels display (the onClick={() =>
setIsOpen(!isOpen)} block) to use a button, add type="button", and include
aria-expanded={isOpen} plus any needed aria-controls; retain the same className
logic (cn(..., selectedLabels.length > 0 && "ring-2 ring-blue-500")) and
preserve Typography children; apply the same change to the other trigger
instance mentioned (the block around lines 691-705) so both multi-select and
date-picker triggers are accessible.
Comment on lines
+826
to
+857
| const [localMin, setLocalMin] = React.useState(value.min?.toString() || ""); | ||
| const [localMax, setLocalMax] = React.useState(value.max?.toString() || ""); | ||
| const debounceTimer = React.useRef<NodeJS.Timeout | undefined>(undefined); | ||
|
|
||
| React.useEffect(() => { | ||
| if (autoApply && debounceMs > 0) { | ||
| if (debounceTimer.current) clearTimeout(debounceTimer.current); | ||
| debounceTimer.current = setTimeout(() => { | ||
| onChange({ | ||
| min: localMin ? Number(localMin) : undefined, | ||
| max: localMax ? Number(localMax) : undefined, | ||
| }); | ||
| }, debounceMs); | ||
| } | ||
| }, [localMin, localMax, autoApply, debounceMs, onChange]); | ||
|
|
||
| React.useEffect(() => { | ||
| return (): void => { | ||
| if (debounceTimer.current) clearTimeout(debounceTimer.current); | ||
| }; | ||
| }, []); | ||
|
|
||
| const handleChange = (field: "min" | "max", val: string): void => { | ||
| if (field === "min") setLocalMin(val); | ||
| else setLocalMax(val); | ||
|
|
||
| if (!autoApply) { | ||
| onChange({ | ||
| min: field === "min" && val ? Number(val) : value.min, | ||
| max: field === "max" && val ? Number(val) : value.max, | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
Sync numeric inputs when value changes externally.
localMin and localMax are initialized from value once, but never updated afterward. Resetting filters or applying a saved search leaves stale numbers in the UI even though activeFilters changed.
Suggested fix
export const SearchNumericRangeFilter: React.FC<SearchNumericRangeFilterProps> = ({
filter,
value = {},
onChange,
className,
}) => {
const { theme, autoApply, debounceMs, isMobile } = useSearch();
const [localMin, setLocalMin] = React.useState(value.min?.toString() || "");
const [localMax, setLocalMax] = React.useState(value.max?.toString() || "");
const debounceTimer = React.useRef<NodeJS.Timeout | undefined>(undefined);
+
+ React.useEffect(() => {
+ setLocalMin(value.min?.toString() || "");
+ setLocalMax(value.max?.toString() || "");
+ }, [value.min, value.max]);
React.useEffect(() => {
if (autoApply && debounceMs > 0) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const [localMin, setLocalMin] = React.useState(value.min?.toString() || ""); | |
| const [localMax, setLocalMax] = React.useState(value.max?.toString() || ""); | |
| const debounceTimer = React.useRef<NodeJS.Timeout | undefined>(undefined); | |
| React.useEffect(() => { | |
| if (autoApply && debounceMs > 0) { | |
| if (debounceTimer.current) clearTimeout(debounceTimer.current); | |
| debounceTimer.current = setTimeout(() => { | |
| onChange({ | |
| min: localMin ? Number(localMin) : undefined, | |
| max: localMax ? Number(localMax) : undefined, | |
| }); | |
| }, debounceMs); | |
| } | |
| }, [localMin, localMax, autoApply, debounceMs, onChange]); | |
| React.useEffect(() => { | |
| return (): void => { | |
| if (debounceTimer.current) clearTimeout(debounceTimer.current); | |
| }; | |
| }, []); | |
| const handleChange = (field: "min" | "max", val: string): void => { | |
| if (field === "min") setLocalMin(val); | |
| else setLocalMax(val); | |
| if (!autoApply) { | |
| onChange({ | |
| min: field === "min" && val ? Number(val) : value.min, | |
| max: field === "max" && val ? Number(val) : value.max, | |
| }); | |
| } | |
| const [localMin, setLocalMin] = React.useState(value.min?.toString() || ""); | |
| const [localMax, setLocalMax] = React.useState(value.max?.toString() || ""); | |
| const debounceTimer = React.useRef<NodeJS.Timeout | undefined>(undefined); | |
| React.useEffect(() => { | |
| setLocalMin(value.min?.toString() || ""); | |
| setLocalMax(value.max?.toString() || ""); | |
| }, [value.min, value.max]); | |
| React.useEffect(() => { | |
| if (autoApply && debounceMs > 0) { | |
| if (debounceTimer.current) clearTimeout(debounceTimer.current); | |
| debounceTimer.current = setTimeout(() => { | |
| onChange({ | |
| min: localMin ? Number(localMin) : undefined, | |
| max: localMax ? Number(localMax) : undefined, | |
| }); | |
| }, debounceMs); | |
| } | |
| }, [localMin, localMax, autoApply, debounceMs, onChange]); | |
| React.useEffect(() => { | |
| return (): void => { | |
| if (debounceTimer.current) clearTimeout(debounceTimer.current); | |
| }; | |
| }, []); | |
| const handleChange = (field: "min" | "max", val: string): void => { | |
| if (field === "min") setLocalMin(val); | |
| else setLocalMax(val); | |
| if (!autoApply) { | |
| onChange({ | |
| min: field === "min" && val ? Number(val) : value.min, | |
| max: field === "max" && val ? Number(val) : value.max, | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/registry/templates/pages/advanced-search-form/index.tsx` around
lines 826 - 857, The local state variables localMin and localMax are only
initialized from value and never updated when value changes externally; add a
React.useEffect that watches value.min and value.max (and possibly
activeFilters) and calls setLocalMin(value.min?.toString() || "") and
setLocalMax(value.max?.toString() || "") to keep the inputs in sync, while
preserving the existing debounceTimer/useEffect and handleChange/onChange
behavior so external resets or saved searches update the displayed inputs
immediately.
Comment on lines
+1288
to
+1297
| const { theme, facetedCategories: contextCategories, activeFilters, setActiveFilters, onSearch, autoApply, isMobile } = useSearch(); | ||
| const categories = propCategories || contextCategories || []; | ||
|
|
||
| const handleSelect = (categoryId: string, value: string): void => { | ||
| if (onSelect) onSelect(categoryId, value); | ||
| else { | ||
| const newFilters = { ...activeFilters, [categoryId]: [value] }; | ||
| setActiveFilters(newFilters); | ||
| if (autoApply && onSearch) onSearch(newFilters); | ||
| } |
Contributor
There was a problem hiding this comment.
Preserve the target filter’s value shape when a facet is selected.
This always writes [value]. That works for multi-select/checkbox filters, but it breaks scalar filters like select because activeFilters[categoryId] becomes the wrong type.
Suggested fix
export const SearchFacetedHints: React.FC<SearchFacetedHintsProps> = ({
categories: propCategories,
onSelect,
className,
}) => {
- const { theme, facetedCategories: contextCategories, activeFilters, setActiveFilters, onSearch, autoApply, isMobile } = useSearch();
+ const { theme, facetedCategories: contextCategories, filters, activeFilters, setActiveFilters, onSearch, autoApply, isMobile } = useSearch();
const categories = propCategories || contextCategories || [];
const handleSelect = (categoryId: string, value: string): void => {
if (onSelect) onSelect(categoryId, value);
else {
- const newFilters = { ...activeFilters, [categoryId]: [value] };
+ const filter = filters.find((item) => item.id === categoryId);
+ const nextValue =
+ filter?.type === "multi-select" || filter?.type === "checkbox"
+ ? [value]
+ : value;
+ const newFilters = { ...activeFilters, [categoryId]: nextValue };
setActiveFilters(newFilters);
if (autoApply && onSearch) onSearch(newFilters);
}
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/registry/templates/pages/advanced-search-form/index.tsx` around
lines 1288 - 1297, handleSelect currently always writes the selected value as an
array ([value]) which breaks scalar filters; update handleSelect to inspect the
existing activeFilters[categoryId] (use Array.isArray or typeof) and preserve
its shape when setting the new value (i.e., set an array if the existing filter
is an array, otherwise set a scalar), then call setActiveFilters and
autoApply/onSearch as before; reference handleSelect, activeFilters,
setActiveFilters, autoApply, and onSearch to locate the change.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (3)
packages/registry/templates/pages/advanced-search-form/index.tsx (1)
1307-1317:⚠️ Potential issue | 🟠 Major
onSelectis declared but never used.
SearchFacetedHintsPropsexposes an override hook, but this component never destructures or calls it. Consumers cannot intercept facet clicks or keep their own filter state in sync.💡 Suggested fix
export const SearchFacetedHints: React.FC<SearchFacetedHintsProps> = ({ categories: propCategories, + onSelect, className, }) => { const { theme, facetedCategories: contextCategories, activeFilters, setActiveFilters, isMobile } = useSearch(); const categories = propCategories || contextCategories || []; const handleSelect = (categoryId: string, value: string): void => { + if (onSelect) { + onSelect(categoryId, value); + return; + } const newFilters = { ...activeFilters, [categoryId]: [value] }; setActiveFilters(newFilters); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/registry/templates/pages/advanced-search-form/index.tsx` around lines 1307 - 1317, The component SearchFacetedHints ignores the optional onSelect hook from SearchFacetedHintsProps; update the component to destructure onSelect from props and call it inside handleSelect (after computing newFilters or before setting state) so consumers can intercept clicks and sync external filter state, while still calling setActiveFilters(newFilters) when onSelect is not provided or as well for internal state updates; reference the prop name onSelect, the component SearchFacetedHints, the handler handleSelect, and the state updater setActiveFilters to locate and modify the code.apps/docs/src/components/Demo/AdvancedSearchFormDemo.tsx (2)
1040-1044:⚠️ Potential issue | 🟠 MajorKeep facet selections in the same shape as the underlying filter.
roleanddepartmentare single-select filters, but this handler always stores[value]. That leaves the controlled selects receiving an array while the rest of the demo treats them as scalars, so the facet chips and form state can drift apart.Suggested fix
<SearchFacetedHints categories={facetedCategories} onSelect={(categoryId, value): void => { - handleFilterChange(categoryId, [value]); + handleFilterChange(categoryId, categoryId === "status" ? [value] : value); }} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/docs/src/components/Demo/AdvancedSearchFormDemo.tsx` around lines 1040 - 1044, The onSelect handler for SearchFacetedHints always wraps the picked value in an array which breaks single-select filters (e.g., role, department); update the onSelect callback to consult the category metadata (from facetedCategories) or your filter schema to detect whether the category is single-select and call handleFilterChange(categoryId, value) for single-selects and handleFilterChange(categoryId, [value]) for multi-selects so the shape matches the underlying filter state; reference SearchFacetedHints, facetedCategories, onSelect and handleFilterChange to locate and change the handler.
485-576:⚠️ Potential issue | 🟠 MajorGenerated code tab still doesn't compile as-is.
For a docs example, the generated React snippet needs to be self-contained. It still uses
useState,mockUsers,roleOptions,departmentOptions, andstatusOptionswithout importing/declaring them, and Line 556 only clearsactiveFilters, sofilteredResultsstays stale after Reset. The code sample should be copy-pasteable and mirror the live demo behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/docs/src/components/Demo/AdvancedSearchFormDemo.tsx` around lines 485 - 576, The generated code string in generateCode is not self-contained: it references useState, mockUsers, roleOptions, departmentOptions, and statusOptions without declaring/importing them and the Reset button only clears activeFilters leaving filteredResults stale; update the string returned by generateCode to include "import React, { useState } from 'react';" (or include useState in the snippet), add a local mockUsers array and roleOptions/departmentOptions/statusOptions definitions, and change the SearchActions onReset handler to both clear filters and reset results (e.g., setActiveFilters({}) and setFilteredResults(mockUsers)) so the copy-paste example compiles and mirrors the live demo behavior while keeping the existing function name generateCode and the AdvancedSearchForm demo structure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/docs/src/components/Demo/AdvancedSearchFormDemo.tsx`:
- Around line 421-427: The handleFilterChange setter currently always preserves
the filter key even when the new value is "empty", causing stale empty values in
activeFilters; update handleFilterChange to detect empty values (e.g., value ===
"" || value == null || (Array.isArray(value) && value.length === 0) || (typeof
value === "object" && !Array.isArray(value) && Object.keys(value).length === 0))
and, when empty, remove the filterId from the map (create a shallow copy of
prev, delete the key, and return it); otherwise set prev[filterId] = value as
before—modify the function named handleFilterChange to implement this behavior
using setActiveFilters.
- Around line 67-80: The statusOptions and departmentOptions constants contain
hard-coded count fields that are out-of-sync with the actual dataset; update the
AdvancedSearchFormDemo to compute facet counts dynamically from the current
result set (e.g., derive counts from mockUsers or the active search results used
by the component) and build the option arrays (statusOptions, departmentOptions)
at render/update time instead of using static numbers so SearchFacetedHints
receives accurate counts; locate where statusOptions/departmentOptions are
defined and where SearchFacetedHints is consumed (also update the similar block
referenced around the 92-108 region) and replace the static count values with
computed counts keyed by option.value.
- Around line 833-873: The menu currently only paints a visual "focused" item
(focusedIndex) without moving real DOM focus, leaving the trigger focused;
update the export menu behavior so keyboard/AT users get real focus: when
showExportMenu becomes true, programmatically focus the first menu item (use
menuRef to query children or add refs per item), implement a roving tabIndex
pattern by setting tabIndex=0 only on the active item and tabIndex=-1 on others
(update setFocusedIndex usage to also update tabIndex), handle ArrowUp/ArrowDown
to move the active tabIndex and call element.focus(), use aria-activedescendant
if you prefer an active-id approach, call handleExport and close with
setShowExportMenu(false) on Enter, and when closing restore focus to the trigger
element (store a triggerRef before opening) so focus is returned for keyboard
users; adjust event cleanup and dependency usage accordingly (menuRef,
focusedIndex, setFocusedIndex, showExportMenu, handleExport, setShowExportMenu).
In
`@apps/docs/versioned_docs/version-1.0.3/templates/pages/forms/advanced-search-form.mdx`:
- Around line 1655-1705: The App component has a dead code path and a typing
issue: handleSearch filters on activeFilters.name but no SearchTextFilter is
rendered and useState is untyped (useState({})) causing TypeScript errors; fix
by declaring a typed filter state (e.g., interface Filters { name?: string;
role?: string; department?: string; status?: string[] } and using const
[activeFilters, setActiveFilters] = useState<Filters>({})) and add a
SearchTextFilter control (matching the pattern of
SearchSelectFilter/SearchMultiSelectFilter) that binds value={activeFilters.name
|| ""} and onChange={(val) => setActiveFilters({ ...activeFilters, name: val })}
so handleSearch's name branch is reachable and typesafe; alternatively remove
the name branch from handleSearch if you prefer not to show the text input.
In `@packages/registry/templates/pages/advanced-search-form/index.tsx`:
- Around line 1108-1134: The Save and Export buttons are rendered even when
their handlers are no-ops; update the component to conditionally render the Save
button only when either onSave or contextOnSave is present and the Export button
only when onExport or contextOnExport is present (leave handleSave,
handleExport, EXPORT_FORMATS, and their prompt/validation logic unchanged);
locate the UI where those buttons are rendered and wrap each button in a check
against the corresponding handlers (onSave || contextOnSave for Save, onExport
|| contextOnExport for Export) so dead controls are not shown.
- Around line 335-348: The change handlers (e.g., handleChange and the numeric
input handler) currently only call onChange when autoApply is true, so edits are
never committed for manual-apply flows; keep setLocalValue(newValue) in those
handlers but do not rely on them to commit state—remove the onChange gating or
only call onChange in the autoApply branch, and add a commit in the manual-apply
action (the Apply/Save/Export handler, e.g., handleApply/onApply) so it calls
onChange with the current localValue (and parsed numeric values) to update
activeFilters; update both the text handler block shown and the corresponding
numeric handlers (855-882 region) to follow this pattern.
---
Duplicate comments:
In `@apps/docs/src/components/Demo/AdvancedSearchFormDemo.tsx`:
- Around line 1040-1044: The onSelect handler for SearchFacetedHints always
wraps the picked value in an array which breaks single-select filters (e.g.,
role, department); update the onSelect callback to consult the category metadata
(from facetedCategories) or your filter schema to detect whether the category is
single-select and call handleFilterChange(categoryId, value) for single-selects
and handleFilterChange(categoryId, [value]) for multi-selects so the shape
matches the underlying filter state; reference SearchFacetedHints,
facetedCategories, onSelect and handleFilterChange to locate and change the
handler.
- Around line 485-576: The generated code string in generateCode is not
self-contained: it references useState, mockUsers, roleOptions,
departmentOptions, and statusOptions without declaring/importing them and the
Reset button only clears activeFilters leaving filteredResults stale; update the
string returned by generateCode to include "import React, { useState } from
'react';" (or include useState in the snippet), add a local mockUsers array and
roleOptions/departmentOptions/statusOptions definitions, and change the
SearchActions onReset handler to both clear filters and reset results (e.g.,
setActiveFilters({}) and setFilteredResults(mockUsers)) so the copy-paste
example compiles and mirrors the live demo behavior while keeping the existing
function name generateCode and the AdvancedSearchForm demo structure.
In `@packages/registry/templates/pages/advanced-search-form/index.tsx`:
- Around line 1307-1317: The component SearchFacetedHints ignores the optional
onSelect hook from SearchFacetedHintsProps; update the component to destructure
onSelect from props and call it inside handleSelect (after computing newFilters
or before setting state) so consumers can intercept clicks and sync external
filter state, while still calling setActiveFilters(newFilters) when onSelect is
not provided or as well for internal state updates; reference the prop name
onSelect, the component SearchFacetedHints, the handler handleSelect, and the
state updater setActiveFilters to locate and modify the code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 96f998c5-27f0-482b-8345-0a980113b3a4
📒 Files selected for processing (7)
apps/docs/src/components/Demo/AdvancedSearchFormDemo.tsxapps/docs/src/components/UI/advanced-search-form/index.tsxapps/docs/versioned_docs/version-1.0.3/templates/pages/forms/advanced-search-form.mdxapps/storybook/src/templates/pages/form-pages/advanced-search-form/advanced-search-form.stories.tsxapps/storybook/src/templates/pages/form-pages/advanced-search-form/index.tsxpackages/registry/registry.jsonpackages/registry/templates/pages/advanced-search-form/index.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
packages/registry/registry.json
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Register components in
packages/registry/components/directory by creating a component subdirectory and adding an entry topackages/registry/registry.json
Files:
packages/registry/registry.json
🧠 Learnings (7)
📚 Learning: 2026-03-18T09:43:15.140Z
Learnt from: CR
Repo: mindfiredigital/ignix-ui PR: 0
File: README.md:0-0
Timestamp: 2026-03-18T09:43:15.140Z
Learning: Applies to registry.json : Update registry.json when adding new components to the codebase
Applied to files:
packages/registry/registry.json
📚 Learning: 2026-03-18T09:43:08.260Z
Learnt from: CR
Repo: mindfiredigital/ignix-ui PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2026-03-18T09:43:08.260Z
Learning: Applies to packages/registry/registry.json : Register components in `packages/registry/components/` directory by creating a component subdirectory and adding an entry to `packages/registry/registry.json`
Applied to files:
packages/registry/registry.json
📚 Learning: 2026-03-18T09:43:15.140Z
Learnt from: CR
Repo: mindfiredigital/ignix-ui PR: 0
File: README.md:0-0
Timestamp: 2026-03-18T09:43:15.140Z
Learning: Applies to src/**/*.tsx : Ensure components are accessible by default and follow accessibility best practices
Applied to files:
apps/docs/src/components/Demo/AdvancedSearchFormDemo.tsxpackages/registry/templates/pages/advanced-search-form/index.tsx
📚 Learning: 2026-03-18T09:43:08.260Z
Learnt from: CR
Repo: mindfiredigital/ignix-ui PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2026-03-18T09:43:08.260Z
Learning: Applies to apps/docs/docs/components/**/*.mdx : Add MDX documentation files for components in `apps/docs/docs/components/` and link them in `apps/docs/sidebars.ts`
Applied to files:
apps/docs/versioned_docs/version-1.0.3/templates/pages/forms/advanced-search-form.mdx
📚 Learning: 2026-03-31T05:22:49.923Z
Learnt from: PoonamKPatil
Repo: mindfiredigital/ignix-ui PR: 723
File: apps/docs/versioned_docs/version-1.0.3/templates/pages/forms/contact-form.mdx:29-31
Timestamp: 2026-03-31T05:22:49.923Z
Learning: In mindfiredigital/ignix-ui documentation under apps/docs/versioned_docs, the CLI command `ignix add component <name>` is an intentional unified command that imports both components and templates. When this command appears in template-related docs/examples (e.g., template documentation snippets), do not flag it as incorrect or misleading; it is the expected usage.
Applied to files:
apps/docs/versioned_docs/version-1.0.3/templates/pages/forms/advanced-search-form.mdx
📚 Learning: 2026-03-18T09:43:08.260Z
Learnt from: CR
Repo: mindfiredigital/ignix-ui PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2026-03-18T09:43:08.260Z
Learning: Applies to apps/storybook/src/components/**/*.stories.{tsx,ts} : Create a Story File named `[component-name].stories.tsx` in the same directory as the component file with argTypes, controls, and example stories
Applied to files:
apps/storybook/src/templates/pages/form-pages/advanced-search-form/advanced-search-form.stories.tsx
📚 Learning: 2026-03-18T09:43:08.260Z
Learnt from: CR
Repo: mindfiredigital/ignix-ui PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2026-03-18T09:43:08.260Z
Learning: Applies to apps/storybook/src/components/**/*.{tsx,ts,jsx,js} : Create Component Files in `apps/storybook/src/components/` directory with a component subdirectory for each component
Applied to files:
apps/storybook/src/templates/pages/form-pages/advanced-search-form/advanced-search-form.stories.tsx
🔇 Additional comments (3)
packages/registry/registry.json (1)
25-43: Registry wiring looks good.The new template entry includes the main template path and the dependent building blocks, so it should integrate cleanly with the registry flow.
Based on learnings: "Update registry.json when adding new components to the codebase"
apps/storybook/src/templates/pages/form-pages/advanced-search-form/advanced-search-form.stories.tsx (1)
61-125: Good Storybook controls baseline.The meta config exposes the important root props and gives the new compound API a solid interactive surface for docs and regression coverage.
apps/docs/src/components/Demo/AdvancedSearchFormDemo.tsx (1)
210-218: Nice defensive escaping before building the HTML export.Escaping header and cell values here prevents exported user data from being interpreted as markup by HTML-capable spreadsheet viewers.
Comment on lines
+67
to
80
| const statusOptions = [ | ||
| { value: "active", label: "Active", count: 12 }, | ||
| { value: "inactive", label: "Inactive", count: 4 }, | ||
| { value: "pending", label: "Pending", count: 3 }, | ||
| { value: "suspended", label: "Suspended", count: 1 }, | ||
| ]; | ||
|
|
||
| const debounceOptions = [ | ||
| { value: '0', label: 'No Debounce' }, | ||
| { value: '300', label: '300ms' }, | ||
| { value: '500', label: '500ms' }, | ||
| { value: '1000', label: '1000ms' }, | ||
| const departmentOptions = [ | ||
| { value: "engineering", label: "Engineering", count: 7 }, | ||
| { value: "sales", label: "Sales", count: 5 }, | ||
| { value: "marketing", label: "Marketing", count: 4 }, | ||
| { value: "hr", label: "Human Resources", count: 3 }, | ||
| { value: "finance", label: "Finance", count: 4 }, | ||
| ]; |
Contributor
There was a problem hiding this comment.
Facet counts are already out of sync with the demo data.
These counts do not match mockUsers even on first render (status.active is 12 here but the dataset has 15; department.finance is 4 here but the dataset has 2). Because SearchFacetedHints consumes these constants directly, the hints are inaccurate and can never satisfy Issue #645's dynamic facet-count requirement. Derive the counts from the current result set instead of hard-coding them.
Also applies to: 92-108
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/docs/src/components/Demo/AdvancedSearchFormDemo.tsx` around lines 67 -
80, The statusOptions and departmentOptions constants contain hard-coded count
fields that are out-of-sync with the actual dataset; update the
AdvancedSearchFormDemo to compute facet counts dynamically from the current
result set (e.g., derive counts from mockUsers or the active search results used
by the component) and build the option arrays (statusOptions, departmentOptions)
at render/update time instead of using static numbers so SearchFacetedHints
receives accurate counts; locate where statusOptions/departmentOptions are
defined and where SearchFacetedHints is consumed (also update the similar block
referenced around the 92-108 region) and replace the static count values with
computed counts keyed by option.value.
Comment on lines
+421
to
427
| // Handle filter changes | ||
| const handleFilterChange = (filterId: string, value: any): void => { | ||
| setActiveFilters(prev => ({ | ||
| ...prev, | ||
| [option]: !prev[option as keyof typeof prev] | ||
| [filterId]: value | ||
| })); | ||
| }; |
Contributor
There was a problem hiding this comment.
Drop cleared filters from activeFilters.
This setter always preserves the key, so clearing a control leaves values like "", [], or {} in state. Downstream checks such as Object.keys(activeFilters).length > 0 then treat the form as still filtered and keep showing the "Clear All Filters" affordance. Remove empty values when updating the map.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/docs/src/components/Demo/AdvancedSearchFormDemo.tsx` around lines 421 -
427, The handleFilterChange setter currently always preserves the filter key
even when the new value is "empty", causing stale empty values in activeFilters;
update handleFilterChange to detect empty values (e.g., value === "" || value ==
null || (Array.isArray(value) && value.length === 0) || (typeof value ===
"object" && !Array.isArray(value) && Object.keys(value).length === 0)) and, when
empty, remove the filterId from the map (create a shallow copy of prev, delete
the key, and return it); otherwise set prev[filterId] = value as before—modify
the function named handleFilterChange to implement this behavior using
setActiveFilters.
Comment on lines
+833
to
+873
| const menuRef = React.useRef<HTMLDivElement | null>(null); | ||
| const [focusedIndex, setFocusedIndex] = React.useState<number>(-1); | ||
| React.useEffect(() => { | ||
| if (!showExportMenu) return; | ||
|
|
||
| const handleClickOutside = (e: MouseEvent) => { | ||
| if (menuRef.current && !menuRef.current.contains(e.target as Node)) { | ||
| setShowExportMenu(false); | ||
| } | ||
| }; | ||
|
|
||
| const handleKeyDown = (e: KeyboardEvent) => { | ||
| if (e.key === 'Escape') { | ||
| setShowExportMenu(false); | ||
| } | ||
|
|
||
| if (e.key === 'ArrowDown') { | ||
| e.preventDefault(); | ||
| setFocusedIndex((prev) => (prev + 1) % 3); | ||
| } | ||
|
|
||
| if (e.key === 'ArrowUp') { | ||
| e.preventDefault(); | ||
| setFocusedIndex((prev) => (prev - 1 + 3) % 3); | ||
| } | ||
|
|
||
| if (e.key === 'Enter' && focusedIndex !== -1) { | ||
| const options = ['csv', 'excel', 'json']; | ||
| handleExport(options[focusedIndex]); | ||
| setShowExportMenu(false); | ||
| } | ||
| }; | ||
|
|
||
| document.addEventListener('mousedown', handleClickOutside); | ||
| document.addEventListener('keydown', handleKeyDown); | ||
|
|
||
| return () => { | ||
| document.removeEventListener('mousedown', handleClickOutside); | ||
| document.removeEventListener('keydown', handleKeyDown); | ||
| }; | ||
| }, [showExportMenu, focusedIndex]); |
Contributor
There was a problem hiding this comment.
The export menu's keyboard navigation never moves real focus.
This custom menu only paints an item as “focused”; it never implements the actual menu focus pattern. focusedIndex changes background color, all items stay tabIndex={0}, and focus remains on the trigger, so keyboard and assistive-technology users do not get a usable active item. Use roving tabIndex or aria-activedescendant, move focus into the first menuitem on open, and restore focus to the trigger on close. Based on learnings: Ensure components are accessible by default and follow accessibility best practices
Also applies to: 986-1025
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/docs/src/components/Demo/AdvancedSearchFormDemo.tsx` around lines 833 -
873, The menu currently only paints a visual "focused" item (focusedIndex)
without moving real DOM focus, leaving the trigger focused; update the export
menu behavior so keyboard/AT users get real focus: when showExportMenu becomes
true, programmatically focus the first menu item (use menuRef to query children
or add refs per item), implement a roving tabIndex pattern by setting tabIndex=0
only on the active item and tabIndex=-1 on others (update setFocusedIndex usage
to also update tabIndex), handle ArrowUp/ArrowDown to move the active tabIndex
and call element.focus(), use aria-activedescendant if you prefer an active-id
approach, call handleExport and close with setShowExportMenu(false) on Enter,
and when closing restore focus to the trigger element (store a triggerRef before
opening) so focus is returned for keyboard users; adjust event cleanup and
dependency usage accordingly (menuRef, focusedIndex, setFocusedIndex,
showExportMenu, handleExport, setShowExportMenu).
Comment on lines
+1655
to
+1705
| function App() { | ||
| const [activeFilters, setActiveFilters] = useState({}); | ||
| const [filteredResults, setFilteredResults] = useState(mockUsers); | ||
|
|
||
| const handleSearch = () => { | ||
| let results = [...mockUsers]; | ||
|
|
||
| if (activeFilters.name) { | ||
| results = results.filter(u => | ||
| u.name.toLowerCase().includes(activeFilters.name.toLowerCase()) | ||
| ); | ||
| } | ||
| if (activeFilters.role) { | ||
| results = results.filter(u => | ||
| u.role.toLowerCase() === activeFilters.role.toLowerCase() | ||
| ); | ||
| } | ||
| if (activeFilters.department) { | ||
| results = results.filter(u => | ||
| u.department.toLowerCase() === activeFilters.department.toLowerCase() | ||
| ); | ||
| } | ||
| if (activeFilters.status && Array.isArray(activeFilters.status)) { | ||
| results = results.filter(u => | ||
| activeFilters.status.includes(u.status.toLowerCase()) | ||
| ); | ||
| } | ||
|
|
||
| setFilteredResults(results); | ||
| }; | ||
|
|
||
| return ( | ||
| <AdvancedSearchForm variant="dark" layout="stacked"> | ||
| <div className="space-y-6"> | ||
| <SearchFilters> | ||
| <div className="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 gap-4"> | ||
| <SearchSelectFilter | ||
| filter={{ id: "role", label: "Role", type: "select", options: roleOptions }} | ||
| value={activeFilters.role || ""} | ||
| onChange={(val) => setActiveFilters({ ...activeFilters, role: val })} | ||
| /> | ||
| <SearchSelectFilter | ||
| filter={{ id: "department", label: "Department", type: "select", options: departmentOptions }} | ||
| value={activeFilters.department || ""} | ||
| onChange={(val) => setActiveFilters({ ...activeFilters, department: val })} | ||
| /> | ||
| <SearchMultiSelectFilter | ||
| filter={{ id: "status", label: "Status", type: "multi-select", options: statusOptions }} | ||
| value={activeFilters.status || []} | ||
| onChange={(val) => setActiveFilters({ ...activeFilters, status: val })} | ||
| /> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the exact file
find . -name "advanced-search-form.mdx" -type f | head -5Repository: mindfiredigital/ignix-ui
Length of output: 155
🏁 Script executed:
# Get line count to understand file size
wc -l apps/docs/versioned_docs/version-1.0.3/templates/pages/forms/advanced-search-form.mdxRepository: mindfiredigital/ignix-ui
Length of output: 158
🏁 Script executed:
# Read the specific lines mentioned in the review (1655-1705)
sed -n '1655,1705p' apps/docs/versioned_docs/version-1.0.3/templates/pages/forms/advanced-search-form.mdxRepository: mindfiredigital/ignix-ui
Length of output: 2046
🏁 Script executed:
# Search for SearchTextFilter in this file to verify if it's used elsewhere
grep -n "SearchTextFilter" apps/docs/versioned_docs/version-1.0.3/templates/pages/forms/advanced-search-form.mdxRepository: mindfiredigital/ignix-ui
Length of output: 608
🏁 Script executed:
# Check broader context around the handleSearch function to see full implementation
sed -n '1600,1750p' apps/docs/versioned_docs/version-1.0.3/templates/pages/forms/advanced-search-form.mdxRepository: mindfiredigital/ignix-ui
Length of output: 6697
🏁 Script executed:
# Check if SearchTextFilter appears in a continuation after line 1705
sed -n '1705,1750p' apps/docs/versioned_docs/version-1.0.3/templates/pages/forms/advanced-search-form.mdx | grep -A 10 -B 5 "SearchTextFilter"Repository: mindfiredigital/ignix-ui
Length of output: 50
🏁 Script executed:
# Verify TypeScript inference behavior with useState({})
# Check if there's any type definition or context wrapping this snippet
sed -n '1630,1665p' apps/docs/versioned_docs/version-1.0.3/templates/pages/forms/advanced-search-form.mdxRepository: mindfiredigital/ignix-ui
Length of output: 2504
Add missing SearchTextFilter and type activeFilters correctly.
The snippet filters on activeFilters.name but never renders a SearchTextFilter control—dead code path. Additionally, useState({}) infers the {} type, making property access like activeFilters.role a type error in real TypeScript. Explicitly type the state object and either add the text filter or remove the unused name branch.
Suggested fix
+type ActiveFilters = {
+ name?: string;
+ role?: string;
+ department?: string;
+ status?: string[];
+};
+
function App() {
- const [activeFilters, setActiveFilters] = useState({});
- const [filteredResults, setFilteredResults] = useState(mockUsers);
+ const [activeFilters, setActiveFilters] = useState<ActiveFilters>({});
+ const [filteredResults, setFilteredResults] = useState(mockUsers);Then add the text filter control:
<div className="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 gap-4">
+ <SearchTextFilter
+ filter={{ id: "name", label: "Name", type: "text" }}
+ value={activeFilters.name || ""}
+ onChange={(val) => setActiveFilters({ ...activeFilters, name: val })}
+ />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/docs/versioned_docs/version-1.0.3/templates/pages/forms/advanced-search-form.mdx`
around lines 1655 - 1705, The App component has a dead code path and a typing
issue: handleSearch filters on activeFilters.name but no SearchTextFilter is
rendered and useState is untyped (useState({})) causing TypeScript errors; fix
by declaring a typed filter state (e.g., interface Filters { name?: string;
role?: string; department?: string; status?: string[] } and using const
[activeFilters, setActiveFilters] = useState<Filters>({})) and add a
SearchTextFilter control (matching the pattern of
SearchSelectFilter/SearchMultiSelectFilter) that binds value={activeFilters.name
|| ""} and onChange={(val) => setActiveFilters({ ...activeFilters, name: val })}
so handleSearch's name branch is reachable and typesafe; alternatively remove
the name branch from handleSearch if you prefer not to show the text input.
Comment on lines
+335
to
+348
| const { theme, autoApply, isMobile } = useSearch(); | ||
| const [localValue, setLocalValue] = React.useState(value || ""); | ||
|
|
||
| React.useEffect(() => { | ||
| setLocalValue(value || ""); | ||
| }, [value]); | ||
|
|
||
| const handleChange = (e: React.ChangeEvent<HTMLInputElement>): void => { | ||
| const newValue = e.target.value; | ||
| setLocalValue(newValue); | ||
|
|
||
| if (autoApply) { | ||
| onChange(newValue); | ||
| } |
Contributor
There was a problem hiding this comment.
Manual apply never captures text or numeric edits.
Both handlers only call onChange inside the autoApply path. When autoApply={false}, the UI updates locally but activeFilters stays stale, so Apply/Save/Export use the previous values.
💡 Suggested fix
- const { theme, autoApply, isMobile } = useSearch();
+ const { theme, isMobile } = useSearch();
...
const handleChange = (e: React.ChangeEvent<HTMLInputElement>): void => {
const newValue = e.target.value;
setLocalValue(newValue);
-
- if (autoApply) {
- onChange(newValue);
- }
+ onChange(newValue);
};- const { theme, autoApply, isMobile } = useSearch();
+ const { theme, isMobile } = useSearch();
...
const handleChange = (field: "min" | "max", val: string): void => {
if (field === "min") setLocalMin(val);
else setLocalMax(val);
-
- if (autoApply) {
- onChange({
- min:
- field === "min"
- ? val
- ? Number(val)
- : undefined
- : localMin
- ? Number(localMin)
- : undefined,
- max:
- field === "max"
- ? val
- ? Number(val)
- : undefined
- : localMax
- ? Number(localMax)
- : undefined,
- });
- }
+
+ onChange({
+ min:
+ field === "min"
+ ? val
+ ? Number(val)
+ : undefined
+ : localMin
+ ? Number(localMin)
+ : undefined,
+ max:
+ field === "max"
+ ? val
+ ? Number(val)
+ : undefined
+ : localMax
+ ? Number(localMax)
+ : undefined,
+ });
};Also applies to: 855-882
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/registry/templates/pages/advanced-search-form/index.tsx` around
lines 335 - 348, The change handlers (e.g., handleChange and the numeric input
handler) currently only call onChange when autoApply is true, so edits are never
committed for manual-apply flows; keep setLocalValue(newValue) in those handlers
but do not rely on them to commit state—remove the onChange gating or only call
onChange in the autoApply branch, and add a commit in the manual-apply action
(the Apply/Save/Export handler, e.g., handleApply/onApply) so it calls onChange
with the current localValue (and parsed numeric values) to update activeFilters;
update both the text handler block shown and the corresponding numeric handlers
(855-882 region) to follow this pattern.
Comment on lines
+1108
to
+1134
| const handleSave = (): void => { | ||
| if (onSave) onSave(); | ||
| else if (contextOnSave) { | ||
| const name = prompt("Enter a name for this search:"); | ||
| if (name) { | ||
| contextOnSave({ | ||
| id: Date.now().toString(), | ||
| name, | ||
| filters: activeFilters, | ||
| createdAt: new Date(), | ||
| }); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| const EXPORT_FORMATS = ["csv", "excel", "json"] as const; | ||
| const handleExport = (): void => { | ||
| if (onExport) return onExport(); | ||
|
|
||
| if (!contextOnExport) return; | ||
|
|
||
| const formatInput = prompt("Export format (csv/excel/json):", "csv"); | ||
|
|
||
| if (formatInput && EXPORT_FORMATS.includes(formatInput as ExportFormat)) { | ||
| contextOnExport(activeFilters, formatInput as ExportFormat); | ||
| } | ||
| }; |
Contributor
There was a problem hiding this comment.
Don’t render Save/Export buttons that can only no-op.
handleSave and handleExport silently return when neither a local handler nor a context handler exists, but the buttons still render by default. That exposes dead controls in the common root usage path.
💡 Suggested fix
- {showSave && (
+ {showSave && (onSave || contextOnSave) && (
<button
onClick={handleSave}
...
- {showExport && (
+ {showExport && (onExport || contextOnExport) && (
<button
onClick={handleExport}Also applies to: 1174-1205
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/registry/templates/pages/advanced-search-form/index.tsx` around
lines 1108 - 1134, The Save and Export buttons are rendered even when their
handlers are no-ops; update the component to conditionally render the Save
button only when either onSave or contextOnSave is present and the Export button
only when onExport or contextOnExport is present (leave handleSave,
handleExport, EXPORT_FORMATS, and their prompt/validation logic unchanged);
locate the UI where those buttons are rendered and wrap each button in a check
against the corresponding handlers (onSave || contextOnSave for Save, onExport
|| contextOnExport for Export) so dead controls are not shown.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
packages/registry/templates/pages/advanced-search-form/index.tsx (6)
156-160:⚠️ Potential issue | 🟠 Major
SavedSearchesstill can’t delete through the root context.The root provides
savedSearchesvia context, but never exposesonDeleteSearch, andSearchSavedSearchesonly renders delete buttons whenonDeleteis passed directly.<AdvancedSearchForm onDeleteSearch={...}><AdvancedSearchForm.SavedSearches /></AdvancedSearchForm>still has no delete path.Also applies to: 1373-1392, 1429-1445, 1654-1675
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/registry/templates/pages/advanced-search-form/index.tsx` around lines 156 - 160, The root AdvancedSearchForm context does not expose an onDeleteSearch handler so AdvancedSearchForm.SavedSearches cannot delete when only onDeleteSearch is passed to the root; update the root context type to include onDeleteSearch, wire the AdvancedSearchForm prop onDeleteSearch into that context provider, and update the SavedSearches component to read onDeleteSearch from context (falling back to a direct onDelete prop) and render the delete button when a handler exists; specifically adjust the AdvancedSearchForm context provider, the AdvancedSearchForm prop list, and SavedSearches so the delete path uses the context onDeleteSearch handler.
348-361:⚠️ Potential issue | 🔴 CriticalManual apply still drops text and numeric edits.
Both filters only call
onChangeinside theautoApplybranch, soactiveFiltersstays stale in manual mode and Apply/Save/Export keep using the previous values.SearchNumericRangeFilteralso never rehydrateslocalMin/localMaxfromvalue, so Reset or saved-search apply can leave old numbers visible.Suggested fix
- const { theme, autoApply, isMobile } = useSearch(); + const { theme, isMobile } = useSearch(); ... const handleChange = (e: React.ChangeEvent<HTMLInputElement>): void => { const newValue = e.target.value; setLocalValue(newValue); - - if (autoApply) { - onChange(newValue); - } + onChange(newValue); };- const { theme, autoApply, isMobile } = useSearch(); + const { theme, isMobile } = useSearch(); const [localMin, setLocalMin] = React.useState(value.min?.toString() || ""); const [localMax, setLocalMax] = React.useState(value.max?.toString() || ""); + + React.useEffect(() => { + setLocalMin(value.min?.toString() || ""); + setLocalMax(value.max?.toString() || ""); + }, [value.min, value.max]); const handleChange = (field: "min" | "max", val: string): void => { if (field === "min") setLocalMin(val); else setLocalMax(val); - - if (autoApply) { - onChange({ - min: - field === "min" - ? val - ? Number(val) - : undefined - : localMin - ? Number(localMin) - : undefined, - max: - field === "max" - ? val - ? Number(val) - : undefined - : localMax - ? Number(localMax) - : undefined, - }); - } + + onChange({ + min: + field === "min" + ? val + ? Number(val) + : undefined + : localMin + ? Number(localMin) + : undefined, + max: + field === "max" + ? val + ? Number(val) + : undefined + : localMax + ? Number(localMax) + : undefined, + }); };Also applies to: 959-987
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/registry/templates/pages/advanced-search-form/index.tsx` around lines 348 - 361, The text filter's handleChange only updates localValue and calls onChange when autoApply is true, which leaves activeFilters stale in manual mode; update the Apply/Save/Export handlers to call the provided onChange with the current localValue when applying manually and ensure any manual change handlers don't swallow the intent to apply later. For the numeric filter (SearchNumericRangeFilter) add a useEffect that rehydrates localMin and localMax from the incoming value prop (similar to how localValue is synced from value) and ensure the Reset/Apply paths call onChange with the numeric value parsed from localMin/localMax so UI and saved-search apply/export use the latest numbers. Reference: useSearch, localValue, setLocalValue, handleChange, onChange, SearchNumericRangeFilter, localMin, localMax, and value.
1455-1458:⚠️ Potential issue | 🟠 Major
SearchFacetedHintsignoresonSelectand still forces array values.
onSelectis declared in the props interface but never destructured or called. The fallback also always writes[value], which breaks scalar filters likeselectand violates the documented filter shapes. Use the callback when provided; otherwise preserve the target filter’s scalar/array shape before updating context.Suggested fix
export const SearchFacetedHints: React.FC<SearchFacetedHintsProps> = ({ categories: propCategories, + onSelect, className, }) => { - const { theme, facetedCategories: contextCategories, activeFilters, setActiveFilters, isMobile } = useSearch(); + const { theme, facetedCategories: contextCategories, filters, activeFilters, setActiveFilters, isMobile } = useSearch(); const categories = propCategories || contextCategories || []; const handleSelect = (categoryId: string, value: string): void => { - const newFilters = { ...activeFilters, [categoryId]: [value] }; + if (onSelect) return onSelect(categoryId, value); + const filter = filters.find((item) => item.id === categoryId); + const nextValue = + filter?.type === "multi-select" || filter?.type === "checkbox" + ? [value] + : value; + const newFilters = { ...activeFilters, [categoryId]: nextValue }; setActiveFilters(newFilters); };Also applies to: 1475-1485
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/registry/templates/pages/advanced-search-form/index.tsx` around lines 1455 - 1458, SearchFacetedHints currently ignores the onSelect prop and always writes the selected value as an array (e.g. [value]), breaking scalar filters; update the SearchFacetedHints component to destructure and call the onSelect(categoryId, value) callback when provided, and otherwise update the filter in context while preserving the target filter’s original shape (keep scalar values scalar and arrays as arrays instead of always wrapping value in an array). Ensure the branch that previously used the unconditional [value] fallback is changed to detect the existing filter type and write a scalar or array accordingly when onSelect is absent.
369-392:⚠️ Potential issue | 🟠 MajorThese controls are still missing real labels, and the date trigger is not keyboard-operable.
The visible
Typographytext is rendered as a sibling, so the text/select/numeric controls are not programmatically labeled withouthtmlForand matching ids. The date picker trigger is still a clickable<div>, which cannot receive focus or expose expanded state. Please wire explicit labels, add distinct Min/Max labels, and use a real button for the custom date trigger.Based on learnings: Applies to src/**/*.tsx : Ensure components are accessible by default and follow accessibility best practices
Also applies to: 438-450, 746-801, 995-1039
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/registry/templates/pages/advanced-search-form/index.tsx` around lines 369 - 392, The controls lack programmatic labels and the date trigger is not keyboard-operable: update the label rendered by Typography to include htmlFor={filter.id} (so it pairs with the existing id on the input used by value={localValue} and onChange={handleChange}), give distinct visible labels for Min/Max where applicable (create separate labelled inputs using unique ids like `${filter.id}-min` and `${filter.id}-max`), and replace any custom date trigger div with a real <button> element (manage focus, keyboard interaction, and expose state with aria-expanded and aria-haspopup) so the date picker component can be operated via keyboard and is accessible. Ensure styling and theme logic (isMobile/theme) remain the same while wiring these accessibility attributes.
247-255:⚠️ Potential issue | 🟠 MajorKeep calendar-only dates out of UTC round-trips.
toISOString()andnew Date("YYYY-MM-DD")both normalize through UTC, so saved or preset dates can shift by one day for non-UTC users. Build and parse theYYYY-MM-DDstring in local time instead.Suggested fix
const formatDate = (date: Date): string => { - return date.toISOString().split('T')[0]; + const year = date.getFullYear(); + const month = String(date.getMonth() + 1).padStart(2, "0"); + const day = String(date.getDate()).padStart(2, "0"); + return `${year}-${month}-${day}`; }; const parseDate = (dateStr: string): Date | null => { if (!dateStr) return null; - const date = new Date(dateStr); - return isNaN(date.getTime()) ? null : date; + const [year, month, day] = dateStr.split("-").map(Number); + if (!year || !month || !day) return null; + const date = new Date(year, month - 1, day); + return isNaN(date.getTime()) ? null : date; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/registry/templates/pages/advanced-search-form/index.tsx` around lines 247 - 255, formatDate and parseDate currently round-trip via UTC (toISOString/new Date("YYYY-MM-DD")) causing off-by-one-day for non-UTC users; change formatDate to build the "YYYY-MM-DD" string from local date components (date.getFullYear(), date.getMonth()+1, date.getDate()) and change parseDate to avoid Date parsing from a string by splitting "YYYY-MM-DD" and constructing a local Date with new Date(year, month-1, day), returning null for invalid parts; update the functions named formatDate and parseDate accordingly to keep all dates in local time.
28-35:⚠️ Potential issue | 🟠 Major
defaultValueis still ignored on first render and reset.
BaseFilterConfig.defaultValueis public, but the root state only hydratesinitialFilters, and Reset still clears to{}. CLI-configured defaults never render initially and are lost again after Reset.Also applies to: 1242-1247, 1605-1633
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/registry/templates/pages/advanced-search-form/index.tsx` around lines 28 - 35, The form ignores BaseFilterConfig.defaultValue because the component only hydrates from initialFilters and Reset clears to an empty object; fix by merging each filter's BaseFilterConfig.defaultValue into the form's starting state and use that merged object as the Reset target. Locate where initialFilters is turned into root state (the form initialization in advanced-search-form/index.tsx) and update it to iterate BaseFilterConfig entries, coalescing defaultValue -> initialFilters (respecting types: string, string[], date-range, numeric-range) so CLI defaults are present on first render; then update the Reset handler to reset to that merged defaults object (not {}) so defaultValue survives resets. Ensure references: BaseFilterConfig.defaultValue, initialFilters, and the Reset handler in this file are updated.apps/docs/versioned_docs/version-1.0.3/templates/pages/forms/advanced-search-form.mdx (1)
1850-1900:⚠️ Potential issue | 🟠 MajorThe “Basic Usage” snippet still won’t type-check, and its
namebranch is dead.
useState({})in this TSX example infers{}, soactiveFilters.role,department,status, andnameare invalid property accesses for copy-paste users. The example also filters onactiveFilters.namewithout rendering aSearchTextFilter, so that branch can never run.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/docs/versioned_docs/version-1.0.3/templates/pages/forms/advanced-search-form.mdx` around lines 1850 - 1900, The App component’s activeFilters is initialized as useState({}) which types it as {} and makes property accesses (activeFilters.name/role/department/status) unsafe, plus the name filter branch in handleSearch is dead because no SearchTextFilter is rendered; fix by defining a proper filter type and initial state (e.g., an interface with name?: string, role?: string, department?: string, status?: string[] and useState<YourFilterType>({ name: "", role: "", department: "", status: [] })) or alternatively remove the name branch from handleSearch and its check, and if you intend to support name searching add a SearchTextFilter component wired to setActiveFilters so the name property is actually set; update usages of setActiveFilters accordingly to preserve types and array handling for status.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/docs/versioned_docs/version-1.0.3/templates/pages/forms/advanced-search-form.mdx`:
- Around line 29-35: The import currently pulls a repo-local helper
("../../../utils/cn") which won't resolve for external consumers; update the top
imports to either (a) replace the import with a public/common helper (for
example from a published package) or (b) inline a small local `cn` utility in
this file, then update references to `cn` used by components like the
class-variance-authority usage and any JSX className concatenations; ensure the
import line for `cn` is removed and references point to the new public helper or
the in-file `cn` function so the Manual tab snippet is copy-pasteable outside
the monorepo.
---
Duplicate comments:
In
`@apps/docs/versioned_docs/version-1.0.3/templates/pages/forms/advanced-search-form.mdx`:
- Around line 1850-1900: The App component’s activeFilters is initialized as
useState({}) which types it as {} and makes property accesses
(activeFilters.name/role/department/status) unsafe, plus the name filter branch
in handleSearch is dead because no SearchTextFilter is rendered; fix by defining
a proper filter type and initial state (e.g., an interface with name?: string,
role?: string, department?: string, status?: string[] and
useState<YourFilterType>({ name: "", role: "", department: "", status: [] })) or
alternatively remove the name branch from handleSearch and its check, and if you
intend to support name searching add a SearchTextFilter component wired to
setActiveFilters so the name property is actually set; update usages of
setActiveFilters accordingly to preserve types and array handling for status.
In `@packages/registry/templates/pages/advanced-search-form/index.tsx`:
- Around line 156-160: The root AdvancedSearchForm context does not expose an
onDeleteSearch handler so AdvancedSearchForm.SavedSearches cannot delete when
only onDeleteSearch is passed to the root; update the root context type to
include onDeleteSearch, wire the AdvancedSearchForm prop onDeleteSearch into
that context provider, and update the SavedSearches component to read
onDeleteSearch from context (falling back to a direct onDelete prop) and render
the delete button when a handler exists; specifically adjust the
AdvancedSearchForm context provider, the AdvancedSearchForm prop list, and
SavedSearches so the delete path uses the context onDeleteSearch handler.
- Around line 348-361: The text filter's handleChange only updates localValue
and calls onChange when autoApply is true, which leaves activeFilters stale in
manual mode; update the Apply/Save/Export handlers to call the provided onChange
with the current localValue when applying manually and ensure any manual change
handlers don't swallow the intent to apply later. For the numeric filter
(SearchNumericRangeFilter) add a useEffect that rehydrates localMin and localMax
from the incoming value prop (similar to how localValue is synced from value)
and ensure the Reset/Apply paths call onChange with the numeric value parsed
from localMin/localMax so UI and saved-search apply/export use the latest
numbers. Reference: useSearch, localValue, setLocalValue, handleChange,
onChange, SearchNumericRangeFilter, localMin, localMax, and value.
- Around line 1455-1458: SearchFacetedHints currently ignores the onSelect prop
and always writes the selected value as an array (e.g. [value]), breaking scalar
filters; update the SearchFacetedHints component to destructure and call the
onSelect(categoryId, value) callback when provided, and otherwise update the
filter in context while preserving the target filter’s original shape (keep
scalar values scalar and arrays as arrays instead of always wrapping value in an
array). Ensure the branch that previously used the unconditional [value]
fallback is changed to detect the existing filter type and write a scalar or
array accordingly when onSelect is absent.
- Around line 369-392: The controls lack programmatic labels and the date
trigger is not keyboard-operable: update the label rendered by Typography to
include htmlFor={filter.id} (so it pairs with the existing id on the input used
by value={localValue} and onChange={handleChange}), give distinct visible labels
for Min/Max where applicable (create separate labelled inputs using unique ids
like `${filter.id}-min` and `${filter.id}-max`), and replace any custom date
trigger div with a real <button> element (manage focus, keyboard interaction,
and expose state with aria-expanded and aria-haspopup) so the date picker
component can be operated via keyboard and is accessible. Ensure styling and
theme logic (isMobile/theme) remain the same while wiring these accessibility
attributes.
- Around line 247-255: formatDate and parseDate currently round-trip via UTC
(toISOString/new Date("YYYY-MM-DD")) causing off-by-one-day for non-UTC users;
change formatDate to build the "YYYY-MM-DD" string from local date components
(date.getFullYear(), date.getMonth()+1, date.getDate()) and change parseDate to
avoid Date parsing from a string by splitting "YYYY-MM-DD" and constructing a
local Date with new Date(year, month-1, day), returning null for invalid parts;
update the functions named formatDate and parseDate accordingly to keep all
dates in local time.
- Around line 28-35: The form ignores BaseFilterConfig.defaultValue because the
component only hydrates from initialFilters and Reset clears to an empty object;
fix by merging each filter's BaseFilterConfig.defaultValue into the form's
starting state and use that merged object as the Reset target. Locate where
initialFilters is turned into root state (the form initialization in
advanced-search-form/index.tsx) and update it to iterate BaseFilterConfig
entries, coalescing defaultValue -> initialFilters (respecting types: string,
string[], date-range, numeric-range) so CLI defaults are present on first
render; then update the Reset handler to reset to that merged defaults object
(not {}) so defaultValue survives resets. Ensure references:
BaseFilterConfig.defaultValue, initialFilters, and the Reset handler in this
file are updated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 84e7317d-f4a2-4398-b13e-4a2a8ee3bd79
📒 Files selected for processing (4)
apps/docs/src/components/UI/advanced-search-form/index.tsxapps/docs/versioned_docs/version-1.0.3/templates/pages/forms/advanced-search-form.mdxapps/storybook/src/templates/pages/form-pages/advanced-search-form/index.tsxpackages/registry/templates/pages/advanced-search-form/index.tsx
📜 Review details
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-03-31T07:07:43.376Z
Learnt from: PoonamKPatil
Repo: mindfiredigital/ignix-ui PR: 723
File: packages/registry/templates/pages/contact-form/index.tsx:143-148
Timestamp: 2026-03-31T07:07:43.376Z
Learning: In `packages/registry/templates/pages/contact-form/index.tsx` (React/TypeScript), the `useToast()` call is deliberately wrapped in try/catch to make toast notifications optional — the ContactForm is designed to work without a ToastProvider. The correct fix is a `useSafeToast` wrapper that calls `React.useContext(ToastContext)` directly and returns null when the context is absent, rather than throwing.
Applied to files:
packages/registry/templates/pages/advanced-search-form/index.tsx
📚 Learning: 2026-03-18T09:43:15.140Z
Learnt from: CR
Repo: mindfiredigital/ignix-ui PR: 0
File: README.md:0-0
Timestamp: 2026-03-18T09:43:15.140Z
Learning: Applies to src/**/*.tsx : Ensure components are accessible by default and follow accessibility best practices
Applied to files:
packages/registry/templates/pages/advanced-search-form/index.tsxapps/docs/versioned_docs/version-1.0.3/templates/pages/forms/advanced-search-form.mdx
📚 Learning: 2026-03-31T05:22:49.923Z
Learnt from: PoonamKPatil
Repo: mindfiredigital/ignix-ui PR: 723
File: apps/docs/versioned_docs/version-1.0.3/templates/pages/forms/contact-form.mdx:29-31
Timestamp: 2026-03-31T05:22:49.923Z
Learning: In mindfiredigital/ignix-ui documentation under apps/docs/versioned_docs, the CLI command `ignix add component <name>` is an intentional unified command that imports both components and templates. When this command appears in template-related docs/examples (e.g., template documentation snippets), do not flag it as incorrect or misleading; it is the expected usage.
Applied to files:
apps/docs/versioned_docs/version-1.0.3/templates/pages/forms/advanced-search-form.mdx
Comment on lines
+29
to
+35
| import * as React from "react"; | ||
| import { cva, type VariantProps } from "class-variance-authority"; | ||
| import { motion, AnimatePresence } from "framer-motion"; | ||
| import { cn } from "../../../utils/cn"; | ||
| import { Typography } from "@ignix-ui/typography"; | ||
| import DatePicker from "@ignix-ui/date-picker"; | ||
| import { Checkbox } from "@ignix-ui/checkbox"; |
Contributor
There was a problem hiding this comment.
The Manual tab still imports a monorepo-only cn path.
../../../utils/cn only resolves inside this repository layout. If this tab is meant to be copy-pasteable for consumers, the snippet fails immediately. Point it at a public helper or inline cn; otherwise label the snippet as repo-internal.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/docs/versioned_docs/version-1.0.3/templates/pages/forms/advanced-search-form.mdx`
around lines 29 - 35, The import currently pulls a repo-local helper
("../../../utils/cn") which won't resolve for external consumers; update the top
imports to either (a) replace the import with a public/common helper (for
example from a published package) or (b) inline a small local `cn` utility in
this file, then update references to `cn` used by components like the
class-variance-authority usage and any JSX className concatenations; ensure the
import line for `cn` is removed and references point to the new public helper or
the in-file `cn` function so the Manual tab snippet is copy-pasteable outside
the monorepo.
anandmindfire
approved these changes
Apr 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Title
Revamp Advanced Search Form UI with Themes, Stories, Docs, and Tests
Description
What does this PR do?
This PR introduces a complete redesign of the Advanced Search Form, improving both UI and developer experience.
Key improvements include:
Redesigned UI for better usability and consistency
Added light and dark theme support
Created Storybook stories for all states and variants
Added a demo implementation for better visualization and testing
Wrote comprehensive test cases
Updated and enhanced documentation
Closes [Feature]: Advanced Search Form (5.5.5) #645
Type of Change
Checklist
npm run lint.New Components or Component API Changes
Bug fixes
Tooling / Config Changes
Docs / Storybook Updates
Breaking changes