From 2fa844e9f16789c627123632c7ca461da317d695 Mon Sep 17 00:00:00 2001 From: James Kerr Date: Wed, 10 Feb 2021 09:55:09 -0800 Subject: [PATCH 1/7] Repro'd with a failing test --- src/js/state/LogDetails/test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/js/state/LogDetails/test.ts b/src/js/state/LogDetails/test.ts index 1b8fdb46e2..8055038fc7 100644 --- a/src/js/state/LogDetails/test.ts +++ b/src/js/state/LogDetails/test.ts @@ -9,6 +9,7 @@ const columns = [ const record = new zng.Record(columns, ["1", "a"]) const record2 = new zng.Record(columns, ["1", "b"]) +const record3 = new zng.Record(columns, ["1", "c"]) let store beforeEach(() => { @@ -55,6 +56,18 @@ test("going back and then forward", () => { expect(log && log.get("letter").toString()).toBe("b") }) +test("going back, then push, then back", () => { + const state = store.dispatchAll([ + LogDetails.push(record), + LogDetails.push(record2), + LogDetails.back(), + LogDetails.push(record3), + LogDetails.back() + ]) + + expect(LogDetails.build(state).get("letter")).toBe("a") +}) + test("updating the current log detail", () => { const state = store.dispatchAll([ LogDetails.push(record), From 794487c002749723bded49b495d727ba9e4486e2 Mon Sep 17 00:00:00 2001 From: James Kerr Date: Wed, 10 Feb 2021 10:14:37 -0800 Subject: [PATCH 2/7] Fixed history bug by splicing array at position --- src/js/models/LogDetailHistory.test.ts | 11 +++++++++++ src/js/models/LogDetailHistory.ts | 4 +++- src/js/state/LogDetails/test.ts | 6 +++++- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/js/models/LogDetailHistory.test.ts b/src/js/models/LogDetailHistory.test.ts index 161c738002..f123c9084a 100644 --- a/src/js/models/LogDetailHistory.test.ts +++ b/src/js/models/LogDetailHistory.test.ts @@ -79,3 +79,14 @@ test("getMostRecent when not empty", () => { expect(history.getMostRecent()).toBe("james") }) + +test("save, save, goBack, save, goBack", () => { + const history = new History() + history.save("a") + history.save("b") + history.goBack() + expect(history.getCurrent()).toBe("a") + history.save("c") + history.goBack() + expect(history.getCurrent()).toBe("a") +}) diff --git a/src/js/models/LogDetailHistory.ts b/src/js/models/LogDetailHistory.ts index 74b8f996b5..172656b882 100644 --- a/src/js/models/LogDetailHistory.ts +++ b/src/js/models/LogDetailHistory.ts @@ -13,7 +13,9 @@ export default class LogDetailHistory { } save(entry: T) { - this.entries.push(entry) + const length = this.entries.length + const index = length - this.position + this.entries.splice(index, length, entry) this.position = 0 } diff --git a/src/js/state/LogDetails/test.ts b/src/js/state/LogDetails/test.ts index 8055038fc7..6796f631f5 100644 --- a/src/js/state/LogDetails/test.ts +++ b/src/js/state/LogDetails/test.ts @@ -65,7 +65,11 @@ test("going back, then push, then back", () => { LogDetails.back() ]) - expect(LogDetails.build(state).get("letter")).toBe("a") + expect( + LogDetails.build(state) + .get("letter") + .toString() + ).toBe("a") }) test("updating the current log detail", () => { From e22798e68e35febebb85ebc60e1bb47bf524df03 Mon Sep 17 00:00:00 2001 From: James Kerr Date: Wed, 10 Feb 2021 11:28:28 -0800 Subject: [PATCH 3/7] Replaced 3 history implementations with one --- app/core/models/history.test.ts | 141 +++++++++++++++++++ app/core/models/history.ts | 62 ++++++++ src/js/brim/entries.test.ts | 113 --------------- src/js/brim/entries.ts | 51 ------- src/js/brim/index.ts | 2 - src/js/components/LogDetailsWindow/index.tsx | 4 +- src/js/components/RightPane.tsx | 4 +- src/js/components/SearchBar/Input.tsx | 12 +- src/js/models/InputHistory.test.ts | 111 --------------- src/js/models/InputHistory.ts | 31 ---- src/js/models/LogDetailHistory.test.ts | 92 ------------ src/js/models/LogDetailHistory.ts | 78 ---------- src/js/state/History/reducer.ts | 35 +++-- src/js/state/History/selectors.ts | 9 +- src/js/state/LogDetails/reducer.ts | 47 +++---- src/js/state/LogDetails/selectors.ts | 50 ++----- src/js/state/LogDetails/types.ts | 1 - 17 files changed, 262 insertions(+), 581 deletions(-) create mode 100644 app/core/models/history.test.ts create mode 100644 app/core/models/history.ts delete mode 100644 src/js/brim/entries.test.ts delete mode 100644 src/js/brim/entries.ts delete mode 100644 src/js/models/InputHistory.test.ts delete mode 100644 src/js/models/InputHistory.ts delete mode 100644 src/js/models/LogDetailHistory.test.ts delete mode 100644 src/js/models/LogDetailHistory.ts diff --git a/app/core/models/history.test.ts b/app/core/models/history.test.ts new file mode 100644 index 0000000000..24a2b605ac --- /dev/null +++ b/app/core/models/history.test.ts @@ -0,0 +1,141 @@ +import History from "./history" + +let history: History +beforeEach(() => { + history = new History() +}) + +test("push new value", () => { + history.push("a") + + expect(history.entries()).toEqual(["a"]) +}) + +test("pushing the same value twice", () => { + history.push("a") + history.push("a") + expect(history.entries()).toEqual(["a"]) +}) + +test("pushing the same value twice after going back", () => { + history.push("a") + history.push("b") + history.back() + history.push("a") + history.push("a") + expect(history.entries()).toEqual(["a", "b"]) +}) + +test("replace", () => { + history.push("a") + history.push("b") + history.back() + history.replace("z") + expect(history.entries()).toEqual(["z", "b"]) +}) + +test("update on object", () => { + type Person = {name: string; age: number} + const history = new History() + history.push({name: "A", age: 1}) + history.push({name: "B", age: 2}) + history.push({name: "C", age: 3}) + history.update({name: "Z"}) + expect(history.current()).toEqual({name: "Z", age: 3}) +}) + +test("push, push, back, push, back", () => { + history.push("a") + history.push("b") + history.back() + history.push("c") + history.back() + + expect(history.current()).toEqual("a") +}) + +test("current gets latest entry", () => { + history.push("a") + history.push("b") + history.push("c") + + expect(history.current()).toEqual("c") +}) + +test("going back", () => { + history.push("a") + history.push("b") + history.push("c") + + history.back() + + expect(history.current()).toEqual("b") +}) + +test("going back twice", () => { + history.push("a") + history.push("b") + history.push("c") + + history.back() + history.back() + + expect(history.current()).toEqual("a") +}) + +test("going back twice then forward twice", () => { + history.push("a") + history.push("b") + history.push("c") + + history.back() + history.back() + history.forward() + history.forward() + + expect(history.current()).toEqual("c") +}) + +test("when going back too far", () => { + history.push("a") + history.push("b") + history.push("c") + + history.back() + history.back() + history.back() + + expect(history.current()).toEqual("a") +}) + +test("when going forward too far and then back", () => { + history.push("a") + history.push("b") + history.push("c") + + history.forward() + + expect(history.current()).toEqual("c") + + history.back() + + expect(history.current()).toEqual("b") +}) + +test("pushing duplicates", () => { + history.push("a") + history.push("b") + history.push("b") + history.push("b") + + expect(history.entries()).toEqual(["a", "b"]) +}) + +test("pushing duplicates after going back", () => { + history.push("a") + history.push("b") + history.back() + history.push("b") + + expect(history.entries()).toEqual(["a", "b"]) +}) diff --git a/app/core/models/history.ts b/app/core/models/history.ts new file mode 100644 index 0000000000..5608956c42 --- /dev/null +++ b/app/core/models/history.ts @@ -0,0 +1,62 @@ +import {isEqual} from "lodash" + +export default class History { + static parse({entries, position}) { + return new History(entries, position) + } + + constructor(private array: Entry[] = [], private index: number = 0) {} + + push(entry: Entry) { + if (!isEqual(entry, this.current())) { + this.array.splice(this.index + 1, this.array.length, entry) + this.index = this.array.length - 1 + } + } + + update(updates: Partial) { + const current = this.current() + if (typeof current == "object") { + this.array[this.index] = {...current, ...updates} + } + } + + replace(entry: Entry) { + this.array[this.index] = entry + } + + entries(): Entry[] { + return this.array + } + + current(): Entry | undefined { + return this.array[this.index] + } + + canGoBack() { + return this.index > 0 + } + + back() { + if (this.canGoBack()) { + this.index -= 1 + } + } + + canGoForward() { + return this.index < this.array.length - 1 + } + + forward() { + if (this.canGoForward()) { + this.index += 1 + } + } + + serialize() { + return { + entries: [...this.array], + position: this.index + } + } +} diff --git a/src/js/brim/entries.test.ts b/src/js/brim/entries.test.ts deleted file mode 100644 index 50a3c1e2c6..0000000000 --- a/src/js/brim/entries.test.ts +++ /dev/null @@ -1,113 +0,0 @@ -import brim from "./" - -let history -beforeEach(() => { - history = brim.entries({entries: [], position: -1}) -}) - -test("#constructor throws error if position is out of bounds", () => { - expect(() => { - brim.entries({entries: ["a"], position: 999}) - }).toThrow("Position out of bounds") -}) - -test("#push", () => { - history.push("a") - history.push("b") - expect(history.getEntries()).toEqual(["a", "b"]) -}) - -test("#push checks for equality of the currentEntry", () => { - history.push("a") - history.push("a") - expect(history.getEntries()).toEqual(["a"]) -}) - -test("#goBack", () => { - history.push("a") - history.push("b") - history.goBack() - expect(history.getCurrentEntry()).toBe("a") -}) - -test("#goForward", () => { - history.push("a") - history.push("b") - history.goBack() - history.goForward() - expect(history.getCurrentEntry()).toBe("b") -}) - -test("#canGoBack when empty", () => { - expect(history.canGoBack()).toBe(false) -}) - -test("#canGoBack when one item", () => { - history.push("a") - expect(history.canGoBack()).toBe(false) -}) - -test("#canGoBack when two", () => { - history.push("a") - history.push("b") - expect(history.canGoBack()).toBe(true) -}) - -test("#canGoBack when backing up all the way", () => { - history.push("a") - history.push("b") - history.goBack() - expect(history.canGoBack()).toBe(false) -}) - -test("#canGoForward when empty", () => { - expect(history.canGoForward()).toBe(false) -}) - -test("#canGoForward when one", () => { - history.push("a") - expect(history.canGoForward()).toBe(false) -}) - -test("#canGoForward when two", () => { - history.push("a") - history.push("b") - expect(history.canGoForward()).toBe(false) -}) - -test("#canGoForward when two then back", () => { - history.push("a") - history.push("b") - history.goBack() - expect(history.canGoForward()).toBe(true) -}) - -test("#push after going back", () => { - history.push("a") - history.push("b") - history.push("c") - history.goBack() - history.push("d") - expect(history.getEntries()).toEqual(["a", "b", "d"]) - expect(history.getCurrentEntry()).toEqual("d") -}) - -test("#goBack can be called many times", () => { - history.push("a") - history.goBack() - history.goBack() - history.goBack() - history.goBack() - history.goBack() - expect(history.getCurrentEntry()).toBe("a") -}) - -test("#goForward can be called many times", () => { - history.push("a") - history.goForward() - history.goForward() - history.goForward() - history.goForward() - history.goForward() - expect(history.getCurrentEntry()).toBe("a") -}) diff --git a/src/js/brim/entries.ts b/src/js/brim/entries.ts deleted file mode 100644 index 2ca1ecd1e6..0000000000 --- a/src/js/brim/entries.ts +++ /dev/null @@ -1,51 +0,0 @@ -import isEqual from "lodash/isEqual" - -type Args = { - position: number - entries: any[] -} - -export default function({entries: initEntries, position}: Args) { - const entries = [...initEntries] - - if (position < -1 || position >= entries.length) { - throw new Error("Position out of bounds") - } - - return { - push(entry: any) { - if (!isEqual(entry, this.getCurrentEntry())) { - entries.splice(position + 1, entries.length, entry) - position = entries.length - 1 - } - return this - }, - update(updates: any) { - entries[position] = {...entries[position], ...updates} - return this - }, - goBack() { - if (this.canGoBack()) position -= 1 - return this - }, - goForward() { - if (this.canGoForward()) position += 1 - return this - }, - canGoBack() { - return entries.length > 1 ? position !== 0 : false - }, - canGoForward() { - return entries.length > 1 ? position < entries.length - 1 : false - }, - getEntries() { - return entries - }, - getCurrentEntry() { - return entries[position] - }, - data() { - return {position, entries} - } - } -} diff --git a/src/js/brim/index.ts b/src/js/brim/index.ts index d75d42b8b8..4e871df33f 100644 --- a/src/js/brim/index.ts +++ b/src/js/brim/index.ts @@ -1,6 +1,5 @@ import ast from "./ast" import dateTuple from "./dateTuple" -import entries from "./entries" import form from "./form" import interop from "./interop" import program from "./program" @@ -35,7 +34,6 @@ export default { form, interop, randomHash, - entries, tab, workspace } diff --git a/src/js/components/LogDetailsWindow/index.tsx b/src/js/components/LogDetailsWindow/index.tsx index e50dbe2779..e4523ddebf 100644 --- a/src/js/components/LogDetailsWindow/index.tsx +++ b/src/js/components/LogDetailsWindow/index.tsx @@ -11,8 +11,8 @@ import ToolbarAction from "app/toolbar/action-button" export default function LogDetailsWindow() { const dispatch = useDispatch() - const prevExists = useSelector(LogDetails.getHistory).prevExists() - const nextExists = useSelector(LogDetails.getHistory).nextExists() + const prevExists = useSelector(LogDetails.getHistory).canGoBack() + const nextExists = useSelector(LogDetails.getHistory).canGoForward() const space = useSelector(Current.mustGetSpace) return (
diff --git a/src/js/components/RightPane.tsx b/src/js/components/RightPane.tsx index c890c86ac0..e774d6864d 100644 --- a/src/js/components/RightPane.tsx +++ b/src/js/components/RightPane.tsx @@ -96,8 +96,8 @@ const stateToProps = (state) => { return { isOpen: Layout.getRightSidebarIsOpen(state), width: Layout.getRightSidebarWidth(state), - prevExists: LogDetails.getHistory(state).prevExists(), - nextExists: LogDetails.getHistory(state).nextExists(), + prevExists: LogDetails.getHistory(state).canGoBack(), + nextExists: LogDetails.getHistory(state).canGoForward(), currentLog: LogDetails.build(state), space: Current.getSpace(state) } diff --git a/src/js/components/SearchBar/Input.tsx b/src/js/components/SearchBar/Input.tsx index d7a66ddba2..ae1dc3397c 100644 --- a/src/js/components/SearchBar/Input.tsx +++ b/src/js/components/SearchBar/Input.tsx @@ -5,8 +5,8 @@ import styled from "styled-components" import {cssVar} from "../../lib/cssVar" import {reactElementProps} from "../../test/integration" import {submitSearch} from "../../flows/submitSearch/mod" -import InputHistory from "../../models/InputHistory" import SearchBar from "../../state/SearchBar" +import History from "app/core/models/history" const StyledInput = styled.input` display: block; @@ -28,7 +28,7 @@ const StyledInput = styled.input` export default function Input() { const dispatch = useDispatch() - const history = useRef(new InputHistory()) + const history = useRef(new History()) const inputValue = useSelector(SearchBar.getSearchBarInputValue) function changeTo(value: string) { @@ -49,12 +49,12 @@ export default function Input() { history.current.push(e.currentTarget.value) } if (e.key === "ArrowUp") { - history.current.goBack() - changeTo(history.current.getCurrentEntry()) + history.current.back() + changeTo(history.current.current()) } if (e.key === "ArrowDown") { - history.current.goForward() - changeTo(history.current.getCurrentEntry()) + history.current.forward() + changeTo(history.current.current()) } } diff --git a/src/js/models/InputHistory.test.ts b/src/js/models/InputHistory.test.ts deleted file mode 100644 index 42b0570e5b..0000000000 --- a/src/js/models/InputHistory.test.ts +++ /dev/null @@ -1,111 +0,0 @@ -import InputHistory from "./InputHistory" - -test("push new value", () => { - const history = new InputHistory() - - history.push("a") - - expect(history.getEntries()).toEqual(["a"]) -}) - -test("getCurrentEntry gets latest entry", () => { - const history = new InputHistory() - - history.push("a") - history.push("b") - history.push("c") - - expect(history.getCurrentEntry()).toEqual("c") -}) - -test("going back", () => { - const history = new InputHistory() - - history.push("a") - history.push("b") - history.push("c") - - history.goBack() - - expect(history.getCurrentEntry()).toEqual("b") -}) - -test("going back twice", () => { - const history = new InputHistory() - - history.push("a") - history.push("b") - history.push("c") - - history.goBack() - history.goBack() - - expect(history.getCurrentEntry()).toEqual("a") -}) - -test("going back twice then forward twice", () => { - const history = new InputHistory() - - history.push("a") - history.push("b") - history.push("c") - - history.goBack() - history.goBack() - history.goForward() - history.goForward() - - expect(history.getCurrentEntry()).toEqual("c") -}) - -test("when going back too far", () => { - const history = new InputHistory() - - history.push("a") - history.push("b") - history.push("c") - - history.goBack() - history.goBack() - history.goBack() - - expect(history.getCurrentEntry()).toEqual("a") -}) - -test("when going forward too far and then back", () => { - const history = new InputHistory() - - history.push("a") - history.push("b") - history.push("c") - - history.goForward() - - expect(history.getCurrentEntry()).toEqual("c") - - history.goBack() - - expect(history.getCurrentEntry()).toEqual("b") -}) - -test("pushing duplicates", () => { - const history = new InputHistory() - - history.push("a") - history.push("b") - history.push("b") - history.push("b") - - expect(history.getEntries()).toEqual(["a", "b"]) -}) - -test("pushing duplicates after going back", () => { - const history = new InputHistory() - - history.push("a") - history.push("b") - history.goBack() - history.push("b") - - expect(history.getEntries()).toEqual(["a", "b"]) -}) diff --git a/src/js/models/InputHistory.ts b/src/js/models/InputHistory.ts deleted file mode 100644 index e5e89e4687..0000000000 --- a/src/js/models/InputHistory.ts +++ /dev/null @@ -1,31 +0,0 @@ -export default class InputHistory { - entries: Entry[] = [] - position = 0 - - push(newVal: Entry) { - if (newVal !== this.entries[this.entries.length - 1]) { - this.position = this.entries.length - this.entries.push(newVal) - } - } - - getEntries() { - return this.entries - } - - getCurrentEntry(): Entry { - return this.entries[this.position] - } - - goBack() { - if (this.position != 0) { - this.position -= 1 - } - } - - goForward() { - if (this.position < this.entries.length - 1) { - this.position += 1 - } - } -} diff --git a/src/js/models/LogDetailHistory.test.ts b/src/js/models/LogDetailHistory.test.ts deleted file mode 100644 index f123c9084a..0000000000 --- a/src/js/models/LogDetailHistory.test.ts +++ /dev/null @@ -1,92 +0,0 @@ -import History from "./LogDetailHistory" - -test("saves to history entries", () => { - const history = new History() - - history.save("james") - history.save("kerr") - - expect(history.entries).toEqual(["james", "kerr"]) -}) - -test("getCurrent", () => { - const history = new History() - - history.save("james") - history.save("kerr") - - expect(history.getCurrent()).toBe("kerr") -}) - -test("getPrev when there is one", () => { - const history = new History(["james", "kerr"]) - - expect(history.getPrev()).toBe("james") -}) - -test("getPrev when there is not one", () => { - const history = new History(["kerr"]) - - expect(history.getPrev()).toBe(null) -}) - -test("getPrev when there an empty array", () => { - const history = new History() - - expect(history.getPrev()).toBe(null) -}) - -test("getNext when there an empty array", () => { - const history = new History() - - expect(history.getNext()).toBe(null) -}) - -test("getNext with one item", () => { - const history = new History(["james"]) - - expect(history.getNext()).toBe(null) -}) - -test("getNext with one item going back and forth", () => { - const history = new History(["james"]) - history.getPrev() - expect(history.getNext()).toBe(null) -}) - -test("getNext after going back", () => { - const history = new History(["james", "kerr"]) - history.goBack() - expect(history.getNext()).toBe("kerr") -}) - -test("getPrev after going forward", () => { - const history = new History(["james", "andrew", "kerr"]) - history.goBack() - history.goBack() - history.goForward() - expect(history.getPrev()).toBe("james") -}) - -test("getMostRecent when empty", () => { - const history = new History() - - expect(history.getMostRecent()).toBe(null) -}) - -test("getMostRecent when not empty", () => { - const history = new History(["james"]) - - expect(history.getMostRecent()).toBe("james") -}) - -test("save, save, goBack, save, goBack", () => { - const history = new History() - history.save("a") - history.save("b") - history.goBack() - expect(history.getCurrent()).toBe("a") - history.save("c") - history.goBack() - expect(history.getCurrent()).toBe("a") -}) diff --git a/src/js/models/LogDetailHistory.ts b/src/js/models/LogDetailHistory.ts deleted file mode 100644 index 172656b882..0000000000 --- a/src/js/models/LogDetailHistory.ts +++ /dev/null @@ -1,78 +0,0 @@ -export default class LogDetailHistory { - entries: T[] - position: number - - constructor(entries: T[] = [], position = 0) { - this.entries = entries - this.position = position - } - - clear() { - this.entries = [] - this.position = 0 - } - - save(entry: T) { - const length = this.entries.length - const index = length - this.position - this.entries.splice(index, length, entry) - this.position = 0 - } - - nextExists() { - return this.position > 0 - } - - prevExists() { - return this.position + 1 < this.entries.length - } - - get(position: number) { - const index = this.entries.length - 1 - position - return this.entries[index] - } - - getMostRecent() { - if (!this.entries.length) return null - return this.entries[this.entries.length - 1] - } - - getCurrent(): T | null | undefined { - if (!this.entries.length) return null - const index = this.entries.length - 1 - this.position - return this.entries[index] - } - - updateCurrent(updates: Partial) { - if (!this.entries.length) return null - const index = this.entries.length - 1 - this.position - const entry = this.entries[index] - this.entries[index] = {...entry, ...updates} - } - - goBack() { - if (this.prevExists()) { - this.position += 1 - } - } - - goForward() { - if (this.nextExists()) { - this.position -= 1 - } - } - - getPrev() { - if (!this.prevExists()) return null - return this.get(this.position + 1) - } - - getNext() { - if (!this.nextExists()) return null - return this.get(this.position - 1) - } - - toArray() { - return this.entries - } -} diff --git a/src/js/state/History/reducer.ts b/src/js/state/History/reducer.ts index 1d4d6639f2..1d9f4bcdd1 100644 --- a/src/js/state/History/reducer.ts +++ b/src/js/state/History/reducer.ts @@ -1,8 +1,9 @@ +import {SearchRecord} from "src/js/types" import {HistoryAction, HistoryState} from "./types" -import brim from "../../brim" +import History from "app/core/models/history" const init: HistoryState = { - position: -1, + position: 0, entries: [] } @@ -10,33 +11,31 @@ export default function reducer( state: HistoryState = init, action: HistoryAction ): HistoryState { + let history: History + switch (action.type) { case "HISTORY_CLEAR": return {...init} case "HISTORY_PUSH": - return brim - .entries(state) - .push(action.entry) - .data() + history = History.parse(state) + history.push(action.entry) + return history.serialize() case "HISTORY_BACK": - return brim - .entries(state) - .goBack() - .data() + history = History.parse(state) + history.back() + return history.serialize() case "HISTORY_FORWARD": - return brim - .entries(state) - .goForward() - .data() + history = History.parse(state) + history.forward() + return history.serialize() case "HISTORY_UPDATE": - return brim - .entries(state) - .update({scrollPos: action.scrollPos}) - .data() + history = History.parse(state) + history.update({scrollPos: action.scrollPos}) + return history.serialize() default: return state diff --git a/src/js/state/History/selectors.ts b/src/js/state/History/selectors.ts index f56f8aa29b..a18f39a175 100644 --- a/src/js/state/History/selectors.ts +++ b/src/js/state/History/selectors.ts @@ -1,12 +1,15 @@ import {TabState} from "../Tab/types" import activeTabSelect from "../Tab/activeTabSelect" -import brim from "../../brim" +import History from "app/core/models/history" +import {SearchRecord} from "src/js/types" export default { prev: (state: TabState) => state.history.entries[state.history.position - 1], current: (state: TabState) => state.history.entries[state.history.position], - canGoBack: (state: TabState) => brim.entries(state.history).canGoBack(), - canGoForward: (state: TabState) => brim.entries(state.history).canGoForward(), + canGoBack: (state: TabState) => + History.parse(state.history).canGoBack(), + canGoForward: (state: TabState) => + History.parse(state.history).canGoForward(), first: activeTabSelect((state: TabState) => state.history.entries[0]), count: activeTabSelect((state: TabState) => state.history.entries.length) } diff --git a/src/js/state/LogDetails/reducer.ts b/src/js/state/LogDetails/reducer.ts index f9df60f491..aa1e39b138 100644 --- a/src/js/state/LogDetails/reducer.ts +++ b/src/js/state/LogDetails/reducer.ts @@ -1,56 +1,39 @@ import {LogDetails, LogDetailsAction, LogDetailsState} from "./types" -import LogDetailHistory from "../../models/LogDetailHistory" +import History from "app/core/models/history" const init = (): LogDetailsState => ({ entries: [], - position: 0, - prevPosition: -1 + position: 0 }) export default function reducer( state: LogDetailsState = init(), action: LogDetailsAction ): LogDetailsState { - let history + let history: History switch (action.type) { case "LOG_DETAIL_PUSH": history = toHistory(state) - history.save({ + history.push({ log: action.record, uidLogs: [], uidStatus: "INIT" }) - return { - entries: history.entries, - position: history.position, - prevPosition: state.position - } + return history.serialize() case "LOG_DETAIL_UPDATE": history = toHistory(state) - history.updateCurrent(action.updates) - return { - entries: history.entries, - position: history.position, - prevPosition: state.position - } + history.update(action.updates) + return history.serialize() case "LOG_DETAIL_FORWARD": history = toHistory(state) - history.goForward() - return { - entries: history.entries, - position: history.position, - prevPosition: state.position - } + history.forward() + return history.serialize() case "LOG_DETAIL_BACK": history = toHistory(state) - history.goBack() - return { - entries: history.entries, - position: history.position, - prevPosition: state.position - } + history.back() + return history.serialize() case "LOG_DETAIL_CLEAR": return init() @@ -60,6 +43,10 @@ export default function reducer( } } -export const toHistory = ({entries, position}: LogDetailsState) => { - return new LogDetailHistory([...entries], position) +export type LogDetailHistory = History +export const toHistory = ({ + entries, + position +}: LogDetailsState): LogDetailHistory => { + return new History([...entries], position) } diff --git a/src/js/state/LogDetails/selectors.ts b/src/js/state/LogDetails/selectors.ts index b498ae82e8..1f0a7f4af0 100644 --- a/src/js/state/LogDetails/selectors.ts +++ b/src/js/state/LogDetails/selectors.ts @@ -2,51 +2,25 @@ import {createSelector} from "reselect" import {State} from "../types" import {TabState} from "../Tab/types" -import {toHistory} from "./reducer" +import {LogDetailHistory, toHistory} from "./reducer" import activeTabSelect from "../Tab/activeTabSelect" -import {LogDetailsState, LogDetails} from "./types" -import LogDetailHistory from "src/js/models/LogDetailHistory" +import {LogDetailsState} from "./types" import {SearchStatus} from "src/js/types/searches" import {zng} from "zealot" -type History = LogDetailHistory const getLogDetails = activeTabSelect((state: TabState) => { return state.logDetails }) -const getPosition = activeTabSelect((state: TabState) => { - return state.logDetails.position -}) - -const getPrevPosition = activeTabSelect((state: TabState) => { - return state.logDetails.prevPosition -}) - -const getHistory = createSelector( +const getHistory = createSelector( getLogDetails, (logDetails) => toHistory(logDetails) ) -const getPrevExists = createSelector( - getHistory, - (history) => history.prevExists() -) - -const getNextExists = createSelector( - getHistory, - (history) => history.nextExists() -) - -const getIsGoingBack = createSelector( - getPosition, - getPrevPosition, - (position, prevPosition) => prevPosition - position < 0 -) - -const build = createSelector( +const build = createSelector( getHistory, (history) => { - const entry = history.getCurrent() + const entry = history.current() if (entry && entry.log) { return zng.Record.deserialize(entry.log) } else { @@ -55,20 +29,20 @@ const build = createSelector( } ) -const getUidLogs = createSelector( +const getUidLogs = createSelector( getHistory, (history) => { - const entry = history.getCurrent() + const entry = history.current() return entry ? entry.uidLogs.map((data) => zng.Record.deserialize(data)) : [] } ) -const getUidStatus = createSelector( +const getUidStatus = createSelector( getHistory, (history) => { - const entry = history.getCurrent() + const entry = history.current() return entry ? entry.uidStatus : "INIT" } ) @@ -84,12 +58,6 @@ export default { getConnLog, getUidStatus, getUidLogs, - getLogDetails, - getPosition, - getPrevPosition, build, - getIsGoingBack, - getNextExists, - getPrevExists, getHistory } diff --git a/src/js/state/LogDetails/types.ts b/src/js/state/LogDetails/types.ts index 2869a70c7a..e46ccd332a 100644 --- a/src/js/state/LogDetails/types.ts +++ b/src/js/state/LogDetails/types.ts @@ -4,7 +4,6 @@ import {SearchStatus} from "../../types/searches" export type LogDetailsState = { entries: LogDetails[] position: number - prevPosition: number } export type LogDetails = { From e74f915c1d95df2c7db266ecd1284ce3bd62537d Mon Sep 17 00:00:00 2001 From: James Kerr Date: Wed, 10 Feb 2021 15:44:02 -0800 Subject: [PATCH 4/7] Copy the array when initializing --- app/core/models/history.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/core/models/history.ts b/app/core/models/history.ts index 5608956c42..3d487bcd79 100644 --- a/app/core/models/history.ts +++ b/app/core/models/history.ts @@ -2,7 +2,7 @@ import {isEqual} from "lodash" export default class History { static parse({entries, position}) { - return new History(entries, position) + return new History([...entries], position) } constructor(private array: Entry[] = [], private index: number = 0) {} @@ -55,7 +55,7 @@ export default class History { serialize() { return { - entries: [...this.array], + entries: this.array, position: this.index } } From 37357ec9930309cf3ffb5410e68b72d291c7d15c Mon Sep 17 00:00:00 2001 From: James Kerr Date: Thu, 11 Feb 2021 10:44:06 -0800 Subject: [PATCH 5/7] Fix command history --- app/core/models/cmd-history.test.ts | 52 +++++++++++++++++++++++++++ app/core/models/cmd-history.ts | 36 +++++++++++++++++++ src/js/components/SearchBar/Input.tsx | 14 ++++---- 3 files changed, 95 insertions(+), 7 deletions(-) create mode 100644 app/core/models/cmd-history.test.ts create mode 100644 app/core/models/cmd-history.ts diff --git a/app/core/models/cmd-history.test.ts b/app/core/models/cmd-history.test.ts new file mode 100644 index 0000000000..dca4bac6e8 --- /dev/null +++ b/app/core/models/cmd-history.test.ts @@ -0,0 +1,52 @@ +import CmdHistory from "./cmd-history" + +let h: CmdHistory +beforeEach(() => { + h = new CmdHistory([], 0, 5) +}) + +test("push", () => { + h.push("a") + + expect(h.all()).toEqual(["a"]) +}) + +test("back", () => { + h.push("a") + h.push("b") + + expect(h.back()).toEqual("a") + expect(h.all()).toEqual(["a", "b"]) +}) + +test("fowrard", () => { + h.push("a") + h.push("b") + h.push("c") + h.back() + h.back() + + expect(h.forward()).toEqual("b") + expect(h.all()).toEqual(["a", "b", "c"]) +}) + +test("back when none", () => { + expect(h.back()).toEqual(null) + expect(h.all()).toEqual([]) +}) + +test("forward when none", () => { + expect(h.forward()).toEqual(null) + expect(h.all()).toEqual([]) +}) + +test("limit", () => { + h.push("a") + h.push("b") + h.push("c") + h.push("d") + h.push("e") + // This one is over the limit + h.push("f") + expect(h.all()).toEqual(["b", "c", "d", "e", "f"]) +}) diff --git a/app/core/models/cmd-history.ts b/app/core/models/cmd-history.ts new file mode 100644 index 0000000000..40525f9acd --- /dev/null +++ b/app/core/models/cmd-history.ts @@ -0,0 +1,36 @@ +export default class CmdHistory { + constructor( + private array: string[] = [], + private index: number = 0, + private limit: number + ) {} + + push(cmd) { + if (this.array.length === this.limit) this.array.shift() + this.array.push(cmd) + this.index = this.array.length - 1 + } + + back() { + if (this.index > 0) this.index -= 1 + return this.current() + } + + forward() { + if (this.index < this.array.length - 1) this.index += 1 + return this.current() + } + + all() { + return this.array + } + + empty() { + return this.array.length === 0 + } + + private current() { + if (this.empty()) return null + return this.array[this.index] + } +} diff --git a/src/js/components/SearchBar/Input.tsx b/src/js/components/SearchBar/Input.tsx index ae1dc3397c..5160556fa0 100644 --- a/src/js/components/SearchBar/Input.tsx +++ b/src/js/components/SearchBar/Input.tsx @@ -7,6 +7,7 @@ import {reactElementProps} from "../../test/integration" import {submitSearch} from "../../flows/submitSearch/mod" import SearchBar from "../../state/SearchBar" import History from "app/core/models/history" +import CmdHistory from "app/core/models/cmd-history" const StyledInput = styled.input` display: block; @@ -25,10 +26,11 @@ const StyledInput = styled.input` outline: none; } ` +// create a tiny little history closure export default function Input() { const dispatch = useDispatch() - const history = useRef(new History()) + const history = useRef(new CmdHistory([], 0, 1000)) const inputValue = useSelector(SearchBar.getSearchBarInputValue) function changeTo(value: string) { @@ -48,13 +50,11 @@ export default function Input() { submit() history.current.push(e.currentTarget.value) } - if (e.key === "ArrowUp") { - history.current.back() - changeTo(history.current.current()) + if (e.key === "ArrowUp" && !history.current.empty()) { + changeTo(history.current.back()) } - if (e.key === "ArrowDown") { - history.current.forward() - changeTo(history.current.current()) + if (e.key === "ArrowDown" && !history.current.empty()) { + changeTo(history.current.forward()) } } From 665eade5e798f81dec2716e6b21ac9675ac9416a Mon Sep 17 00:00:00 2001 From: James Kerr Date: Thu, 11 Feb 2021 10:44:51 -0800 Subject: [PATCH 6/7] lint --- src/js/components/SearchBar/Input.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/js/components/SearchBar/Input.tsx b/src/js/components/SearchBar/Input.tsx index 5160556fa0..38f89ad3b4 100644 --- a/src/js/components/SearchBar/Input.tsx +++ b/src/js/components/SearchBar/Input.tsx @@ -6,7 +6,6 @@ import {cssVar} from "../../lib/cssVar" import {reactElementProps} from "../../test/integration" import {submitSearch} from "../../flows/submitSearch/mod" import SearchBar from "../../state/SearchBar" -import History from "app/core/models/history" import CmdHistory from "app/core/models/cmd-history" const StyledInput = styled.input` From 32623a5f280b23d191d1eebc3723bb51f651d9be Mon Sep 17 00:00:00 2001 From: James Kerr Date: Thu, 11 Feb 2021 15:43:14 -0800 Subject: [PATCH 7/7] Remove comment --- src/js/components/SearchBar/Input.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/js/components/SearchBar/Input.tsx b/src/js/components/SearchBar/Input.tsx index 38f89ad3b4..80882d6fcf 100644 --- a/src/js/components/SearchBar/Input.tsx +++ b/src/js/components/SearchBar/Input.tsx @@ -25,7 +25,6 @@ const StyledInput = styled.input` outline: none; } ` -// create a tiny little history closure export default function Input() { const dispatch = useDispatch()