Pull request 67: Fix column selection features and button theme

Completed

!67

Dobromir Tsvetkov

proposes to merge into master from dTsvetkov/fix-column-selection-features

0/5 comments resolved

Overview Files Updates Commits

Zdravko Kolev completed this pull request Feb 28

Show details

No merge conflictsLast checked Feb 28

Description

Fix column selection features and button theme

igx-context-menu.directive.ts/projects/igniteui-angular-extras/src/lib/context-menu/igx-context-menu.directive.ts

Mar 5

Hristo Anastasov

Mar 5

Everything that modifies this._analyticsBtnSetting, along withe the assinging of new auto strategy in beginning of method, makes sense to live in a separate method that is known to modify this, instead of living under renderHeaderButton method which does more than one thing.

igx-context-menu.directive.ts/projects/igniteui-angular-extras/src/lib/context-menu/igx-context-menu.directive.ts

Mar 5

Hristo Anastasov

Mar 5

Export this in a method, i.e. "getNewAutoPositionStrategy()" for example

igx-context-menu.directive.ts/projects/igniteui-angular-extras/src/lib/context-menu/igx-context-menu.directive.ts

Mar 5

Hristo Anastasov

Mar 5

Suggested change

import { Pipe, PipeTransform } from '@angular/core';

import { DomSanitizer, SafeHtml } from '@angular/platform-browser';

@Pipe({

name: 'svg'

name: 'svg',

})

export class SvgPipe implements PipeTransform {

constructor(private sanitizer: DomSanitizer) {}

transform(markup: string): SafeHtml {

return this.sanitizer.bypassSecurityTrustHtml(markup);

}

igx-context-menu.directive.ts/projects/igniteui-angular-extras/src/lib/context-menu/igx-context-menu.directive.ts

Mar 5

Hristo Anastasov

Mar 5

First the columns are iterated to get the indexes, then the index is used to iterate the columns again and find the column.

This whole thing can be improved to iterate only over the columns, until finding the last visible column. And assigning the last visible column to a col constant can be moved just before the break. I think this would make the code less verbose and easier to maintain:

while (selectedColumns.length) {
    // get last column
    if (this.grid.navigation.isColumnFullyVisible(lastCol.index)) {
        col = lastCol.col;
        break;
    }
    selectedColumns.pop()
}

if (!col) {
    return;
}

// continue

igx-context-menu.directive.ts/projects/igniteui-angular-extras/src/lib/context-menu/igx-context-menu.directive.ts

Mar 5

Hristo Anastasov

Mar 5

Please add a comment why the debounce time is needed.

Zdravko Kolev completed the pull request

Feb 28

Zdravko Kolev approved the pull request

Feb 28

Zdravko Kolev joined as a reviewer

Feb 28

Dobromir Tsvetkov pushed 1 commit

Feb 28

Dobromir Tsvetkov created the pull request

Feb 28

Reviewers

Required

No required reviewers

Optional

Zdravko Kolev

Approved

Activity feed