igxGrid - refactor header and filter cell components 6.2.x #3017

SAndreeva · 2018-11-14T10:33:04Z

Closes #2972, Closes #2926, Closes #2923, Closes #2917, Closes #2783, Closes #3027, Closes #2938.

This PR introduces the following changes that are with high regression potential:

a new component (header group) to accommodate the header and filtering cell components;
all column moving and column resizing related DOM structures are moved to the new header group component;
MCH are hierarchical structure, comprising of these header group components, only the deepest groups should have a header and a filter cell;
the old header component is cleaned up, it takes care only for sorting;
column resizing base logic is moved to a separate service;

# Conflicts: # projects/igniteui-angular/src/lib/grids/column.component.ts # projects/igniteui-angular/src/lib/grids/grid-header.component.ts # projects/igniteui-angular/src/lib/grids/grid/grid.component.html # projects/igniteui-angular/src/lib/grids/grid/grid.component.ts # projects/igniteui-angular/src/lib/grids/tree-grid/tree-grid.component.html

# Conflicts: # projects/igniteui-angular/src/lib/grids/column.component.ts # projects/igniteui-angular/src/lib/grids/grid-base.component.ts # projects/igniteui-angular/src/lib/grids/grid-header-group.component.html # projects/igniteui-angular/src/lib/grids/grid-header-group.component.ts # projects/igniteui-angular/src/lib/grids/grid/grid.component.html # projects/igniteui-angular/src/lib/grids/grid/grid.component.ts # projects/igniteui-angular/src/lib/grids/tree-grid/tree-grid.component.html

bkulov · 2018-11-14T13:00:31Z

projects/igniteui-angular/src/lib/grids/column.component.ts

 import { IgxGridHeaderComponent } from './grid-header.component';
-import { valToPxlsUsingRange } from '../core/utils';
 import { DefaultSortingStrategy, ISortingStrategy } from '../data-operations/sorting-strategy';
+import { valToPxlsUsingRange, flatten } from '../core/utils';


valToPxlsUsingRange name sounds weird. Could we change it to something like getNodeSizeViaRange?

bkulov · 2018-11-21T13:46:40Z

projects/igniteui-angular/src/lib/directives/dragdrop/dragdrop.directive.ts

     */
-    protected createDragGhost(event) {
-        this._dragGhost = this.element.nativeElement.cloneNode(true);
+    protected createDragGhost(event, node = null) {


You might define the type of the node so if someone needs to use this method to be aware of the type.

bkulov · 2018-11-22T07:36:33Z

projects/igniteui-angular/src/lib/grids/grid-base.component.ts

+    /**
+     * @hidden
+     */
+    get headerCheckboxWidth() {


Not all paths return value.

bkulov · 2018-11-22T07:44:38Z

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

+    public resizeEndTimeout = isFirefox() ? 200 : 0;
+    public column: IgxColumnComponent;
+
+    private pinnedMaxWidth;


Put private members before the public ones.

Add type to this member.

bkulov · 2018-11-22T07:46:34Z

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

+
+    public startResizePos: number;
+    public isColumnResizing: boolean;
+    public resizeCursor: any = null;


I suppose this could be 'string' instead of 'any'.

bkulov · 2018-11-22T07:48:30Z

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

+        }
+    }
+
+    public autosizeColumnOnDblClick() {


You might document the public members (methods, properties) even though this service is for internal use only.

bkulov · 2018-11-22T07:49:53Z

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

+        }
+    }
+
+    public resizeColumn(event) {


Put a type on the argument.

bkulov · 2018-11-22T07:55:46Z

projects/igniteui-angular/src/lib/grids/grid-header-group.component.ts

+    selector: 'igx-grid-header-group',
+    templateUrl: './grid-header-group.component.html'
+})
+export class IgxGridHeaderGroupComponent implements DoCheck {


You might document all public members even thought this class is excluded from typedoc generation.

bkulov · 2018-11-22T07:56:47Z

projects/igniteui-angular/src/lib/grids/grid-header-group.component.ts

+            'igx-grid__th--filtering': this.isFiltered
+        };
+
+        Object.entries(classList).forEach(([klass, value]) => {


klass should be class.

bkulov · 2018-11-22T07:57:45Z

projects/igniteui-angular/src/lib/grids/grid-header-group.component.ts

+        if (!this.column.pinned) {
+            return null;
+        }
+        return 9999 - this.grid.pinnedColumns.indexOf(this.column);


The magic number (9999) should be defined as constant in the class.

rkaraivanov · 2018-11-22T12:34:52Z

projects/igniteui-angular/src/lib/grids/filtering/grid-filtering-cell.component.ts

        if (this.moreIcon) {
            this.filteringService.columnToMoreIconHidden.set(this.column.field, true);
        }
+        this.cdr.detectChanges();


Do we need this extra change detection cycle here?

Yes. The idea is to initially show all chips, measure them and hide the ones which don't fit.

Related issues: #542, #2938, #2995

SAndreeva added 6 commits November 14, 2018 12:01

feat(filtering): some MCH related improvements #542

a96cd3f

feat(filtering): fix some minor issues #542

c1298b0

test(*): start fixing failing tests for MCH #542

419561a

feat(filtering): fix build error #542

1b1acf8

SAndreeva added 🛠️ status: in-development Issues and PRs with active development on them grid: general version: 6.2.x labels Nov 14, 2018

SAndreeva assigned npaunov Nov 14, 2018

SAndreeva requested review from bkulov and sstoyanovIG November 14, 2018 10:33

bkulov requested a review from rkaraivanov November 14, 2018 12:42

Merge branch '6.2.x' into SAndreeva/header-group-component-6.2.x

b953461

bkulov reviewed Nov 14, 2018

View reviewed changes

SAndreeva and others added 15 commits November 14, 2018 15:55

Merge branch '6.2.x' into SAndreeva/header-group-component-6.2.x

4daf8d8

chore(filtering): Small refactoring (#2938)

3e47a1a

test(*): refactor failing tests #542

67751be

feat(filtering): CD strategy for header group component #542

ad4e781

feat(filtering): rework column moving dragGhost creation #542

1de81f5

feat(filtering): minor fixes and improvements #542

06f58f3

Merge branch '6.2.x' into SAndreeva/header-group-component-6.2.x

ef196bd

chore(filtering): Change filtering components strategy to OnPush (#2938)

334c1a2

test(filtering): Fix failing tests (#2938)

0aec11c

test(filtering): Fix filtering UI tests (#2938)

e1f3c4c

test(filtering): filtering UI tests refactoring #542

f100af8

Merge branch '6.2.x' into SAndreeva/header-group-component-6.2.x

4533778

test(filtering): fix timer issues in filtering UI test #542

9a4bdba

fix(*): sorting icon not being updated when sort/groupBy via API #542

611e93b

Merge branch '6.2.x' into SAndreeva/header-group-component-6.2.x

e68e4db

feat(filtering): fix broken behaviors #542

79cafbb

SAndreeva added 💥 status: in-test PRs currently being tested and removed 🛠️ status: in-development Issues and PRs with active development on them labels Nov 21, 2018

chore(*): some clean up, etc. #542

878e2eb

npaunov added ✅ status: verified Applies to PRs that have passed manual verification and removed 💥 status: in-test PRs currently being tested labels Nov 21, 2018

kdinev added 3 commits November 21, 2018 16:12

Merge branch '6.2.x' into SAndreeva/header-group-component-6.2.x

5f3b310

Merge branch '6.2.x' into SAndreeva/header-group-component-6.2.x

d14657b

Merge branch '6.2.x' into SAndreeva/header-group-component-6.2.x

5703000

bkulov suggested changes Nov 22, 2018

View reviewed changes

SAndreeva and others added 4 commits November 22, 2018 11:45

feat(filtering): some API refinements #542

8555ba9

Merge branch '6.2.x' into SAndreeva/header-group-component-6.2.x

e45fb70

Merge branch '6.2.x' into SAndreeva/header-group-component-6.2.x

678a352

Merge branch '6.2.x' into SAndreeva/header-group-component-6.2.x

d520736

bkulov previously approved these changes Nov 22, 2018

View reviewed changes

rkaraivanov requested changes Nov 22, 2018

View reviewed changes

rkaraivanov previously approved these changes Nov 22, 2018

View reviewed changes

bkulov and others added 2 commits November 22, 2018 14:52

Merge branch '6.2.x' into SAndreeva/header-group-component-6.2.x

e1fda82

fix(filtering): Fix async issue in test (#2995)

67910c3

bkulov dismissed stale reviews from rkaraivanov and themself via 67910c3 November 22, 2018 15:26

bkulov previously approved these changes Nov 22, 2018

View reviewed changes

bkulov and others added 2 commits November 23, 2018 08:01

Merge branch '6.2.x' into SAndreeva/header-group-component-6.2.x

d003b54

fix(filtering): Fix empty bool expected result (#2938)

6f92a55

bkulov dismissed their stale review via 6f92a55 November 23, 2018 07:47

bkulov approved these changes Nov 23, 2018

View reviewed changes

sstoyanovIG approved these changes Nov 23, 2018

View reviewed changes

kdinev merged commit 00a73cf into 6.2.x Nov 23, 2018

kdinev deleted the SAndreeva/header-group-component-6.2.x branch November 23, 2018 08:17

bkulov pushed a commit that referenced this pull request Nov 26, 2018

refactor(filtering): Refactor header cell component (#3017)

7fb02a4

Related issues: #542, #2938, #2995

igxGrid - refactor header and filter cell components 6.2.x #3017

igxGrid - refactor header and filter cell components 6.2.x #3017

Uh oh!

Conversation

SAndreeva commented Nov 14, 2018 • edited by kdinev Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bkulov Nov 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

SAndreeva commented Nov 14, 2018 •

edited by kdinev

Loading

bkulov Nov 22, 2018 •

edited

Loading