feat(h-grid): update adv filtering schema on row expand - #15927
igdmdimitrov wants to merge 12 commits into
Conversation
|
@igdmdimitrov There's an issue when expanding inner levels:
Result: The following error is thrown in the console: Expected result: No errors |
|
As currently setup, the AF dialog cannot be open in HGrid Advanced Filtering dev demo. The following console error is thrown: |
|
@igdmdimitrov In the |
gedinakova
added
💥 status: in-test
gedinakova
left a comment
gedinakova
left a comment
There was a problem hiding this comment.
We need a Changelog update for this change.
|
There has been no recent activity and this PR has been marked inactive. |
github-actions
Bot
added
the
status: inactive
Co-authored-by: Galina Edinakova <gedinakova@infragistics.com>
|
There has been no recent activity and this PR has been marked inactive. |
github-actions
Bot
added
the
status: inactive
|
There has been no recent activity and this PR has been marked inactive. |
github-actions
Bot
added
the
status: inactive
|
There has been no recent activity and this PR has been marked inactive. |
github-actions
Bot
added
the
status: inactive
|
There has been no recent activity and this PR has been marked inactive. |
github-actions
Bot
added
the
status: inactive
damyanpetev
left a comment
damyanpetev
left a comment
There was a problem hiding this comment.
Not entirely sure all dynamic column scenarios are handled, though that might've been the case even before - generateSchema() is called only on init and for some reason when setting the expression tree. updateColumns/onColumnsChanged?
schema will likely benefit greatly being a computed() signal eventually (will make it lazy too), sadly don't think we have enough of the rest of the pieces there yet (columnList/childLayoutList).
And we'll need to update the deprecated versions :)
| public set schema(entities: EntityType[]) { | ||
| this._hGridSchema = entities; | ||
| } | ||
| public set schema(entities: EntityType[]) {} |
There was a problem hiding this comment.
A bit iffy on the setter being noop - while it's doubtful that its seen major usage, ideally we'd have it fully functional until its removed.
Otherwise, at least make sure whatever scenario this prop was meant to support, that even with the inactive setter doesn't change with the new functionality handling schema internally.
| this.batchEditing = val; | ||
| }); | ||
| } | ||
| this.columnsAutogenerated.subscribe((e) => { |
There was a problem hiding this comment.
With pipe(takeUntilDestroyed()) like other subs?
| } | ||
|
|
||
| private generateChildEntity(rowIsland: IgxRowIslandComponent, firstRowData: any[]): EntityType { | ||
| private updateRootGridSchema(event: any) { |
There was a problem hiding this comment.
I'm sure the even has a type, other than any
| defaultTimeFormat: f.defaultTimeFormat, | ||
| defaultDateTimeFormat: f.defaultDateTimeFormat | ||
| })) as FieldType[]; | ||
| } |
There was a problem hiding this comment.
While the optimization for a single path update sounds nice, grid setups rarely changes w/ high frequency and the structures being processed are rarely, but sure - I can recall a few massive column lists, but even those are likely in the 100s, which is nothing much for list.
Regardless, two questions about this:
- This logic is more/less a repeat of the one in
generateSchema()? If so, would be nice if those are either combined, reused or otherwise refactored into a single thing that does the islands/columns -> entity/fields mapping - And isn't the column directly compatible with
FieldType? Does the QB modify something in the schema fields? Otherwise, I wonder why not just take the filtered columns directly?
igdmdimitrov
added
🛠️ status: in-development
There was a problem hiding this comment.
Pull request overview
This PR addresses Hierarchical Grid advanced filtering schema generation for nested layouts (notably when child data is missing) by avoiding schema inference from the first record and instead updating the generated schema when row islands expand/auto-generate their columns. It also introduces a deprecation note for the schema input and adjusts tests and changelog accordingly.
Changes:
- Deprecates the
IgxHierarchicalGridComponent.schemainput and shifts schema updates to occur when row islands expand (columns auto-generate). - Updates schema generation to avoid reading nested child data from the first record (prevents runtime errors when data is missing).
- Updates the advanced filtering spec to validate schema updates after row island expansion; updates the changelog; makes
getFormatsafer.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| projects/igniteui-angular/query-builder/src/query-builder/query-builder-tree.component.ts | Adjusts query builder formatting retrieval to be more null-safe. |
| projects/igniteui-angular/grids/hierarchical-grid/src/hierarchical-grid.component.ts | Deprecates schema input behavior, changes schema generation for child entities, and adds root schema updates on column auto-generation. |
| projects/igniteui-angular/grids/grid/src/grid-filtering-advanced.spec.ts | Reworks a test to validate schema updates after row island expansion (advanced filtering UI). |
| CHANGELOG.md | Documents the IgxHierarchicalGrid.schema deprecation. |
| public getFormat(field: string) { | ||
| return this.fields?.find(el => el.field === field).pipeArgs.format; | ||
| return this.fields?.find(el => el.field === field)?.pipeArgs.format; | ||
| } |
| * const schema = this.grid.schema; | ||
| * this.grid.schema = [{ name: 'Products', fields: [...], childEntities: [...] }]; | ||
| * ``` | ||
| * @deprecated in version 20.2.0. | ||
| */ |
| @Input() | ||
| public set schema(entities: EntityType[]) { | ||
| this._hGridSchema = entities; | ||
| } | ||
| public set schema(entities: EntityType[]) {} | ||
|
|
| let childEntity = schema[0]; | ||
| for (let i = 0; i < path.length; i++) { | ||
| childEntity = childEntity.childEntities.find(e => e.name === path[i]); | ||
| } | ||
|
|
Closes #15827
Closes #15908
Additional information (check all that apply):
Checklist:
feature/README.MDupdates for the feature docsREADME.MDCHANGELOG.MDupdates for newly added functionalityng updatemigrations for the breaking changes (migrations guidelines)