feat: add rich filter component with modular field engine and primitive components#1213
Open
robmanganelly wants to merge 5 commits intospartan-ng:mainfrom
Open
feat: add rich filter component with modular field engine and primitive components#1213robmanganelly wants to merge 5 commits intospartan-ng:mainfrom
robmanganelly wants to merge 5 commits intospartan-ng:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a comprehensive rich filter system for building dynamic, type-safe query filters with a modular field engine. It includes two new primitive components (hlm-range-slider and hlm-time-input) and supports multiple field types: text, date, date range, time, boolean, combo (sync/async), range, and select. The implementation features focus management, keyboard navigation, ARIA attributes, and extensive unit test coverage.
Changes:
- Adds new brain/helm primitives: range-slider and time-input components with full accessibility support
- Implements modular filter engine with builders, handlers, operators, parser, and state management
- Creates field components for all supported types with proper focus management and keyboard navigation
- Includes comprehensive unit tests for engine utilities, handlers, parser, field specs, and primitives
Reviewed changes
Copilot reviewed 64 out of 64 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.base.json | Adds path mappings for new range-slider and time-input packages |
| libs/brain/range-slider/* | Implements headless range slider with dual thumbs, keyboard control, and CVA support |
| libs/brain/time-input/* | Implements 12-hour time input with segments, keyboard navigation, and picker popover |
| libs/helm/range-slider/* | Styled range slider component wrapping brain implementation |
| libs/helm/time-input/* | Styled time input with popover picker and clock icon trigger |
| apps/app/.../rich-filter/engine/* | Core filter engine with types, operators, builders, handlers, and parser |
| apps/app/.../rich-filter/fields/* | Field component implementations for all supported types |
| apps/app/.../rich-filter/rich-filter.ts | Main filter component with dynamic field rendering |
| *.spec.ts | Comprehensive test suites for all new functionality |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Greptile SummaryThis PR introduces a comprehensive rich filter system with modular architecture and two new primitives ( Architecture:
Key strengths:
Implementation notes:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User Interaction] --> B{Add Filter Field}
B --> C[buildFilterModel creates state]
C --> D[Field added to model]
D --> E[SpartanRichFilter renders field]
E --> F[NgComponentOutlet with cached injector]
F --> G[Field-specific handler provides state access]
G --> H{Field Type}
H -->|Text/Number| I[Basic input fields]
H -->|Date/Time| J[Calendar/Time picker]
H -->|Combo/Select| K[Dropdown/Combobox]
H -->|Async Combo| L[HTTP Resource with debounce]
H -->|Range| M[Dual-thumb Range Slider]
H -->|Boolean| N[Checkbox field]
I --> O[User edits value]
J --> O
K --> O
L --> O
M --> O
N --> O
O --> P[Handler updates state via WritableSignal]
P --> Q[filterParser strips private fields]
Q --> R[Output: Clean query object]
Last reviewed commit: 1eddd32 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+131
to
+134
| // TODO fix combobox api to expose a reference to the input element | ||
| // to make it possible to focus without querying the DOM | ||
| const el = this.monitoredInput().nativeElement.querySelector('input[brnComboboxInput]'); | ||
| this.service.isFocused() && this.focusMonitor.focusVia(el, FAKE_FOCUS_ORIGIN); |
There was a problem hiding this comment.
DOM query to find input element - consider exposing input ref from combobox API to avoid direct DOM manipulation
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Author
There was a problem hiding this comment.
out of scope for this task, if this is desired must be resolved via issue, not in this PR
| Promise.resolve().then(() => { | ||
| this.popoverBtn()?.nativeElement.click(); | ||
| }); | ||
| // TODO: Does BrainPopoverTrigger expose a method to close the popover programmatically? |
There was a problem hiding this comment.
Check if BrnPopoverTrigger exposes a close method to avoid this workaround
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Author
There was a problem hiding this comment.
out of scope for this task, if this is desired must be resolved via issue, not in this PR
Collaborator
|
This looks awesome! I noticed that you added a new component for range-slider, however, the slider has recently received some updates and includes now a range as well. Can you have a look if you could use the already existing slider range https://spartan.ng/components/slider#examples__range? |
robmanganelly
force-pushed
the
feature-rich-filter-squashed-commits
branch
from
ad55841 to
b24ba73
Compare
…ve components Introduces a comprehensive rich filter system for building dynamic, type-safe query filters. Includes a field engine with builders, handlers, operators, parser, and state management, along with new hlm-range-slider and hlm-time-input primitive components. Field types supported: text, date, date range, time, boolean, combo (sync and async), range, and select. Async combobox fields include debounced resource request handling and a configurable itemToString default. Focus management and keyboard navigation improvements are applied across all field types. ARIA attributes and accessibility enhancements are included for range-slider and time-input components. Unit tests cover engine utilities, builders, handlers, parser, field specs, and the new primitive components.
Add explicit `public` modifiers to satisfy @typescript-eslint/explicit-member-accessibility. Add eslint-disable comments for @typescript-eslint/naming-convention on `_`-prefixed bindings that must retain their names. Remove unused imports (ResourceRef, WritableSignal, computed, input, RangeValue, FilterModelRef, screen). Rename FieldClose output from `onCloseField` to `fieldclosed` to satisfy @angular-eslint/no-output-on-prefix. Prefix test host selectors with `spartan-` per @angular-eslint/component-selector. Remove redundant `standalone: true` (default since Angular 19). Reorder @component metadata keys per @nx/workspace-component-directive-key-order. Use `type` imports where values are only used as types.
Run format:write on the codebase to fix failing CI task
Add CLI generator infrastructure for the range-slider and time-input helm primitives, enabling scaffolding via `nx g @spartan-ng/cli:ui`. - Add generator entry points and EJS templates for both primitives - Register range-slider and time-input in primitives type, dependency map, and supported UI libraries config - Update generate-ui-docs snapshot to reflect time-input API changes
robmanganelly
force-pushed
the
feature-rich-filter-squashed-commits
branch
from
b24ba73 to
cd114e2
Compare
…filter - Remove custom range slider cmp and related files - Update rich filter field cmp to use helm slider - Adjust imports and templates in rich filter fields - Remove all references to the old range slider from the codebase
robmanganelly
force-pushed
the
feature-rich-filter-squashed-commits
branch
from
cd114e2 to
3bf0039
Compare
Author
commit 3bf0039 implements this feedback |
Author
|
any update here? |
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
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.
Introduces a comprehensive rich filter system for building dynamic, type-safe query filters. Includes a field engine with builders, handlers, operators, parser, and state management, along with new hlm-range-slider and hlm-time-input primitive components.
Field types supported: text, date, date range, time, boolean, combo (sync and async), range, and select. Async combobox fields include debounced resource request handling and a configurable itemToString default.
Focus management and keyboard navigation improvements are applied across all field types. ARIA attributes and accessibility enhancements are included for range-slider and time-input components.
Unit tests cover engine utilities, builders, handlers, parser, field specs, and the new primitive components.
PR Checklist
Please check if your PR fulfills the following requirements:
guidelines: https://github.com/spartan-ng/spartan/blob/main/CONTRIBUTING.md#-commit-message-guidelines
PR Type
What kind of change does this PR introduce?
Which package are you modifying?
Primitives
Others
What is the current behavior?
Nothing modified, existing flows remain unchanged
Closes #
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Screen Recording of the expected functionality
compressed-Screen.Recording.mp4
Architectural decisions
Engine is kept apart from the component to make it easier for users to implement their own version of the engine.
Each field comes with its own handler, Every new field should also have its own handler,
Feature uses function composition to extend functionalities, instead of inheritance
This feature can benefit from signal forms, once spartan supports them
I tried to not add any new dependency while creating this feature
The 2 new primitives that were created are not documented, pending approval for the spartan team, I did not want to waste time documenting them before knowing if they are going to make the cut, but this block really benefits from them
I decided to not extend the slider primitive and instead add one slider that have both edges movable, to prevent any breaking change in the project,
Feedback is highly appreciated, I hope you see this as useful as I do