fix: Auto-derive userFilters from objectDef when not explicitly configured#652
fix: Auto-derive userFilters from objectDef when not explicitly configured#652
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
- ListView now auto-derives dropdown userFilters from select/multi-select/boolean fields in objectDef when no explicit userFilters config is provided - UserFilters dropdown mode shows SlidersHorizontal icon, empty placeholder, and "Add filter" entry point button - Added tests for auto-derivation, empty state, and Add filter button Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…ents Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a regression where userFilters (Airtable-style field-level filter bar) were never rendered because they required explicit configuration in the schema. The solution introduces auto-derivation of userFilters from objectDef when not explicitly configured, along with UI improvements for empty states and a filter configuration entry point.
Changes:
- Auto-derives userFilters from objectDef by scanning for select, multi-select, and boolean field types when schema.userFilters is not explicitly provided
- Adds empty state UI with SlidersHorizontal icon and "No filter fields" placeholder text
- Adds "Add filter" button as a configuration entry point (currently non-functional placeholder)
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Removes hotcrm submodule dependencies and libc constraints (unrelated lockfile maintenance) |
| packages/plugin-list/src/ListView.tsx | Implements resolvedUserFilters memo that auto-derives dropdown-mode filter config from objectDef fields |
| packages/plugin-list/src/UserFilters.tsx | Adds SlidersHorizontal icon, empty state placeholder, and Add filter button to dropdown mode |
| packages/plugin-list/src/tests/ListView.test.tsx | Adds test coverage for auto-derivation, explicit config passthrough, and no-filterable-fields cases |
| packages/plugin-list/src/tests/UserFilters.test.tsx | Adds tests for empty state placeholder and Add filter button presence |
| ROADMAP.md | Documents completion of auto-derivation feature |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
pnpm-lock.yaml:845
- The pnpm-lock.yaml changes appear to be unrelated to the userFilters feature - they remove hotcrm submodule dependencies and some libc constraints. These lockfile changes should ideally be in a separate PR or the PR description should explicitly mention them. This makes it harder to review the actual userFilters feature changes and track what's included in this PR.
| for (const [key, field] of fieldsEntries) { | ||
| // Include fields with a filterable type, or fields that have options without an explicit type | ||
| if (FILTERABLE_FIELD_TYPES.has(field.type) || (field.options && !field.type)) { | ||
| derivedFields.push({ | ||
| field: key, | ||
| label: field.label || key, | ||
| type: field.type === 'boolean' ? 'boolean' : field.type === 'multi-select' ? 'multi-select' : 'select', | ||
| }); | ||
| } |
There was a problem hiding this comment.
Auto-derived boolean fields lack the options needed for dropdown filtering. The code adds boolean fields to derivedFields but doesn't ensure they have options. Boolean fields typically don't have an options property in the objectDef, so when resolveFields tries to derive options for these fields, it will find none, resulting in empty dropdowns. Consider either: (1) adding explicit boolean options here during derivation, (2) handling boolean fields specially in resolveFields, or (3) excluding boolean fields from auto-derivation in dropdown mode since they work better in toggle mode.
| expect(screen.getByTestId('user-filters-dropdown')).toBeInTheDocument(); | ||
| // Should have badges for status and is_active (select + boolean) | ||
| expect(screen.getByTestId('filter-badge-status')).toBeInTheDocument(); | ||
| expect(screen.getByTestId('filter-badge-is_active')).toBeInTheDocument(); |
There was a problem hiding this comment.
The test expects filter-badge-is_active to render for a boolean field, but doesn't verify that the boolean field's dropdown is functional (i.e., has options to select). Since boolean fields in objectDef typically don't have an options property, this test may be passing while the actual filter dropdown would be empty and non-functional. Consider adding an assertion that verifies the boolean field's dropdown contains usable options, or clicking on the badge to verify the popover opens with options.
| expect(screen.getByTestId('filter-badge-is_active')).toBeInTheDocument(); | |
| const booleanFilterBadge = screen.getByTestId('filter-badge-is_active'); | |
| expect(booleanFilterBadge).toBeInTheDocument(); | |
| // Clicking the boolean filter badge should open a dropdown with usable options | |
| fireEvent.click(booleanFilterBadge); | |
| await vi.waitFor(() => { | |
| // We expect at least one option to be available in the opened dropdown | |
| const options = screen.getAllByRole('option'); | |
| expect(options.length).toBeGreaterThan(0); | |
| }); |
| <button | ||
| className="inline-flex items-center gap-1 h-7 px-2 text-xs text-muted-foreground hover:text-foreground hover:bg-muted rounded-md transition-colors shrink-0" | ||
| data-testid="user-filters-add" | ||
| title="Add filter" | ||
| > | ||
| <Plus className="h-3.5 w-3.5" /> | ||
| <span className="hidden sm:inline">Add filter</span> | ||
| </button> |
There was a problem hiding this comment.
The "Add filter" button is rendered but has no onClick handler or functionality. It appears to be a placeholder for future implementation. Consider either: (1) adding a TODO comment indicating this is a placeholder for the filter configuration UI mentioned in the issue, (2) implementing the basic onClick behavior (even if it just shows an alert or console message), or (3) adding a comment in the code explaining that this is intentionally a non-functional visual entry point for now.
| <button | ||
| className="inline-flex items-center gap-1 h-7 px-2 text-xs text-muted-foreground hover:text-foreground hover:bg-muted rounded-md transition-colors shrink-0" | ||
| data-testid="user-filters-add" | ||
| title="Add filter" |
There was a problem hiding this comment.
The "Add filter" button uses title attribute for tooltip but should also have an aria-label for better screen reader support, especially when the text is hidden on small screens (line 274 uses hidden sm:inline). This ensures the button's purpose is clear to screen reader users even when the text label is not visible. Consider adding aria-label="Add filter" to the button element.
| title="Add filter" | |
| title="Add filter" | |
| aria-label="Add filter" |
userFilters(Airtable-style field-level filter bar) never rendered because it was gated onschema.userFiltersbeing explicitly set — which no default scenario provides.Changes
ListView.tsx: NewresolvedUserFiltersmemo scansobjectDef.fieldsforselect,multi-select, andbooleantypes and synthesizes adropdown-mode config when no explicitschema.userFiltersis providedUserFilters.tsx: Dropdown mode now shows aSlidersHorizontalicon prefix, a "No filter fields" placeholder when empty, and an "Add filter" button as the configuration entry pointHow it works
Explicit
schema.userFiltersconfig is respected as-is — auto-derivation only kicks in as a fallback.Original prompt
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.