Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hierarchical Grid: scrolled child views remain after the root grid has been destroyed #4440

Closed
damyanpetev opened this issue Apr 2, 2019 · 1 comment
Assignees
Labels
🐛 bug Any issue that describes a bug 🧨 severity: medium grid: hierarchical-grid version: 7.2.x ✅ status: resolved Applies to issues that have pending PRs resolving them, or PRs that have already merged.

Comments

@damyanpetev
Copy link
Member

Description

Turned up from a test case
hierarchical-grid.navigation.spec > IgxHierarchicalGrid Complex Navigation > should allow navigating up from parent into nested child grid
that scrolls child instances by a significant size (to last).

Already looked into it with @MayaKirova and as far as we can see after the root Hierarchical Grid instance had been destroyed, 3 child grids in IgxTemplateOutletDirective's _embeddedViewsMap cache remained not destroyed, thus not going through OnDestroy hook and removing their handlers.
The error in the tests is merely a side-effect of that.

I've commented out the test that reproduces this behavior and added a repro one:

xit('should allow navigating up from parent into nested child grid', (async () => {
hierarchicalGrid.verticalScrollContainer.scrollTo(2);
await wait(100);
fixture.detectChanges();
const child = hierarchicalGrid.hgridAPI.getChildGrids(false)[0];
const lastIndex = child.verticalScrollContainer.igxForOf.length - 1;
child.verticalScrollContainer.scrollTo(lastIndex);
await wait(100);
fixture.detectChanges();
child.verticalScrollContainer.scrollTo(lastIndex);
await wait(100);
fixture.detectChanges();
const parentCell = hierarchicalGrid.getCellByColumn(2, 'ID');
parentCell.nativeElement.focus();
await wait(100);
fixture.detectChanges();
const keyboardEvent = new KeyboardEvent('keydown', {
code: 'ArrowUp',
key: 'ArrowUp'
});
parentCell.dispatchEvent(keyboardEvent);
await wait(100);
fixture.detectChanges();
const nestedChild = child.hgridAPI.getChildGrids(false)[5];
const lastCell = nestedChild.getCellByColumn(4, 'ID');
expect(lastCell.selected).toBe(true);
expect(lastCell.focused).toBe(true);
expect(lastCell.rowIndex).toBe(4);
}));
xit('ViewDestroyedError: Attempt to use a destroyed view trigger', (async () => {
// Repro for leftover un-destroyed grid views (leaked resize handler):
// fit this and the test above
window.dispatchEvent(new Event('resize'));
}));

  • igniteui-angular version: 7.2.x (7.2.2)
  • browser: N/A

Steps to reproduce

  1. [Optional] If feat(igxGrid): Multi cell selection #3916 is not yer merged, switch to the branch
  2. fit the "should allow navigating up from parent into nested child grid" and the repro test after it in hierarchical-grid.navigation.spec.ts
  3. Run the tests
  4. Observe the console

Result

The remaining resize handlers trigger actions on the destroyed fixture and throw:
image

Expected result

After the Hierarchical Grid is destroyed, there shouldn't be any child instances left behind.

Attachments

Attach a sample if available, and screenshots, if applicable.

@damyanpetev
Copy link
Member Author

Note: Reproducible in 7.2.x as well (if you debug the resize handler in the base grid), except the test lacks the onChunkLoaded helper that would throw.

zdrawku added a commit that referenced this issue Apr 3, 2019
@MayaKirova MayaKirova added 🛠️ status: in-development Issues and PRs with active development on them and removed 🆕 status: new labels Apr 3, 2019
zdrawku pushed a commit that referenced this issue Apr 3, 2019
* chore(igxGrid): Initial MCS PoC

* chore(*): Restore cell edit logic

* refactor(igxCell): More refactor, more selection logic

* test(grid): add mcs test #3794

* refactor(selection): Selection service refactor

Keyboard selection through summary and group rows

* refactor(*): Refactor setSelection method

* refactor(selection): Remove pointer event range duplication

* test(MultiCellSelection): add integration tests for sorting and filtering #3794

* chore(selection): Fix _updateCellSelection with kb nav

* refactor(selection): Added drag scroll directive

Refactored some service state

* test(grid): update mcs tests #3794

* test(grid): Update mcs tests #3794

* refactor(selection): Pinned, filtered, hidden states

* refactor(selection): Groupby state returns correct data

* refactor(selection): Return only expanded states

* chore(selection): Resolve DI errors

* test(MCS): added intregration test for moving, paging, CRUD, summaries #3794

* test(MCS): added test component for grid with transaction enabled #3794

* test(grid): add selection tests #3794

* test(treeGrid): Add tree grid tests #3794

* test(MCS): test refactorin and additional test when hide all columns #3794

* test(grid): add mcs tests #3794

* test(MSC): added treeGrid tests for CRUD and range selection #3794

* feat(grid): Multi selection in grids

Grid cells now have onPush by default
Cleared and probably broke some of the CRUD operations

Closes #3915

* refactor(grid): Restore polyfills and search specs

* test(MCS): fix tests for treeGrid and CRUD #3794

* merge rkaraivanov/mcs-plus-refactor

* test(treeGrid): Update failing tests #3794

* test(MCS): trigger CD over grid when perform pointer interactions #3794

* test(treeGrid): update failing test #3794

* feat(grid): Column moving with multi selection

Column moving should no longer move selection/focus on drop
Unit tests refactored for cell onPush
Tree cell gets its expanded state as an input now

Closes #3915

* test(treeGrid): Update failing summaries keyboard tests #3794

* feat(grid): Hierarchical navigation and selection

Closes #3915

* fix(grid): Refocus correct cell in tree grid row roggle

Closes #3915

* test(MCS): selection should not change on column moving #3794

* fix(igxGrid): refactoring column init fix #3916

* chore(*): adding breaking change to changelog

* chore(*): selection should be preserved on column move and scroll

* fix(selection): Only one selection range unless ctrl is held

Closes #3915

* refactor(grid): Cleaned up editing logic

* refactor(grid): More editing cleanup

* test(MCS): empty array should not clear selection #3794

* fix(selection): getSelectedData omits MCH

Closes #3915

* test(MCS): selected range not changed on moving #3794

* refactor(grid): Adjust tests and restore cell editMode setter

* chore(*): Remove fdescribe

* fix(selection): getSelectedData retruns correct results

Fixed several issues with API based updates for cell/row
Corrected some unit tests for cell onPush strategy

Closes #3915

* fix(selection): Selection with paging state

Correctly trigger cdr on perPage change and clear active selection style
on paging event

Closes #3915

* chore(*): add change detection in search tests

* chore(*): remove fdescribe from search

* chore(*): pagning test emit

* chore(*): cast cell value to number after emit, break when updating invalid row

* fix(grid): Row edit args - don't pass a reference

Closes #3915

* chore(*): fix failing tests for row editing / transactions

* refactor(selection): Cell/selection service code cleanup

Removed a unit test for selection as it tested the old behavior. Fixed
some tests expecting invalid values.

* refactor(*): Thank you tslint

* fix(selection): Clear all through API call

Closes #3915

* refactor(grid): CRUD fixes and unit test fixes

* fix(grid): Actually return a value from castToNumber

Closes #3915

* chore(*): do not dispatch pointer down when simulate mouse drag

* fix(grid): endEdit with API calls

Fixed a CRUD unit test updating a number column and expecting a string
back

Closes #3915

* chore(*): use correctly IgxHierarchicalTransactionService

* fix(grid): Don't submit when nothing is in edit mode

Closes #3915

* refactor(selection): Tree grid get selected data

Tree cell indent level is now an input property of the cell itself

* refactor(treeGrid): populate flatData for pk/fk binding

* fix(selection): Shift key through groupby/summary

Update selection state when navigating through groupby rows and summary
cells
All pointer events are executed outside of ngzone

Closes #3915

* test(MCS): clear selectedRange when navigate from summaryCell witout shift #3794

* refactor(selection): Simplify getSelectedData

* test(MCS): fixed test with keyboard MCS with KB navigation #3794

* fix(selection): Active cell style remains on drag scroll

Export drag select directive in the grid common module.

Closes #3915

* test(grid): add test for bug 4084 #3794

* test(grid): add mcs tests with keybord and mouse events #3794

* test(treeGrid): add test for msc #3794

* perf(igx-grid): Grid runtime performance optimization

OnPush everywhere, caches and pipes for the most accessed properties
Some getters were turned into simple input properties
Reduces number of change detection in scroll handlers

Closes #3915

* fix(igx-grid): Filtering service injections

Closes #3915

* test(igx-grid): Keyboard navigation with scroll

Closes #3915

* chore(*): Fixing change detection issues in HierarchicalGrid navigation tests.

* fix(treeGrid): fix search active highlight #3915

* refactor(igx-grid): Reduce number of getter calls

Use basic @input values
Resolve cell styles using a pipe

* test(grid): fix failing search tests with scroll  #3794

* chore(igx-grid): Fix lint import error

* test(grid): update failing paging test #3794

* fix(igx-tree-grid): Cell indicator and MCS with expand/collapse

Closes #3915

* test(grid): update failing tests #3794

* fix(igx-grid): Recalc scroll offset after chunk size change

Closes #3915

* test(hgrid): update failing tests #3794

* refactor(grid): prevent potential mutation observer memory leak

* fix(igxFor): Making sure correct offsets are applied when data in igForOf changes.

* fix(igxFor): Fixing chunksize calculations so that there are no empty spaces at the end in case there are large items in the data.

* refactor(igx-grid): Summary cell/row

Removed imports which are not used
Moved some of the getter logic in the template

* test(grid): fix async tests that cause ViewDestroyedError #3794

* test(hierarchical-grid): fix async tests that cause ViewDestroyedError #3794

* test(tree-grid): fix async tests up to integr causing ViewDestroyedError #3794

* test(tree-grid): fix async tests up to integr causing ViewDestroyedError #3794

* test(hierarchical-grid): fix async tests causing ViewDestroyedError #3794

Added wait on a test triggering multiple rAf-s and commented out an HGrid
navigation test (that leaked resize handlers) until the issue is fixed.

* fix(igx-grid): Drag selection from outside the grid

Closes #3916

* chore(*): Skip 2 tests #4440
MayaKirova pushed a commit that referenced this issue Apr 4, 2019
@MayaKirova MayaKirova added ✅ status: resolved Applies to issues that have pending PRs resolving them, or PRs that have already merged. and removed 🛠️ status: in-development Issues and PRs with active development on them labels Apr 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Any issue that describes a bug 🧨 severity: medium grid: hierarchical-grid version: 7.2.x ✅ status: resolved Applies to issues that have pending PRs resolving them, or PRs that have already merged.
Projects
None yet
Development

No branches or pull requests

4 participants