From 66864b8f549da505b9ecd309385e1ad66935cfe5 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Tue, 16 Mar 2021 14:54:53 -0700 Subject: [PATCH] Implement reversing the data source Summary: For context see https://fb.workplace.com/notes/470523670998369 This diff adds support for reversing the data collection (in a table this would be used to toggle between ascending and descending sorting). The actual implementation is cleaned up in next diffs and the intermediate collection introduced here is dropped, so this diff is basically only about the unit tests, the implementation is not interesting at this point. Reviewed By: nikoant Differential Revision: D25975353 fbshipit-source-id: 2da6da2ed940c2e49e1986696d9b93a7b984db9b --- .../src/state/datasource/DataSource.tsx | 100 ++++++++++++++---- .../__tests__/datasource-basics.node.tsx | 90 ++++++++++++++++ 2 files changed, 172 insertions(+), 18 deletions(-) diff --git a/desktop/flipper-plugin/src/state/datasource/DataSource.tsx b/desktop/flipper-plugin/src/state/datasource/DataSource.tsx index 18f9a2197..f2f57b502 100644 --- a/desktop/flipper-plugin/src/state/datasource/DataSource.tsx +++ b/desktop/flipper-plugin/src/state/datasource/DataSource.tsx @@ -40,13 +40,17 @@ class DataSource< private _recordsById: Map = new Map(); private keyAttribute: undefined | keyof T; private idToIndex: Map = new Map(); - dataUpdateQueue: DataEvent[] = []; + private dataUpdateQueue: DataEvent[] = []; private sortBy: undefined | ((a: T) => number | string); private _sortedRecords: T[] | undefined; - viewRecords: T[] = []; - nextViewRecords: T[] = []; // for double buffering + private reverse: boolean = false; + private _reversedRecords: T[] | undefined; + + // TODO: + // private viewRecords: T[] = []; + // private nextViewRecords: T[] = []; // for double buffering /** * Returns a direct reference to the stored records. @@ -78,6 +82,14 @@ class DataSource< return this.sortBy ? this._sortedRecords! : this._records; } + /** + * Exposed for testing only. + * Returns the set of records after applying sorting and reversing (if applicable) + */ + get reversedRecords(): readonly T[] { + return this.reverse ? this._reversedRecords! : this.sortedRecords; + } + constructor(keyAttribute: KEY | undefined) { this.keyAttribute = keyAttribute; this.setSortBy(undefined); @@ -197,6 +209,26 @@ class DataSource< this.insertSorted(value); }); } + // TODO: clean up to something easier to follow + if (this.reverse) { + this.toggleReversed(); // reset + this.toggleReversed(); // reapply + } + } + + toggleReversed() { + this.setReversed(!this.reverse); + } + + setReversed(reverse: boolean) { + if (this.reverse !== reverse) { + this.reverse = reverse; + if (reverse) { + this._reversedRecords = this.sortedRecords.slice().reverse(); + } else { + this._reversedRecords = undefined; + } + } } emitDataEvent(event: DataEvent) { @@ -212,39 +244,70 @@ class DataSource< processEvent = (event: DataEvent) => { const {value} = event; + const {_sortedRecords, _reversedRecords} = this; switch (event.type) { - case 'append': + case 'append': { + let insertionIndex = this._records.length - 1; // sort - if (this.sortBy) { - this.insertSorted(value); + if (_sortedRecords) { + insertionIndex = this.insertSorted(value); + } + // reverse append + if (_reversedRecords) { + _reversedRecords.splice( + _reversedRecords.length - insertionIndex, // N.b. no -1, since we're appending + 0, + value, + ); } - // reverse // filter // notify break; + } case 'update': // sort - if (this.sortBy) { + if (_sortedRecords) { // find old entry const oldIndex = this.getSortedIndex(event.oldValue); - if ( - this.sortBy(this._sortedRecords![oldIndex]) === this.sortBy(value) - ) { + if (this.sortBy!(_sortedRecords[oldIndex]) === this.sortBy!(value)) { // sort value is the same? just swap the item this._sortedRecords![oldIndex] = value; + if (_reversedRecords) { + _reversedRecords[ + _reversedRecords.length - 1 - event.index + ] = value; + } } else { // sort value is different? remove and add this._sortedRecords!.splice(oldIndex, 1); - this.insertSorted(value); + if (_reversedRecords) { + _reversedRecords.splice( + _reversedRecords.length - 1 - oldIndex, + 1, + ); + } + const insertionIndex = this.insertSorted(value); + if (_reversedRecords) { + _reversedRecords.splice( + _reversedRecords.length - insertionIndex, + 0, + value, + ); // N.b. no -1, since we're appending + } } - // reverse - - // filter - - // notify } + // reverse + else if (_reversedRecords) { + // only handle reverse separately if not sorting, otherwise handled above + _reversedRecords[_reversedRecords.length - 1 - event.index] = value; + } + + // filter + + // notify + break; default: throw new Error('unknown event type'); @@ -263,13 +326,14 @@ class DataSource< return index; } - private insertSorted(value: T) { + private insertSorted(value: T): number { const insertionIndex = sortedLastIndexBy( this._sortedRecords, value, this.sortBy!, ); this._sortedRecords!.splice(insertionIndex, 0, value); + return insertionIndex; } } diff --git a/desktop/flipper-plugin/src/state/datasource/__tests__/datasource-basics.node.tsx b/desktop/flipper-plugin/src/state/datasource/__tests__/datasource-basics.node.tsx index 60d338fbf..ecc7dab98 100644 --- a/desktop/flipper-plugin/src/state/datasource/__tests__/datasource-basics.node.tsx +++ b/desktop/flipper-plugin/src/state/datasource/__tests__/datasource-basics.node.tsx @@ -165,3 +165,93 @@ test('sorting preserves insertion order with equal keys', () => { expect(ds.records).toEqual([b1, c, b2r, a, b3r, b4]); expect(ds.sortedRecords).toEqual([a, b3r, b1, b2r, b4, c]); }); + +test('reverse without sorting', () => { + const ds = createDataSource([eatCookie, drinkCoffee]); + expect(ds.reversedRecords).toEqual([eatCookie, drinkCoffee]); + + ds.toggleReversed(); + expect(ds.reversedRecords).toEqual([drinkCoffee, eatCookie]); + + ds.append(submitBug); + expect(ds.records).toEqual([eatCookie, drinkCoffee, submitBug]); + expect(ds.reversedRecords).toEqual([submitBug, drinkCoffee, eatCookie]); + + const x = {id: 'x', title: 'x'}; + ds.update(0, x); + expect(ds.records).toEqual([x, drinkCoffee, submitBug]); + expect(ds.reversedRecords).toEqual([submitBug, drinkCoffee, x]); + const y = {id: 'y', title: 'y'}; + const z = {id: 'z', title: 'z'}; + ds.update(2, z); + ds.update(1, y); + + expect(ds.records).toEqual([x, y, z]); + expect(ds.reversedRecords).toEqual([z, y, x]); + + ds.setReversed(false); + expect(ds.reversedRecords).toEqual([x, y, z]); +}); + +test('reverse with sorting', () => { + type N = { + $: string; + name: string; + }; + + const a = {$: 'a', name: 'a'}; + const b1 = {$: 'b', name: 'b1'}; + const b2 = {$: 'b', name: 'b2'}; + const b3 = {$: 'b', name: 'b3'}; + const c = {$: 'c', name: 'c'}; + + const ds = createDataSource([]); + ds.setReversed(true); + ds.append(b1); + ds.append(c); + expect(ds.sortedRecords).toEqual([b1, c]); + expect(ds.reversedRecords).toEqual([c, b1]); + + ds.setSortBy('$'); + expect(ds.sortedRecords).toEqual([b1, c]); + expect(ds.reversedRecords).toEqual([c, b1]); + + ds.append(b2); + expect(ds.sortedRecords).toEqual([b1, b2, c]); + expect(ds.reversedRecords).toEqual([c, b2, b1]); + + ds.append(a); + expect(ds.sortedRecords).toEqual([a, b1, b2, c]); + expect(ds.reversedRecords).toEqual([c, b2, b1, a]); + + ds.append(b3); + expect(ds.sortedRecords).toEqual([a, b1, b2, b3, c]); + expect(ds.reversedRecords).toEqual([c, b3, b2, b1, a]); + + // if we append a new item with existig item, it should end up in the end + const b4 = { + $: 'b', + name: 'b4', + }; + ds.append(b4); + expect(ds.sortedRecords).toEqual([a, b1, b2, b3, b4, c]); + expect(ds.reversedRecords).toEqual([c, b4, b3, b2, b1, a]); + + // if we replace the middle item, it should end up in the middle + const b2r = { + $: 'b', + name: 'b2replacement', + }; + ds.update(2, b2r); + expect(ds.sortedRecords).toEqual([a, b1, b2r, b3, b4, c]); + expect(ds.reversedRecords).toEqual([c, b4, b3, b2r, b1, a]); + + // if we replace something with a different sort value, it should be sorted properly, and the old should disappear + const b3r = { + $: 'aa', + name: 'b3replacement', + }; + ds.update(4, b3r); + expect(ds.sortedRecords).toEqual([a, b3r, b1, b2r, b4, c]); + expect(ds.reversedRecords).toEqual([c, b4, b2r, b1, b3r, a]); +});