Skip to content

Commit

Permalink
feat(transition): Ignore duplicate transitions (double clicks)
Browse files Browse the repository at this point in the history
Previously, any calls to state.go would supersede the running transition.
If the user starts a transition, then immediately starts another identical transition,
the new transition supersedes the old transition.

Now, if the new transition is identical to the running transition, the new transition is ignored.
Identical basically means: same destination state and parameter values.

Closes #42
  • Loading branch information
christopherthielen committed Apr 20, 2017
1 parent 5a54b1d commit bd1bd0b
Show file tree
Hide file tree
Showing 21 changed files with 296 additions and 112 deletions.
2 changes: 1 addition & 1 deletion src/common/trace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import {Transition} from "../transition/transition";
import {ActiveUIView, ViewConfig, ViewContext} from "../view/interface";
import {stringify, functionToString, maxLength, padString} from "./strings";
import {Resolvable} from "../resolve/resolvable";
import {PathNode} from "../path/node";
import {PathNode} from "../path/pathNode";
import {PolicyWhen} from "../resolve/interface";
import {TransitionHook} from "../transition/transitionHook";
import {HookResult} from "../transition/interface";
Expand Down
18 changes: 15 additions & 3 deletions src/hooks/ignoredTransition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { trace } from '../common/trace';
import { Rejection } from '../transition/rejectFactory';
import { TransitionService } from '../transition/transitionService';
import { Transition } from '../transition/transition';
import {PathUtils} from '../path/pathFactory';

/**
* A [[TransitionHookFn]] that skips a transition if it should be ignored
Expand All @@ -13,10 +14,21 @@ import { Transition } from '../transition/transition';
* then the transition is ignored and not processed.
*/
function ignoredHook(trans: Transition) {
if (trans.ignored()) {
trace.traceTransitionIgnored(this);
return Rejection.ignored().toPromise();
const ignoredReason = trans._ignoredReason();
if (!ignoredReason) return;

trace.traceTransitionIgnored(trans);

const pending = trans.router.globals.transition;

// The user clicked a link going back to the *current state* ('A')
// However, there is also a pending transition in flight (to 'B')
// Abort the transition to 'B' because the user now wants to be back at 'A'.
if (ignoredReason === 'SameAsCurrent' && pending) {
pending.abort();
}

return Rejection.ignored().toPromise();
}

export const registerIgnoredTransitionHook = (transitionService: TransitionService) =>
Expand Down
2 changes: 1 addition & 1 deletion src/path/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
/** @module path */ /** for typedoc */
export * from "./node";
export * from "./pathNode";
export * from "./pathFactory";
64 changes: 54 additions & 10 deletions src/path/pathFactory.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
/** @module path */ /** for typedoc */

import {extend, find, pick, omit, tail, mergeR, values, unnestR, Predicate, inArray} from "../common/common";
import {
extend, find, pick, omit, tail, mergeR, values, unnestR, Predicate, inArray, arrayTuples,
} from "../common/common";
import {prop, propEq, not} from "../common/hof";

import {RawParams} from "../params/interface";
Expand All @@ -10,13 +12,14 @@ import {_ViewDeclaration} from "../state/interface";

import {StateObject} from "../state/stateObject";
import {TargetState} from "../state/targetState";
import {PathNode} from "../path/node";
import {GetParamsFn, PathNode} from "./pathNode";
import {ViewService} from "../view/view";
import { Param } from '../params/param';

/**
* This class contains functions which convert TargetStates, Nodes and paths from one type to another.
*/
export class PathFactory {
export class PathUtils {

constructor() { }

Expand All @@ -33,9 +36,9 @@ export class PathFactory {

/** Given a fromPath: PathNode[] and a TargetState, builds a toPath: PathNode[] */
static buildToPath(fromPath: PathNode[], targetState: TargetState): PathNode[] {
let toPath: PathNode[] = PathFactory.buildPath(targetState);
let toPath: PathNode[] = PathUtils.buildPath(targetState);
if (targetState.options().inherit) {
return PathFactory.inheritParams(fromPath, toPath, Object.keys(targetState.params()));
return PathUtils.inheritParams(fromPath, toPath, Object.keys(targetState.params()));
}
return toPath;
}
Expand All @@ -49,7 +52,7 @@ export class PathFactory {
// Only apply the viewConfigs to the nodes for the given states
path.filter(node => inArray(states, node.state)).forEach(node => {
let viewDecls: _ViewDeclaration[] = values(node.state.views || {});
let subPath = PathFactory.subPath(path, n => n === node);
let subPath = PathUtils.subPath(path, n => n === node);
let viewConfigs: ViewConfig[][] = viewDecls.map(view => $view.createViewConfig(subPath, view));
node.views = viewConfigs.reduce(unnestR, []);
});
Expand Down Expand Up @@ -97,15 +100,18 @@ export class PathFactory {
return <PathNode[]> toPath.map(makeInheritedParamsNode);
}

static nonDynamicParams = (node: PathNode): Param[] =>
node.state.parameters({ inherit: false })
.filter(param => !param.dynamic);

/**
* Computes the tree changes (entering, exiting) between a fromPath and toPath.
*/
static treeChanges(fromPath: PathNode[], toPath: PathNode[], reloadState: StateObject): TreeChanges {
let keep = 0, max = Math.min(fromPath.length, toPath.length);
const staticParams = (state: StateObject) =>
state.parameters({ inherit: false }).filter(not(prop('dynamic'))).map(prop('id'));

const nodesMatch = (node1: PathNode, node2: PathNode) =>
node1.equals(node2, staticParams(node1.state));
node1.equals(node2, PathUtils.nonDynamicParams);

while (keep < max && fromPath[keep].state !== reloadState && nodesMatch(fromPath[keep], toPath[keep])) {
keep++;
Expand All @@ -132,6 +138,43 @@ export class PathFactory {
return { from, to, retained, exiting, entering };
}

/**
* Returns a new path which is: the subpath of the first path which matches the second path.
*
* The new path starts from root and contains any nodes that match the nodes in the second path.
* It stops before the first non-matching node.
*
* Nodes are compared using their state property and their parameter values.
* If a `paramsFn` is provided, only the [[Param]] returned by the function will be considered when comparing nodes.
*
* @param pathA the first path
* @param pathB the second path
* @param paramsFn a function which returns the parameters to consider when comparing
*
* @returns an array of PathNodes from the first path which match the nodes in the second path
*/
static matching(pathA: PathNode[], pathB: PathNode[], paramsFn?: GetParamsFn): PathNode[] {
let done = false;
let tuples: PathNode[][] = arrayTuples(pathA, pathB);
return tuples.reduce((matching, [nodeA, nodeB]) => {
done = done || !nodeA.equals(nodeB, paramsFn);
return done ? matching : matching.concat(nodeA);
}, []);
}

/**
* Returns true if two paths are identical.
*
* @param pathA
* @param pathB
* @param paramsFn a function which returns the parameters to consider when comparing
* @returns true if the the states and parameter values for both paths are identical
*/
static equals(pathA: PathNode[], pathB: PathNode[], paramsFn?: GetParamsFn): boolean {
return pathA.length === pathB.length &&
PathUtils.matching(pathA, pathB, paramsFn).length === pathA.length;
}

/**
* Return a subpath of a path, which stops at the first matching node
*
Expand All @@ -149,5 +192,6 @@ export class PathFactory {
}

/** Gets the raw parameter values from a path */
static paramValues = (path: PathNode[]) => path.reduce((acc, node) => extend(acc, node.paramValues), {});
static paramValues = (path: PathNode[]) =>
path.reduce((acc, node) => extend(acc, node.paramValues), {});
}
67 changes: 31 additions & 36 deletions src/path/node.ts → src/path/pathNode.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/** @module path */ /** for typedoc */
import {extend, applyPairs, find, allTrueR} from "../common/common";
import {extend, applyPairs, find, allTrueR, pairs, arrayTuples} from "../common/common";
import {propEq} from "../common/hof";
import {StateObject} from "../state/stateObject";
import {RawParams} from "../params/interface";
Expand All @@ -8,6 +8,8 @@ import {Resolvable} from "../resolve/resolvable";
import {ViewConfig} from "../view/interface";

/**
* @internalapi
*
* A node in a [[TreeChanges]] path
*
* For a [[TreeChanges]] path, this class holds the stateful information for a single node in the path.
Expand All @@ -27,19 +29,19 @@ export class PathNode {
public views: ViewConfig[];

/** Creates a copy of a PathNode */
constructor(state: PathNode);
constructor(node: PathNode);
/** Creates a new (empty) PathNode for a State */
constructor(state: StateObject);
constructor(stateOrPath: any) {
if (stateOrPath instanceof PathNode) {
let node: PathNode = stateOrPath;
constructor(stateOrNode: any) {
if (stateOrNode instanceof PathNode) {
let node: PathNode = stateOrNode;
this.state = node.state;
this.paramSchema = node.paramSchema.slice();
this.paramValues = extend({}, node.paramValues);
this.resolvables = node.resolvables.slice();
this.views = node.views && node.views.slice();
} else {
let state: StateObject = stateOrPath;
let state: StateObject = stateOrNode;
this.state = state;
this.paramSchema = state.parameters({ inherit: false });
this.paramValues = {};
Expand All @@ -63,42 +65,35 @@ export class PathNode {
* @returns true if the state and parameter values for another PathNode are
* equal to the state and param values for this PathNode
*/
equals(node: PathNode, keys = this.paramSchema.map(p => p.id)): boolean {
const paramValsEq = (key: string) =>
this.parameter(key).type.equals(this.paramValues[key], node.paramValues[key]);
return this.state === node.state && keys.map(paramValsEq).reduce(allTrueR, true);
}

/** Returns a clone of the PathNode */
static clone(node: PathNode) {
return new PathNode(node);
equals(node: PathNode, paramsFn?: GetParamsFn): boolean {
const diff = this.diff(node, paramsFn);
return diff && diff.length === 0;
}

/**
* Returns a new path which is a subpath of the first path which matched the second path.
* Finds Params with different parameter values on another PathNode.
*
* The new path starts from root and contains any nodes that match the nodes in the second path.
* Nodes are compared using their state property and parameter values.
* Given another node (of the same state), finds the parameter values which differ.
* Returns the [[Param]] (schema objects) whose parameter values differ.
*
* @param pathA the first path
* @param pathB the second path
* @param ignoreDynamicParams don't compare dynamic parameter values
* Given another node for a different state, returns `false`
*
* @param node The node to compare to
* @param paramsFn A function that returns which parameters should be compared.
* @returns The [[Param]]s which differ, or null if the two nodes are for different states
*/
static matching(pathA: PathNode[], pathB: PathNode[], ignoreDynamicParams = true): PathNode[] {
let matching: PathNode[] = [];

for (let i = 0; i < pathA.length && i < pathB.length; i++) {
let a = pathA[i], b = pathB[i];
diff(node: PathNode, paramsFn?: GetParamsFn): Param[] | false {
if (this.state !== node.state) return false;

if (a.state !== b.state) break;

let changedParams = Param.changed(a.paramSchema, a.paramValues, b.paramValues)
.filter(param => !(ignoreDynamicParams && param.dynamic));
if (changedParams.length) break;

matching.push(a);
}
const params: Param[] = paramsFn ? paramsFn(this) : this.paramSchema;
return Param.changed(params, this.paramValues, node.paramValues);
}

return matching
/** Returns a clone of the PathNode */
static clone(node: PathNode) {
return new PathNode(node);
}
}
}

/** @hidden */
export type GetParamsFn = (pathNode: PathNode) => Param[];
2 changes: 1 addition & 1 deletion src/resolve/resolvable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {stringify} from "../common/strings";
import {isFunction, isObject} from "../common/predicates";
import {Transition} from "../transition/transition";
import {StateObject} from "../state/stateObject";
import {PathNode} from "../path/node";
import {PathNode} from "../path/pathNode";


// TODO: explicitly make this user configurable
Expand Down
8 changes: 4 additions & 4 deletions src/resolve/resolveContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ import { propEq, not } from "../common/hof";
import { trace } from "../common/trace";
import { services, $InjectorLike } from "../common/coreservices";
import { resolvePolicies, PolicyWhen, ResolvePolicy } from "./interface";
import { PathNode } from "../path/node";
import { PathNode } from "../path/pathNode";
import { Resolvable } from "./resolvable";
import { StateObject } from "../state/stateObject";
import { PathFactory } from "../path/pathFactory";
import { PathUtils } from "../path/pathFactory";
import { stringify } from "../common/strings";
import { Transition } from "../transition/transition";
import { UIInjector } from "../interface";
Expand Down Expand Up @@ -82,7 +82,7 @@ export class ResolveContext {
* `let AB = ABCD.subcontext(a)`
*/
subContext(state: StateObject): ResolveContext {
return new ResolveContext(PathFactory.subPath(this._path, node => node.state === state));
return new ResolveContext(PathUtils.subPath(this._path, node => node.state === state));
}

/**
Expand Down Expand Up @@ -164,7 +164,7 @@ export class ResolveContext {
let node = this.findNode(resolvable);
// Find which other resolvables are "visible" to the `resolvable` argument
// subpath stopping at resolvable's node, or the whole path (if the resolvable isn't in the path)
let subPath: PathNode[] = PathFactory.subPath(this._path, x => x === node) || this._path;
let subPath: PathNode[] = PathUtils.subPath(this._path, x => x === node) || this._path;
let availableResolvables: Resolvable[] = subPath
.reduce((acc, node) => acc.concat(node.resolvables), []) //all of subpath's resolvables
.filter(res => res !== resolvable); // filter out the `resolvable` argument
Expand Down
14 changes: 8 additions & 6 deletions src/state/stateService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import { isDefined, isObject, isString } from '../common/predicates';
import { Queue } from '../common/queue';
import { services } from '../common/coreservices';

import { PathFactory } from '../path/pathFactory';
import { PathNode } from '../path/node';
import { PathUtils } from '../path/pathFactory';
import { PathNode } from '../path/pathNode';

import { HookResult, TransitionOptions } from '../transition/interface';
import { defaultTransOpts } from '../transition/transitionService';
Expand Down Expand Up @@ -93,7 +93,7 @@ export class StateService {
* @internalapi
*/
private _handleInvalidTargetState(fromPath: PathNode[], toState: TargetState) {
let fromState = PathFactory.makeTargetState(fromPath);
let fromState = PathUtils.makeTargetState(fromPath);
let globals = this.router.globals;
const latestThing = () => globals.transitionHistory.peekTail();
let latest = latestThing();
Expand Down Expand Up @@ -340,9 +340,11 @@ export class StateService {
*/
const rejectedTransitionHandler = (transition: Transition) => (error: any): Promise<any> => {
if (error instanceof Rejection) {
const isLatest = router.globals.lastStartedTransitionId === transition.$id;

if (error.type === RejectType.IGNORED) {
isLatest && router.urlRouter.update();
// Consider ignored `Transition.run()` as a successful `transitionTo`
router.urlRouter.update();
return services.$q.when(globals.current);
}

Expand All @@ -355,7 +357,7 @@ export class StateService {
}

if (error.type === RejectType.ABORTED) {
router.urlRouter.update();
isLatest && router.urlRouter.update();
return services.$q.reject(error);
}
}
Expand Down Expand Up @@ -593,7 +595,7 @@ export class StateService {
if (!state || !state.lazyLoad) throw new Error("Can not lazy load " + stateOrName);

let currentPath = this.getCurrentPath();
let target = PathFactory.makeTargetState(currentPath);
let target = PathUtils.makeTargetState(currentPath);
transition = transition || this.router.transitionService.create(currentPath, target);

return lazyLoadState(transition, state);
Expand Down
2 changes: 1 addition & 1 deletion src/transition/hookBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
import {Transition} from "./transition";
import {TransitionHook} from "./transitionHook";
import {StateObject} from "../state/stateObject";
import {PathNode} from "../path/node";
import {PathNode} from "../path/pathNode";
import {TransitionService} from "./transitionService";
import {TransitionEventType} from "./transitionEventType";
import {RegisteredHook} from "./hookRegistry";
Expand Down
2 changes: 1 addition & 1 deletion src/transition/hookRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/ /** for typedoc */
import { extend, removeFrom, tail, values, identity, map } from "../common/common";
import {isString, isFunction} from "../common/predicates";
import {PathNode} from "../path/node";
import {PathNode} from "../path/pathNode";
import {
TransitionStateHookFn, TransitionHookFn, TransitionHookPhase, TransitionHookScope, IHookRegistry, PathType
} from "./interface"; // has or is using
Expand Down
2 changes: 1 addition & 1 deletion src/transition/interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {Predicate} from "../common/common";

import {Transition} from "./transition";
import {StateObject} from "../state/stateObject";
import {PathNode} from "../path/node";
import {PathNode} from "../path/pathNode";
import {TargetState} from "../state/targetState";
import {RegisteredHook} from "./hookRegistry";

Expand Down
Loading

0 comments on commit bd1bd0b

Please sign in to comment.