From 95ddff5d0ff9f3b43332c911a5718f82311c9b07 Mon Sep 17 00:00:00 2001 From: Oscar Lorentzon Date: Thu, 21 Dec 2017 07:39:37 +0000 Subject: [PATCH] fix: render position correctly after sequence change When navigating to a new node and directly thereafter pressing the timeline thumb the thumb should be rendered in the new position after the sequence switch has occured. Add unit test case for above scenario. --- .../sequence/SequenceComponent.spec.ts | 150 +++++++++++++++++- src/component/sequence/SequenceComponent.ts | 89 ++++++----- 2 files changed, 196 insertions(+), 43 deletions(-) diff --git a/spec/component/sequence/SequenceComponent.spec.ts b/spec/component/sequence/SequenceComponent.spec.ts index 1e7da866..4ea67845 100644 --- a/spec/component/sequence/SequenceComponent.spec.ts +++ b/spec/component/sequence/SequenceComponent.spec.ts @@ -1,6 +1,7 @@ /// import {Observable} from "rxjs/Observable"; +import {ReplaySubject} from "rxjs/ReplaySubject"; import {Subject} from "rxjs/Subject"; import {VirtualTimeScheduler} from "rxjs/scheduler/VirtualTimeScheduler"; @@ -304,6 +305,8 @@ describe("SequenceComponent.activate", () => { const renderSpy: jasmine.Spy = spyOn(renderer, "render").and.stub(); + (>navigatorMock.stateService.currentNode$) = new ReplaySubject(1); + const component: SequenceComponent = createComponent(); component.activate(); @@ -341,6 +344,8 @@ describe("SequenceComponent.activate", () => { const renderSpy: jasmine.Spy = spyOn(renderer, "render").and.stub(); + (>navigatorMock.stateService.currentNode$) = new ReplaySubject(1); + const component: SequenceComponent = createComponent(); component.activate(); @@ -377,6 +382,67 @@ describe("SequenceComponent.activate", () => { expect(renderSpy.calls.mostRecent().args[5]).toBe(1); }); + it("should render correct index on sequence change when changing position simultaneously", () => { + const sequenceSubject$: Subject = new Subject(); + (navigatorMock.graphService.cacheSequence$).and.returnValue(sequenceSubject$); + (navigatorMock.graphService.cacheSequenceNodes$).and.returnValue(new Subject()); + (navigatorMock.graphService.cacheNode$).and.returnValue(new Subject()); + + const changedSubject$: Subject = new Subject(); + mockCreator.mockProperty(renderer, "changed$", changedSubject$); + mockCreator.mockProperty(renderer, "changingPosition", false); + spyOn(renderer, "getContainerWidth").and.returnValue(100); + + const renderSpy: jasmine.Spy = spyOn(renderer, "render").and.stub(); + + (>navigatorMock.stateService.currentNode$) = new ReplaySubject(1); + + const component: SequenceComponent = createComponent(); + component.activate(); + + (>navigatorMock.playService.speed$).next(1); + + const sequenceKey1: string = "sequenceKey1"; + const sequenceKey2: string = "sequenceKey2"; + const nodeKey1: string = "nodeKey1"; + const nodeKey2: string = "nodeKey2"; + + const node1: Node = nodeHelper.createNode(); + mockCreator.mockProperty(node1, "spatialEdges", { cached: false, edges: [] }); + mockCreator.mockProperty(node1, "sequenceEdges$", Observable.of({ cached: false, edges: [] })); + mockCreator.mockProperty(node1, "key", nodeKey1); + mockCreator.mockProperty(node1, "sequenceKey", sequenceKey1); + (>navigatorMock.stateService.currentNode$).next(node1); + + expect(renderSpy.calls.count()).toBe(1); + + sequenceSubject$.next(new Sequence({ key: sequenceKey1, keys: [nodeKey1] })); + + expect(renderSpy.calls.count()).toBe(2); + expect(renderSpy.calls.mostRecent().args[4]).toBe(0); + expect(renderSpy.calls.mostRecent().args[5]).toBe(0); + + (renderer.changingPosition) = true; + changedSubject$.next(renderer); + + const node2: Node = nodeHelper.createNode(); + mockCreator.mockProperty(node2, "spatialEdges", { cached: false, edges: [] }); + mockCreator.mockProperty(node2, "sequenceEdges$", Observable.of({ cached: false, edges: [] })); + mockCreator.mockProperty(node2, "key", nodeKey2); + mockCreator.mockProperty(node2, "sequenceKey", sequenceKey2); + (>navigatorMock.stateService.currentNode$).next(node2); + + (renderer.changingPosition) = false; + changedSubject$.next(renderer); + + sequenceSubject$.next(new Sequence({ key: sequenceKey2, keys: [nodeKey2] })); + + expect(renderSpy.calls.count()).toBeGreaterThan(2); + expect(renderSpy.calls.mostRecent().args[4]).toBe(0); + expect(renderSpy.calls.mostRecent().args[5]).toBe(0); + + }); + it("should render correct index on input emit", () => { const sequenceSubject$: Subject = new Subject(); (navigatorMock.graphService.cacheSequence$).and.returnValue(sequenceSubject$); @@ -394,6 +460,8 @@ describe("SequenceComponent.activate", () => { const renderSpy: jasmine.Spy = spyOn(renderer, "render").and.stub(); + (>navigatorMock.stateService.currentNode$) = new ReplaySubject(1); + const component: SequenceComponent = createComponent(); component.activate(); @@ -413,8 +481,8 @@ describe("SequenceComponent.activate", () => { expect(renderSpy.calls.count()).toBe(1); - changedSubject$.next(renderer); sequenceSubject$.next(new Sequence({ key: sequenceKey1, keys: [nodeKey1, nodeKey2, nodeKey3] })); + changedSubject$.next(renderer); indexSubject$.next(0); expect(renderSpy.calls.count()).toBeGreaterThan(1); @@ -422,6 +490,86 @@ describe("SequenceComponent.activate", () => { expect(renderSpy.calls.mostRecent().args[5]).toBe(2); }); + it("should render on first node emit after sequence change and on second thereafter", () => { + const sequenceSubject$: Subject = new Subject(); + (navigatorMock.graphService.cacheSequence$).and.returnValue(sequenceSubject$); + (navigatorMock.graphService.cacheSequenceNodes$).and.returnValue(new Subject()); + (navigatorMock.graphService.cacheNode$).and.returnValue(new Subject()); + + (navigatorMock.moveToKey$).and.returnValue(new Subject()); + + const changedSubject$: Subject = new Subject(); + mockCreator.mockProperty(renderer, "changed$", changedSubject$); + const indexSubject$: Subject = new Subject(); + mockCreator.mockProperty(renderer, "index$", indexSubject$); + mockCreator.mockProperty(renderer, "changingPosition", true); + spyOn(renderer, "getContainerWidth").and.returnValue(100); + + const renderSpy: jasmine.Spy = spyOn(renderer, "render").and.stub(); + + (>navigatorMock.stateService.currentNode$) = new ReplaySubject(1); + + const component: SequenceComponent = createComponent(); + component.activate(); + + (>navigatorMock.playService.speed$).next(1); + + const sequenceKey1: string = "sequenceKey1"; + const nodeKey1: string = "nodeKey1"; + const nodeKey2: string = "nodeKey2"; + const nodeKey3: string = "nodeKey3"; + + const node1: Node = nodeHelper.createNode(); + mockCreator.mockProperty(node1, "spatialEdges", { cached: false, edges: [] }); + mockCreator.mockProperty(node1, "sequenceEdges$", Observable.of({ cached: false, edges: [] })); + mockCreator.mockProperty(node1, "key", nodeKey1); + mockCreator.mockProperty(node1, "sequenceKey", sequenceKey1); + (>navigatorMock.stateService.currentNode$).next(node1); + + expect(renderSpy.calls.count()).toBe(1); + expect(renderSpy.calls.mostRecent().args[4]).toBe(null); + expect(renderSpy.calls.mostRecent().args[5]).toBe(null); + + sequenceSubject$.next(new Sequence({ key: sequenceKey1, keys: [nodeKey1, nodeKey2, nodeKey3] })); + + expect(renderSpy.calls.count()).toBe(2); + expect(renderSpy.calls.mostRecent().args[4]).toBe(0); + expect(renderSpy.calls.mostRecent().args[5]).toBe(2); + + changedSubject$.next(renderer); + + let callCount: number = renderSpy.calls.count(); + + expect(callCount).toBeGreaterThan(2); + expect(renderSpy.calls.mostRecent().args[4]).toBe(0); + expect(renderSpy.calls.mostRecent().args[5]).toBe(2); + + indexSubject$.next(1); + + expect(renderSpy.calls.count()).toBeGreaterThan(callCount); + callCount = renderSpy.calls.count(); + + expect(renderSpy.calls.mostRecent().args[4]).toBe(1); + expect(renderSpy.calls.mostRecent().args[5]).toBe(2); + + (renderer.changingPosition) = false; + + changedSubject$.next(renderer); + + expect(renderSpy.calls.mostRecent().args[4]).toBe(1); + expect(renderSpy.calls.mostRecent().args[5]).toBe(2); + + const node3: Node = nodeHelper.createNode(); + mockCreator.mockProperty(node3, "spatialEdges", { cached: false, edges: [] }); + mockCreator.mockProperty(node3, "sequenceEdges$", Observable.of({ cached: false, edges: [] })); + mockCreator.mockProperty(node3, "key", nodeKey3); + mockCreator.mockProperty(node3, "sequenceKey", sequenceKey1); + (>navigatorMock.stateService.currentNode$).next(node3); + + expect(renderSpy.calls.mostRecent().args[4]).toBe(2); + expect(renderSpy.calls.mostRecent().args[5]).toBe(2); + }); + it("should move to key on first index change", () => { const sequenceSubject$: Subject = new Subject(); (navigatorMock.graphService.cacheSequence$).and.returnValue(sequenceSubject$); diff --git a/src/component/sequence/SequenceComponent.ts b/src/component/sequence/SequenceComponent.ts index 93ecd4bf..0d7110a0 100644 --- a/src/component/sequence/SequenceComponent.ts +++ b/src/component/sequence/SequenceComponent.ts @@ -289,6 +289,10 @@ export class SequenceComponent extends Component { ([index, sequence]: [number, Sequence]): string => { return sequence != null ? sequence.keys[index] : null; }) + .filter( + (key: string): boolean => { + return !!key; + }) .distinctUntilChanged() .publishReplay(1) .refCount(); @@ -327,10 +331,6 @@ export class SequenceComponent extends Component { rendererKey$.debounceTime(100, this._scheduler); }) .distinctUntilChanged() - .filter( - (key: string): boolean => { - return key != null; - }) .switchMap( (key: string): Observable => { return this._navigator.moveToKey$(key) @@ -428,50 +428,55 @@ export class SequenceComponent extends Component { .subscribe(); let firstRendererKey: boolean = true; - let firstCurrentKey: boolean = true; - const position$: Observable<{ index: number, max: number }> = this._sequenceDOMRenderer.changed$ - .map( - (renderer: SequenceDOMRenderer): boolean => { - return renderer.changingPosition; - }) - .startWith(false) - .distinctUntilChanged() + + const position$: Observable<{ index: number, max: number }> = sequence$ .switchMap( - (changingPosition: boolean): Observable => { - let skip: number = 1; - - if (changingPosition && firstRendererKey) { - skip = 0; - firstRendererKey = false; - } else if (!changingPosition && firstCurrentKey) { - skip = 0; - firstCurrentKey = false; + (sequence: Sequence): Observable<{ index: number, max: number }> => { + if (!sequence) { + return Observable.of({ index: null, max: null }); } - return changingPosition ? - rendererKey$.skip(skip) : - this._navigator.stateService.currentNode$ - .map( - (node: Node): string => { - return node.key; - }) - .distinctUntilChanged() - .skip(skip); - }) - .combineLatest(sequence$) - .map( - ([key, sequence]: [string, Sequence]): { index: number, max: number } => { - if (key == null || sequence == null) { - return { index: null, max: null }; - } + let firstCurrentKey: boolean = true; - const index: number = sequence.keys.indexOf(key); + return this._sequenceDOMRenderer.changed$ + .map( + (renderer: SequenceDOMRenderer): boolean => { + return renderer.changingPosition; + }) + .startWith(false) + .distinctUntilChanged() + .switchMap( + (changingPosition: boolean): Observable => { + let skip: number = 1; - if (index === -1) { - return { index: null, max: null }; - } + if (changingPosition && firstRendererKey) { + skip = 0; + firstRendererKey = false; + } else if (!changingPosition && firstCurrentKey) { + skip = 0; + firstCurrentKey = false; + } - return { index: index, max: sequence.keys.length - 1 }; + return changingPosition ? + rendererKey$.skip(skip) : + this._navigator.stateService.currentNode$ + .map( + (node: Node): string => { + return node.key; + }) + .distinctUntilChanged() + .skip(skip); + }) + .map( + (key: string): { index: number, max: number } => { + const index: number = sequence.keys.indexOf(key); + + if (index === -1) { + return { index: null, max: null }; + } + + return { index: index, max: sequence.keys.length - 1 }; + }); }); this._renderSubscription = Observable