List view grid column change event #1897
Conversation
…. Also fixed issue where grids were not emitting their event at the correct time.
Codecov Report
@@ Coverage Diff @@
## master #1897 +/- ##
======================================
Coverage 100% 100%
======================================
Files 414 414
Lines 8664 8678 +14
Branches 1282 1285 +3
======================================
+ Hits 8664 8678 +14
Continue to review full report at Codecov.
|
|
Created #1906 to document new output property. |
| .map((result: AsyncList<SkyGridColumnModel>) => { | ||
| /* istanbul ignore next */ | ||
| /* sanity check */ | ||
| return result.items.map((column: SkyGridColumnModel) => { | ||
| return column.id || column.field; | ||
| }); | ||
| }).distinctUntilChanged(); | ||
| }).distinctUntilChanged((previousValue: string[], newValue: string[]) => { | ||
| if (previousValue.length !== newValue.length) { |
There was a problem hiding this comment.
To help with code readability, would you mind wrapping up this logic into a well named private method? Something like emitIfColumnsChanged() ?
There was a problem hiding this comment.
Done. I went with a slightly different name as it compares along with emitting but the extraction is done.
There was a problem hiding this comment.
Thanks. I like that a lot better! One suggestion - I'm still learning SKY UX conventions, but I don't see many instances of methods/properties prefixed with have. I wonder if something like columnIdsChanged would suffice?
| @@ -139,6 +139,9 @@ export class SkyGridComponent implements AfterContentInit, OnChanges, OnDestroy | |||
| this.setDisplayedColumns(true); | |||
| } else if (changes['selectedColumnIds'] && this.columns) { | |||
| this.setDisplayedColumns(); | |||
| if (changes['selectedColumnIds'].previousValue !== changes['selectedColumnIds'].currentValue) { | |||
| this.selectedColumnIdsChange.emit(this.selectedColumnIds); | |||
There was a problem hiding this comment.
This might emit twice if you're dragging a header (see line 228). Is there a way to ensure this only gets emitted once?
There was a problem hiding this comment.
Spoke to Alex offline. This second emit only happens for the list view grid due to how it loads the grid. However, the list view grid handles this case so we should be good to go here.
Blackbaud-AlexKingman
added
Status: Ready to merge
and removed
Status: Ready for review
labels
Blackbaud-AlexKingman
deleted the
1092-list-view-grid-column-change-event
branch
Resolves #1092