Refactoring filter cell navigation so that it is handled in the navigation service. Handling special scenarios for hierarchical grid in the hierarchical navigation service. #4267

MayaKirova · 2019-03-11T13:40:27Z

Closes #4217
Closes #4277

Additional information (check all that apply):

Checklist:

All relevant tags have been applied to this PR
This PR includes unit tests covering all the new code
This PR includes API docs for newly added methods/properties
This PR includes feature/README.MD updates for the feature docs
This PR includes general feature table updates in the root README.MD
This PR includes CHANGELOG.MD updates for newly added functionality
This PR contains breaking changes
This PR includes ng update migrations for the breaking changes
This PR includes behavioral changes and the feature specification has been updated with them

…is handled in the navigation service. Hanling special scenarios for hierarchical grid in the hierarchical navigation service.

…ilter cells in performTab since navigation should consider them only for Tab navigation (should skip them with arrow keys). Adding test.

sstoyanovIG · 2019-03-11T14:46:15Z

projects/igniteui-angular/src/lib/grids/grid-navigation.service.ts

        }
    }

+    public navigatePrevFilterCell(column: IgxColumnComponent, eventArgs) {
+        const cols = this.grid.filteringService.unpinnedFilterableColumns;
+        const prevFilterableIndex = this.grid.filteringService.unpinnedFilterableColumns.indexOf(column) - 1;


You can use the 'cols' variable you defined above instead of 'this.grid.filteringService.unpinnedFilterableColumns'

sstoyanovIG · 2019-03-11T14:47:08Z

projects/igniteui-angular/src/lib/grids/grid-navigation.service.ts

+
+        if (prevIndex >= 0 && visibleIndex > 0 && !this.isColumnLeftFullyVisible(prevIndex) && !column.pinned) {
+            eventArgs.preventDefault();
+            this.grid.filteringService.scrollToFilterCell(this.grid.filteringService.unpinnedFilterableColumns[prevIndex], false);


You can use the 'cols' variable here as well

projects/igniteui-angular/src/lib/grids/grid-navigation.service.ts

sstoyanovIG · 2019-03-12T08:59:26Z

projects/igniteui-angular/src/lib/grids/grid-navigation.service.ts

+        return;
+    }
+
+    if (nextIndex >= this.grid.filteringService.unpinnedFilterableColumns.length) {


You can use the 'cols' variable here.

sstoyanovIG · 2019-03-12T08:59:39Z

projects/igniteui-angular/src/lib/grids/grid-navigation.service.ts

+        eventArgs.preventDefault();
+    } else if (!column.pinned && !this.isColumnFullyVisible(nextIndex)) {
+        eventArgs.preventDefault();
+        this.grid.filteringService.scrollToFilterCell(this.grid.filteringService.unpinnedFilterableColumns[nextIndex], true);


You can use the 'cols' variable here.

rkaraivanov · 2019-03-12T09:20:55Z

...cts/igniteui-angular/src/lib/grids/hierarchical-grid/hierarchical-grid-navigation.service.ts

@@ -1,6 +1,7 @@
 import { IgxGridNavigationService } from '../grid-navigation.service';
 import { IgxHierarchicalGridComponent } from './hierarchical-grid.component';
 import { first } from 'rxjs/operators';
+import { IgxColumnComponent, FilterMode } from '../grid';


Wrong import! Either import them from the respective files or explicitly import from the barrel file

rkaraivanov · 2019-03-12T09:27:00Z

projects/igniteui-angular/src/lib/grids/grid-navigation.service.ts

+
+    private getLastPinnedFilterableColumn(): IgxColumnComponent {
+        const pinnedFilterableColums =
+            this.grid.pinnedColumns.filter(col => !(col instanceof IgxColumnGroupComponent) && col.filterable);


Columns and column groups have a getter for this check: get columnGroup(): Boolean

rkaraivanov · 2019-03-12T09:27:39Z

projects/igniteui-angular/src/lib/grids/grid-navigation.service.ts

+    }
+
+    private getFirstPinnedFilterableColumn(): IgxColumnComponent {
+        return this.grid.pinnedColumns.filter(col => !(col instanceof IgxColumnGroupComponent) && col.filterable)[0];


rkaraivanov · 2019-03-12T09:30:06Z

...cts/igniteui-angular/src/lib/grids/hierarchical-grid/hierarchical-grid-navigation.service.ts

+        const isNextChild = nextIndex <= virt.igxForOf.length - 1 &&
+          this.grid.isChildGridRecord(virt.igxForOf[nextIndex]);
+          if (!this.grid.rowList.find(row => row.index === rowIndex + 1) && this.grid.parent &&
+          this.grid.unpinnedColumns[this.grid.unpinnedColumns.length - 1].visibleIndex === visibleColumnIndex) {


Is that not the same check as line 231?

rkaraivanov · 2019-03-12T09:31:44Z

projects/igniteui-angular/src/lib/grids/grid-navigation.service.ts

+    public navigateNextFilterCell(column: IgxColumnComponent, eventArgs) {
+        const cols = this.grid.filteringService.unpinnedFilterableColumns;
+        const nextIndex = cols.indexOf(column) + 1;
+        if (column === this.getLastPinnedFilterableColumn() &&


Please format this code

…ui-angular.git

skrustev · 2019-03-13T10:02:10Z

projects/igniteui-angular/src/lib/grids/grid-navigation.service.ts

+    public navigatePrevFilterCell(column: IgxColumnComponent, eventArgs) {
+        const cols = this.grid.filteringService.unpinnedFilterableColumns;
+        const prevFilterableIndex = cols.indexOf(column) - 1;
+        const prevIndex = cols[prevFilterableIndex].visibleIndex;


The prevFilterableIndex can become -1 if the index of the column is 0 which will result in error getting visibleIndex straight without a check.

skrustev · 2019-03-13T13:20:16Z

projects/igniteui-angular/src/lib/grids/grid-navigation.service.ts

+
+        if (prevIndex >= 0 && visibleIndex > 0 && !this.isColumnLeftFullyVisible(prevIndex) && !column.pinned) {
+            eventArgs.preventDefault();
+            this.grid.filteringService.scrollToFilterCell(cols[prevIndex], false);


If visible index is used for the prevIndex, for the unpinned filterable columns collection the index of the column may be different that the visible index.

skrustev · 2019-03-13T13:28:18Z

projects/igniteui-angular/src/lib/grids/grid-navigation.service.ts

+                this.goToFirstCell();
+            }
+            eventArgs.preventDefault();
+        } else if (!column.pinned && !this.isColumnFullyVisible(nextIndex)) {


isColumnFullyVisible() takes visible index for a column here as well.

skrustev · 2019-03-13T13:38:49Z

projects/igniteui-angular/src/lib/grids/grid-navigation.service.ts

+    public navigateNextFilterCell(column: IgxColumnComponent, eventArgs) {
+        const cols = this.grid.filteringService.unpinnedFilterableColumns;
+        const nextIndex = cols.indexOf(column) + 1;
+        if (column === this.getLastPinnedFilterableColumn() && !this.isColumnFullyVisible(nextIndex)) {


isColumnFullyVisible() takes visible index for a column

skrustev · 2019-03-13T13:39:47Z

projects/igniteui-angular/src/lib/grids/grid-navigation.service.ts

-        if (this.isColumnFullyVisible(columns.length - 1)) {
-            this.grid.filteringService.focusFilterCellChip(columns[columns.length - 1], false);
+        const targetIndex = toStart ? 0 : columns.length - 1;
+        const isVisible = toStart ? this.isColumnLeftFullyVisible(targetIndex) : this.isColumnFullyVisible(targetIndex);


isColumnFullyVisible() takes visible index and index from the unpinnedFilterableColumns collection wouldn't work in some scenarios.

…ng sure isColumnFullyVisible uses visibleIndex, while scrollToFilterCell uses index from the unpinnedFilterableColumns collection.

…llyVisible.

PavlovVasil · 2019-03-27T14:29:02Z

@MayaKirova @mpavlinov
I don't know if these should be logged as separate issues, or fixed with this PR:

I noticed that the Ctrl + Right Arrow is not working in an expanded child grid - it gets scrolled down and the selected cell get out of viewport:

Open http://localhost:4200/hierarchicalGrid
expand the first row - ID 0
Click the first cell in the child grid - ID 00
Press Ctrl + Right Arrow

A few more:

Run the igniteui-angular-samples and open:
http://localhost:4201/samples/hierarchical-grid/hierarchical-grid-filtering

When the child grid gets expanded and has Filtering enabled, when Tab is used to navigate through its filtering chips, the two red vertical lines on the right do not appear, until the first child cell gets reached - refer to the screenshot below. Either the vertical lines should appear when the first filtering chip in the child grid gets selected, or they should disappear if the user uses Shift + Tab from the first child cell and selects the last filtering chip (meaning chips would not trigger the vertical lines)?

When the expanded child grid has no data, using Tab scrolls the parent grid and the child is not visible: open the same Filtering sample: http://localhost:4201/samples/hierarchical-grid/hierarchical-grid-filtering . Expand the second to last record - Alicia Stanger and scroll down so that the child grid is into view. Then select the "Alicia Stanger" cell in the parent grid and press Tab several times - it should select the Filtering chips in the first row island. Once you get to the second one (with no data), the parent grid gets scrolled up and the child is out of view - see the attached gif:

MayaKirova · 2019-03-27T15:06:19Z

@PavlovVasil Are issues 1) and 3) related to this particular PR or are they also reproducible in branch 7.2.x?
Regarding 2) Since the filter cells are not selectable it is not expected for the red lines to appear as their purpose is to show the path to the currently selected (not focused) cell and selection is available only on data cells.

…not in view when moving from filter cell.

…ui-angular.git

MayaKirova · 2019-03-27T17:13:40Z

@PavlovVasil
Regarding 1) - Log as separate issue as it also reproduces in 7.2.x.
Regarding 2) - Expected behavior.
Regarding 3) - Fixed.

Please re-test.

MKirova added 2 commits March 8, 2019 18:32

fix(HierarchicalGrid): Refactoring filter cell navigation so that it …

e6ed866

…is handled in the navigation service. Hanling special scenarios for hierarchical grid in the hierarchical navigation service.

fix(HierarchicalGrid): Moving handling for sceanrios when child has f…

2069c97

…ilter cells in performTab since navigation should consider them only for Tab navigation (should skip them with arrow keys). Adding test.

MayaKirova added ❌ status: awaiting-test PRs awaiting manual verification keyboard-navigation filtering grid: hierarchical-grid version: 7.2.x labels Mar 11, 2019

MayaKirova assigned mpavlinov Mar 11, 2019

MayaKirova requested review from ChronosSF, bkulov, skrustev, mpavlinov and sstoyanovIG March 11, 2019 13:40

sstoyanovIG suggested changes Mar 11, 2019

View reviewed changes

chore(*): Storing and reusing cols collection.

801cf17

MayaKirova requested a review from sstoyanovIG March 11, 2019 16:17

MayaKirova mentioned this pull request Mar 12, 2019

Keyboard Navigation Issues in the Hierarchical Grid #4280

Closed

3 tasks

sstoyanovIG suggested changes Mar 12, 2019

View reviewed changes

rkaraivanov requested changes Mar 12, 2019

View reviewed changes

mpavlinov and others added 3 commits March 12, 2019 11:38

Merge branch '7.2.x' into mkirova/fix-4217

232f288

chore(*): Applying code review comments.

9b2f44d

Merge branch 'mkirova/fix-4217' of https://github.com/IgniteUI/ignite…

819893e

…ui-angular.git

MayaKirova requested a review from rkaraivanov March 12, 2019 15:47

Merge from 7.2.x

63850ee

skrustev requested changes Mar 13, 2019

View reviewed changes

skrustev reviewed Mar 13, 2019

View reviewed changes

skrustev requested changes Mar 13, 2019

View reviewed changes

MKirova added 2 commits March 14, 2019 14:32

chore(*): Moving checks whether next/prev filter cell exists and maki…

87ba131

…ng sure isColumnFullyVisible uses visibleIndex, while scrollToFilterCell uses index from the unpinnedFilterableColumns collection.

chore(*): Using visible index for isColumnLeftFullyVisible/isColumnFu…

920a087

…llyVisible.

skrustev previously approved these changes Mar 26, 2019

View reviewed changes

PavlovVasil added 💥 status: in-test PRs currently being tested and removed ❌ status: awaiting-test PRs awaiting manual verification labels Mar 26, 2019

Merge branch '7.2.x' into mkirova/fix-4217

3b0b1ec

PavlovVasil added 🛠️ status: in-development Issues and PRs with active development on them and removed 💥 status: in-test PRs currently being tested labels Mar 27, 2019

MKirova added 2 commits March 27, 2019 18:55

fix(HierarchicalGrid): Additional fix in case next child grid row is …

8e0de99

…not in view when moving from filter cell.

Merge branch 'mkirova/fix-4217' of https://github.com/IgniteUI/ignite…

02e234b

…ui-angular.git

MayaKirova dismissed stale reviews from skrustev and sstoyanovIG via 02e234b March 27, 2019 16:56

MayaKirova added ❌ status: awaiting-test PRs awaiting manual verification and removed 🛠️ status: in-development Issues and PRs with active development on them labels Mar 27, 2019

Merge branch '7.2.x' into mkirova/fix-4217

b939adb

PavlovVasil added 💥 status: in-test PRs currently being tested ✅ status: verified Applies to PRs that have passed manual verification and removed ❌ status: awaiting-test PRs awaiting manual verification 💥 status: in-test PRs currently being tested labels Mar 28, 2019

bkulov removed their request for review March 28, 2019 12:39

mpavlinov and others added 4 commits March 28, 2019 15:32

Merge branch '7.2.x' into mkirova/fix-4217

ef225f2

Merge branch '7.2.x' into mkirova/fix-4217

be06b88

Merge branch '7.2.x' into mkirova/fix-4217

120014e

Merge branch '7.2.x' into mkirova/fix-4217

22d1407

rkaraivanov approved these changes Apr 1, 2019

View reviewed changes

ChronosSF approved these changes Apr 1, 2019

View reviewed changes

rkaraivanov approved these changes Apr 1, 2019

View reviewed changes

mpavlinov merged commit f171ea7 into 7.2.x Apr 1, 2019

mpavlinov deleted the mkirova/fix-4217 branch April 1, 2019 11:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactoring filter cell navigation so that it is handled in the navigation service. Handling special scenarios for hierarchical grid in the hierarchical navigation service. #4267

Refactoring filter cell navigation so that it is handled in the navigation service. Handling special scenarios for hierarchical grid in the hierarchical navigation service. #4267

MayaKirova commented Mar 11, 2019 •

edited by skrustev

sstoyanovIG Mar 11, 2019

sstoyanovIG Mar 11, 2019

sstoyanovIG Mar 12, 2019

sstoyanovIG Mar 12, 2019

rkaraivanov Mar 12, 2019

rkaraivanov Mar 12, 2019

rkaraivanov Mar 12, 2019

rkaraivanov Mar 12, 2019

rkaraivanov Mar 12, 2019

skrustev Mar 13, 2019 •

edited

skrustev Mar 13, 2019 •

edited

skrustev Mar 13, 2019 •

edited

skrustev Mar 13, 2019

skrustev Mar 13, 2019

PavlovVasil commented Mar 27, 2019

MayaKirova commented Mar 27, 2019

MayaKirova commented Mar 27, 2019

Refactoring filter cell navigation so that it is handled in the navigation service. Handling special scenarios for hierarchical grid in the hierarchical navigation service. #4267

Refactoring filter cell navigation so that it is handled in the navigation service. Handling special scenarios for hierarchical grid in the hierarchical navigation service. #4267

Conversation

MayaKirova commented Mar 11, 2019 • edited by skrustev

Additional information (check all that apply):

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skrustev Mar 13, 2019 • edited

Choose a reason for hiding this comment

skrustev Mar 13, 2019 • edited

Choose a reason for hiding this comment

skrustev Mar 13, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PavlovVasil commented Mar 27, 2019

MayaKirova commented Mar 27, 2019

MayaKirova commented Mar 27, 2019

MayaKirova commented Mar 11, 2019 •

edited by skrustev

skrustev Mar 13, 2019 •

edited

skrustev Mar 13, 2019 •

edited

skrustev Mar 13, 2019 •

edited