diff --git a/package-lock.json b/package-lock.json index 92cdcc57ec..88d88379a1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "clr-all", - "version": "0.11.20", + "version": "0.11.21", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/src/app/datagrid/full/full.html b/src/app/datagrid/full/full.html index 93ef859eb8..7ea80c1197 100644 --- a/src/app/datagrid/full/full.html +++ b/src/app/datagrid/full/full.html @@ -77,7 +77,7 @@

Full Datagrid demo

Multi-line text No users found - + {{user.id}} {{user.name}} {{user.creation | date}} @@ -115,7 +115,7 @@

Full Datagrid demo

Multi-line text No users found - + {{user.id}} {{user.name}} {{user.creation | date}} diff --git a/src/app/datagrid/full/full.ts b/src/app/datagrid/full/full.ts index a5929cb8b7..0ad2fb1874 100644 --- a/src/app/datagrid/full/full.ts +++ b/src/app/datagrid/full/full.ts @@ -11,12 +11,11 @@ import {ColorFilter} from "../utils/color-filter"; import {PokemonComparator} from "../utils/pokemon-comparator"; import {PokemonFilter} from "../utils/pokemon-filter"; - @Component({ selector: "clr-datagrid-full-demo", providers: [Inventory], templateUrl: "./full.html", - styleUrls: ["../datagrid.demo.scss"] + styleUrls: ["../datagrid.demo.scss"], }) export class DatagridFullDemo { options = { @@ -82,13 +81,20 @@ export class DatagridFullDemo { if (filter instanceof ColorFilter) { filters.color = (filter).listSelected(); } else { - const {property, value} = <{property: string, value: string}>filter; + const {property, value} = < { + property: string; + value: string + } + > filter; filters[property] = [value]; } } } this.inventory.filter(filters) - .sort(<{by: string, reverse: boolean}>state.sort) + .sort(<{ + by: string; + reverse: boolean + }>state.sort) .fetch(state.page && state.page.from, state.page && state.page.size) .then((result: FetchResult) => { this.users = result.users; @@ -96,4 +102,8 @@ export class DatagridFullDemo { this.loading = false; }); } + + trackById(idx, item) { + return item.id; + } } diff --git a/src/clr-angular/data/datagrid/datagrid.spec.ts b/src/clr-angular/data/datagrid/datagrid.spec.ts index f1292d3bd8..8a54c84865 100644 --- a/src/clr-angular/data/datagrid/datagrid.spec.ts +++ b/src/clr-angular/data/datagrid/datagrid.spec.ts @@ -36,7 +36,7 @@ export default function(): void { it("allows to manually force a refresh of displayed items when data mutates", function() { const items: Items = context.getClarityProvider(Items); let refreshed = false; - items.change.subscribe(() => refreshed = true); + items.change.subscribe(() => (refreshed = true)); expect(refreshed).toBe(false); context.clarityDirective.dataChanged(); expect(refreshed).toBe(true); @@ -155,7 +155,7 @@ export default function(): void { by: comparator, reverse: false, }, - filters: [filter] + filters: [filter], }); }); @@ -171,7 +171,9 @@ export default function(): void { filters.add(builtinStringFilter); context.detectChanges(); expect(context.testComponent.latestState.filters).toEqual([ - customFilter, testStringFilter, {property: "test", value: "1234"} + customFilter, + testStringFilter, + {property: "test", value: "1234"}, ]); }); @@ -299,7 +301,7 @@ export default function(): void { describe("Template API", function() { it("sets the currentSingle binding", function() { - expect(selection.currentSingle).toBeNull(); + expect(selection.currentSingle).toBeUndefined(); context.testComponent.selected = 1; context.detectChanges(); expect(selection.currentSingle).toEqual(1); @@ -308,8 +310,68 @@ export default function(): void { expect(selection.currentSingle).toBeNull(); }); + it("does not emit a change event for on initialization, before selection", function() { + let singleSelectedchangeCount: number = 0; + const sub = + context.clarityDirective.singleSelectedChanged.subscribe(s => singleSelectedchangeCount++); + + expect(selection.currentSingle).toBeUndefined(); + expect(singleSelectedchangeCount).toEqual(0); + + sub.unsubscribe(); + }); + + it("it emits a change event when changing the selection", function() { + let singleSelectedchangeCount: number = 0; + const sub = + context.clarityDirective.singleSelectedChanged.subscribe(s => singleSelectedchangeCount++); + + context.testComponent.selected = 1; + context.detectChanges(); + expect(selection.currentSingle).toEqual(1); + expect(singleSelectedchangeCount).toEqual(1); + + sub.unsubscribe(); + }); + + it("it does not emit a change event when setting selection to undefined/null if already undefined/null", + function() { + let singleSelectedchangeCount: number = 0; + const sub = + context.clarityDirective.singleSelectedChanged.subscribe(s => singleSelectedchangeCount++); + + expect(selection.currentSingle).toBeUndefined(); + expect(singleSelectedchangeCount).toEqual(0); + + context.testComponent.selected = null; + context.detectChanges(); + expect(selection.currentSingle).toBeUndefined(); + expect(singleSelectedchangeCount).toEqual(0); + + sub.unsubscribe(); + }); + + it("it does not emit a change event when selecting the same value", function() { + let singleSelectedchangeCount: number = 0; + const sub = + context.clarityDirective.singleSelectedChanged.subscribe(s => singleSelectedchangeCount++); + + context.testComponent.selected = 1; + context.detectChanges(); + expect(selection.currentSingle).toEqual(1); + expect(singleSelectedchangeCount).toEqual(1); + + // re-assigning to the same value should not increase the singleSelectedchangeCount + context.testComponent.selected = 1; + context.detectChanges(); + expect(selection.currentSingle).toEqual(1); + expect(singleSelectedchangeCount).toEqual(1); + + sub.unsubscribe(); + }); + it("offers two way binding on the currentSingle value", function() { - expect(selection.currentSingle).toBeNull(); + expect(selection.currentSingle).toBeUndefined(); context.testComponent.selected = 1; context.detectChanges(); expect(selection.currentSingle).toEqual(1); @@ -417,7 +479,7 @@ export default function(): void { {{items.length}} items -` +`, }) class FullTest { items = [1, 2, 3]; @@ -456,7 +518,7 @@ class FullTest { {{items.length}} items -` +`, }) class NgForTest { items = [1, 2, 3]; @@ -475,7 +537,7 @@ class NgForTest { {{items.length}} items -` +`, }) class TrackByTest { items = [1, 2, 3]; @@ -490,7 +552,7 @@ class TrackByTest { @Component({ template: ` - ` + `, }) class OnPushTest { items = [1, 2, 3]; @@ -509,7 +571,7 @@ class OnPushTest { {{item * item}} `, - changeDetection: ChangeDetectionStrategy.OnPush + changeDetection: ChangeDetectionStrategy.OnPush, }) class MultiSelectionTest { @Input() items: any[] = []; @@ -529,7 +591,7 @@ class MultiSelectionTest { {{selected}} -` +`, }) class SingleSelectionTest { items = [1, 2, 3]; @@ -554,7 +616,7 @@ class SingleSelectionTest { {{items.length}} items -` +`, }) class ActionableRowTest { items = [1, 2, 3]; @@ -577,14 +639,13 @@ class ActionableRowTest { {{items.length}} items -` +`, }) class ExpandableRowTest { items = [1, 2, 3]; expandable = true; } - @Component({ template: ` @@ -604,7 +665,7 @@ class ExpandableRowTest { {{items.length}} items - ` + `, }) class ChocolateClrDgItemsTest { items = [1, 2, 3]; @@ -631,7 +692,7 @@ class ChocolateClrDgItemsTest { {{items.length}} items - ` + `, }) class ChocolateNgForTest { items = [1, 2, 3]; @@ -663,7 +724,6 @@ class TestStringFilter implements ClrDatagridStringFilterInterface { } } - @Component({ selector: "hidden-column-test", template: ` @@ -679,7 +739,7 @@ class TestStringFilter implements ClrDatagridStringFilterInterface { {{item}} {{item * item}} - ` + `, }) class HiddenColumnTest { items = [1, 2, 3]; diff --git a/src/clr-angular/data/datagrid/datagrid.ts b/src/clr-angular/data/datagrid/datagrid.ts index cc501b1bc5..53d3c92cb3 100644 --- a/src/clr-angular/data/datagrid/datagrid.ts +++ b/src/clr-angular/data/datagrid/datagrid.ts @@ -13,7 +13,7 @@ import { Input, OnDestroy, Output, - QueryList + QueryList, } from "@angular/core"; import {Subscription} from "rxjs/Subscription"; @@ -52,7 +52,7 @@ import {DatagridRenderOrganizer} from "./render/render-organizer"; StateProvider, ColumnToggleButtonsService, ], - host: {"[class.datagrid-host]": "true"} + host: {"[class.datagrid-host]": "true"}, }) export class ClrDatagrid implements AfterContentInit, AfterViewInit, OnDestroy { constructor(private columnService: HideableColumnService, private organizer: DatagridRenderOrganizer, @@ -112,9 +112,12 @@ export class ClrDatagrid implements AfterContentInit, AfterViewInit, OnDestroy { @Input("clrDgSingleSelected") set singleSelected(value: any) { this.selection.selectionType = SelectionType.Single; + // the clrDgSingleSelected is updated in one of two cases: + // 1. an explicit value is passed + // 2. is being set to null or undefined, where previously it had a value if (value) { this.selection.currentSingle = value; - } else { + } else if (this.selection.currentSingle) { this.selection.currentSingle = null; } } diff --git a/src/clr-angular/data/datagrid/providers/selection.spec.ts b/src/clr-angular/data/datagrid/providers/selection.spec.ts index 37259fc5f6..2c0983f0f7 100644 --- a/src/clr-angular/data/datagrid/providers/selection.spec.ts +++ b/src/clr-angular/data/datagrid/providers/selection.spec.ts @@ -250,7 +250,18 @@ export default function(): void { }); describe("client-side selection and pagination", function() { - const items = [{id: 1}, {id: 2}, {id: 3}, {id: 4}, {id: 5}, {id: 6}, {id: 7}, {id: 8}, {id: 9}, {id: 10}]; + const items = [ + {id: 1}, + {id: 2}, + {id: 3}, + {id: 4}, + {id: 5}, + {id: 6}, + {id: 7}, + {id: 8}, + {id: 9}, + {id: 10}, + ]; function cloneItems() { return items.map(item => { @@ -313,6 +324,16 @@ export default function(): void { expect(selectionInstance.isSelected(items[5])).toBe(true); }); + it("should clear selection if it is no longer in dataset", fakeAsync(() => { + selectionInstance.currentSingle = items[2]; + pageInstance.current = 2; + expect(selectionInstance.isSelected(items[2])).toBe(true); + + itemsInstance.all = cloneItems().splice(2, 1); + tick(); + expect(selectionInstance.currentSingle).toBe(undefined); + })); + it("does not apply trackBy to single selection with no items", () => { const emptyItems = new Items(filtersInstance, sortInstance, pageInstance); const selection: Selection = new Selection(emptyItems, filtersInstance); diff --git a/src/clr-angular/data/datagrid/providers/selection.ts b/src/clr-angular/data/datagrid/providers/selection.ts index e5d3922419..300b7fc988 100644 --- a/src/clr-angular/data/datagrid/providers/selection.ts +++ b/src/clr-angular/data/datagrid/providers/selection.ts @@ -16,17 +16,17 @@ let nbSelection: number = 0; export enum SelectionType { None, Single, - Multi + Multi, } @Injectable() export class Selection { public id: string; - private selected: any[] = []; // Refs of selected items - private selectedSingle: any; // Ref of single selected item + private prevSelectionRefs: any[] = []; // Refs of selected items + private prevSingleSelectionRef: any; // Ref of single selected item constructor(private _items: Items, private _filters: FiltersProvider) { - this.id = "clr-dg-selection" + (nbSelection++); + this.id = "clr-dg-selection" + nbSelection++; this.subscriptions.push(this._filters.change.subscribe(() => { if (!this._selectable) { @@ -36,44 +36,83 @@ export class Selection { })); this.subscriptions.push(this._items.allChanges.subscribe(updatedItems => { - if (!this._selectable) { - return; - } - let leftOver: any[] = this.current.slice(); - let newSingle: any; + switch (this.selectionType) { + case SelectionType.None: { + break; + } - // Calculate the references for each item - const trackBy: TrackByFunction = this._items.trackBy; - const matched = []; - updatedItems.forEach((item, index) => { - const ref = trackBy(index, item); - // Look in current selected refs array if item is selected, and update actual value - if (this.selectedSingle === ref) { - newSingle = item; - } else if (this.selected.length) { - const selectedIndex = this.selected.indexOf(ref); - if (selectedIndex > -1) { - matched.push(selectedIndex); - leftOver[selectedIndex] = item; + case SelectionType.Single: { + let newSingle: any; + const trackBy: TrackByFunction = this._items.trackBy; + let selectionUpdated: boolean = false; + + updatedItems.forEach((item, index) => { + const ref = trackBy(index, item); + // If one of the updated items is the previously selectedSingle, set it as the new one + if (this.prevSingleSelectionRef === ref) { + newSingle = item; + selectionUpdated = true; + } + }); + + // Delete the currentSingle if it doesn't exist anymore if we're using smart datagrids + // where we expect all items to be present. + // No explicit "delete" is required, since it would still be undefined at this point. + // Marking it as selectionUpdated will emit the change when the currentSingle is updated below. + if (this._items.smart && !newSingle) { + selectionUpdated = true; } + + // TODO: Discussed this with Eudes and this is fine for now. + // But we need to figure out a different pattern for the + // child triggering the parent change detection problem. + // Using setTimeout for now to fix this. + setTimeout(() => { + if (selectionUpdated) { + this.currentSingle = newSingle; + } + }, 0); + break; } - }); - // Filter out any unmatched items if we're using smart datagrids where we expect all items to be present - if (this._items.smart) { - leftOver = leftOver.filter(selected => updatedItems.indexOf(selected) > -1); - } + case SelectionType.Multi: { + let leftOver: any[] = this.current.slice(); + const trackBy: TrackByFunction = this._items.trackBy; + let selectionUpdated: boolean = false; + + updatedItems.forEach((item, index) => { + const ref = trackBy(index, item); + // Look in current selected refs array if item is selected, and update actual value + const selectedIndex = this.prevSelectionRefs.indexOf(ref); + if (selectedIndex > -1) { + leftOver[selectedIndex] = item; + selectionUpdated = true; + } + }); + + // Filter out any unmatched items if we're using smart datagrids where we expect all items to be + // present + if (this._items.smart) { + leftOver = leftOver.filter(selected => updatedItems.indexOf(selected) > -1); + if (this.current.length !== leftOver.length) { + selectionUpdated = true; + } + } - // TODO: Discussed this with Eudes and this is fine for now. - // But we need to figure out a different pattern for the - // child triggering the parent change detection problem. - // Using setTimeout for now to fix this. - setTimeout(() => { - if (typeof newSingle !== "undefined") { - this.currentSingle = newSingle; + // TODO: Discussed this with Eudes and this is fine for now. + // But we need to figure out a different pattern for the + // child triggering the parent change detection problem. + // Using setTimeout for now to fix this. + setTimeout(() => { + if (selectionUpdated) { + this.current = leftOver; + } + }, 0); + break; } - this.current = leftOver; - }, 0); + + default: { break; } + } })); } @@ -101,7 +140,7 @@ export class Selection { public rowSelectionMode: boolean = false; private get _selectable(): boolean { - return (this._selectionType === SelectionType.Multi) || (this._selectionType === SelectionType.Single); + return this._selectionType === SelectionType.Multi || this._selectionType === SelectionType.Single; } /** * Ignore items changes in the same change detection cycle. @@ -113,7 +152,6 @@ export class Selection { */ private subscriptions: Subscription[] = []; - /** * Cleans up our subscriptions to other providers */ @@ -135,13 +173,13 @@ export class Selection { this._currentSingle = value; if (this._items.all && this._items.trackBy && value) { const lookup = this._items.all.findIndex(maybe => maybe === value); - this.selectedSingle = this._items.trackBy(lookup, value); + this.prevSingleSelectionRef = this._items.trackBy(lookup, value); } this.emitChange(); // Ignore items changes in the same change detection cycle. // @TODO This can likely be removed! this.debounce = true; - setTimeout(() => this.debounce = false); + setTimeout(() => (this.debounce = false)); } /** @@ -157,7 +195,7 @@ export class Selection { // Ignore items changes in the same change detection cycle. // @TODO This can likely be removed! this.debounce = true; - setTimeout(() => this.debounce = false); + setTimeout(() => (this.debounce = false)); } /** @@ -196,7 +234,7 @@ export class Selection { if (this._items.trackBy) { // Push selected ref onto array const lookup = this._items.all.findIndex(maybe => maybe === item); - this.selected.push(this._items.trackBy(lookup, item)); + this.prevSelectionRefs.push(this._items.trackBy(lookup, item)); } } @@ -205,9 +243,9 @@ export class Selection { */ private deselectItem(indexOfItem: number): void { this.current.splice(indexOfItem, 1); - if (this._items.trackBy && indexOfItem < this.selected.length) { + if (this._items.trackBy && indexOfItem < this.prevSelectionRefs.length) { // Keep selected refs array in sync - this.selected.splice(indexOfItem, 1); + this.prevSelectionRefs.splice(indexOfItem, 1); } } @@ -240,7 +278,7 @@ export class Selection { * Checks if all currently displayed items are selected */ public isAllSelected(): boolean { - if ((this._selectionType !== SelectionType.Multi) || !this._items.displayed) { + if (this._selectionType !== SelectionType.Multi || !this._items.displayed) { return false; } const displayedItems: any[] = this._items.displayed; @@ -264,7 +302,7 @@ export class Selection { * If at least one item isn't selected, we select every currently displayed item. */ if (this.isAllSelected()) { - this._items.displayed.forEach((item, displayIndex) => { + this._items.displayed.forEach(item => { const currentIndex = this.current.indexOf(item); if (currentIndex > -1) { this.deselectItem(currentIndex);