From 3df5490e364d08f0ee15b1f10759623780626a3e Mon Sep 17 00:00:00 2001 From: Tomina Date: Sat, 30 Jan 2021 18:54:43 +0100 Subject: [PATCH] fix(selection): deselect events not being fired (#1310) --- cypress/integration/visual/selection.spec.ts | 99 +++++++++++++++++++- cypress/pages/standard-cytest-script.ts | 61 ++++++++++-- cypress/support/commands/index.ts | 1 + cypress/support/commands/types.ts | 12 ++- cypress/support/commands/vis-assert-event.ts | 77 +++++++++++++++ cypress/support/commands/vis-run.ts | 23 +++-- lib/network/Network.js | 1 + lib/network/modules/SelectionHandler.js | 12 ++- test/network/selection.test.js | 52 ++++++++++ 9 files changed, 317 insertions(+), 21 deletions(-) create mode 100644 cypress/support/commands/vis-assert-event.ts diff --git a/cypress/integration/visual/selection.spec.ts b/cypress/integration/visual/selection.spec.ts index d5a258259e..353c6e4be2 100644 --- a/cypress/integration/visual/selection.spec.ts +++ b/cypress/integration/visual/selection.spec.ts @@ -22,7 +22,7 @@ context("Selection", (): void => { ], physics: false, }, - { requireNewerVersionThan: "8.5.3" } + { requireNewerVersionThan: "9.0.0" } ); }); @@ -32,12 +32,34 @@ context("Selection", (): void => { nodes: ["N_1"], edges: ["E_1-2", "E_1-3", "E_1-4", "E_1-5"], }); + cy.visShiftAndAssertEventFired("select"); + cy.visShiftAndAssertEventFired("selectEdge"); + cy.visShiftAndAssertEventFired("selectNode"); + cy.visShiftAndAssertEventNone( + "deselectEdge", + "deselectNode", + "select", + "selectEdge", + "selectNode" + ); cy.visClickPoint({ x: 500 + 200, y: 500 + 0 }); cy.visAssertSelection({ nodes: ["N_2"], edges: ["E_5-2", "E_1-2", "E_2-3"], }); + cy.visShiftAndAssertEventFired("deselectEdge"); + cy.visShiftAndAssertEventFired("deselectNode"); + cy.visShiftAndAssertEventFired("select"); + cy.visShiftAndAssertEventFired("selectEdge"); + cy.visShiftAndAssertEventFired("selectNode"); + cy.visShiftAndAssertEventNone( + "deselectEdge", + "deselectNode", + "select", + "selectEdge", + "selectNode" + ); cy.visSnapshotOpenedPage("select-one-by-click"); }); @@ -55,6 +77,13 @@ context("Selection", (): void => { nodes: [], edges: [], }); + cy.visShiftAndAssertEventNone( + "deselectEdge", + "deselectNode", + "select", + "selectEdge", + "selectNode" + ); cy.visSnapshotOpenedPage("select-none-by-single-drag"); }); @@ -72,6 +101,16 @@ context("Selection", (): void => { nodes: ["N_1"], edges: ["E_1-2", "E_1-3", "E_1-4", "E_1-5"], }); + cy.visShiftAndAssertEventFired("select"); + cy.visShiftAndAssertEventFired("selectEdge"); + cy.visShiftAndAssertEventFired("selectNode"); + cy.visShiftAndAssertEventNone( + "deselectEdge", + "deselectNode", + "select", + "selectEdge", + "selectNode" + ); cy.visSnapshotOpenedPage("select-one-by-single-drag-(TL_to_BR)"); }); @@ -89,6 +128,16 @@ context("Selection", (): void => { nodes: ["N_1", "N_2", "N_3"], edges: ["E_1-2", "E_1-3", "E_1-4", "E_1-5", "E_3-4", "E_2-3", "E_5-2"], }); + cy.visShiftAndAssertEventFired("select"); + cy.visShiftAndAssertEventFired("selectEdge"); + cy.visShiftAndAssertEventFired("selectNode"); + cy.visShiftAndAssertEventNone( + "deselectEdge", + "deselectNode", + "select", + "selectEdge", + "selectNode" + ); cy.visSnapshotOpenedPage("select-three-by-single-drag-(BR_to_TL)"); }); @@ -106,6 +155,16 @@ context("Selection", (): void => { nodes: ["N_1", "N_3"], edges: ["E_1-2", "E_1-3", "E_1-4", "E_1-5", "E_3-4", "E_2-3"], }); + cy.visShiftAndAssertEventFired("select"); + cy.visShiftAndAssertEventFired("selectEdge"); + cy.visShiftAndAssertEventFired("selectNode"); + cy.visShiftAndAssertEventNone( + "deselectEdge", + "deselectNode", + "select", + "selectEdge", + "selectNode" + ); cy.visDrag([ { @@ -119,6 +178,16 @@ context("Selection", (): void => { nodes: ["N_1", "N_2", "N_3"], edges: ["E_1-2", "E_1-3", "E_1-4", "E_1-5", "E_3-4", "E_2-3", "E_5-2"], }); + cy.visShiftAndAssertEventFired("select"); + cy.visShiftAndAssertEventFired("selectEdge"); + cy.visShiftAndAssertEventFired("selectNode"); + cy.visShiftAndAssertEventNone( + "deselectEdge", + "deselectNode", + "select", + "selectEdge", + "selectNode" + ); cy.visSnapshotOpenedPage( "select-three-by-two-drags-(TR_to_BL_then_BL_to_TR)" @@ -132,6 +201,13 @@ context("Selection", (): void => { edges: ["E_1-2", "E_1-3", "E_1-4", "E_1-5", "E_3-4", "E_2-3"], }); }); + cy.visShiftAndAssertEventNone( + "deselectEdge", + "deselectNode", + "select", + "selectEdge", + "selectNode" + ); cy.visSnapshotOpenedPage("select-via-method"); }); @@ -140,6 +216,13 @@ context("Selection", (): void => { cy.visRun(({ network }): void => { network.setSelection({ nodes: ["N_1", "N_5"] }, { unselectAll: false }); }); + cy.visShiftAndAssertEventNone( + "deselectEdge", + "deselectNode", + "select", + "selectEdge", + "selectNode" + ); cy.visSnapshotOpenedPage("programmatic-select-nodes"); cy.visRun(({ network }): void => { @@ -148,11 +231,25 @@ context("Selection", (): void => { { unselectAll: false } ); }); + cy.visShiftAndAssertEventNone( + "deselectEdge", + "deselectNode", + "select", + "selectEdge", + "selectNode" + ); cy.visSnapshotOpenedPage("programmatic-select-edges"); cy.visRun(({ network }): void => { network.unselectAll(); }); + cy.visShiftAndAssertEventNone( + "deselectEdge", + "deselectNode", + "select", + "selectEdge", + "selectNode" + ); cy.visSnapshotOpenedPage("programmatic-select-unselect-all"); }); }); diff --git a/cypress/pages/standard-cytest-script.ts b/cypress/pages/standard-cytest-script.ts index 7a30bcc211..4027eab065 100644 --- a/cypress/pages/standard-cytest-script.ts +++ b/cypress/pages/standard-cytest-script.ts @@ -1,9 +1,9 @@ // These imports are there only for their types. Their values can't be used as // this will be loaded in a web browser without bundling. -import * as visNetworkStandalone from "../../standalone"; -import * as visUtil from "vis-util"; -import { Options } from "../../standalone"; -import { UniversalConfig } from "../support/commands/types"; +import type * as visNetworkStandalone from "../../standalone"; +import type * as visUtil from "vis-util"; +import type { Options } from "../../standalone"; +import type { UniversalConfig, VisWindow } from "../support/commands/types"; type VisNetworkStandalone = typeof visNetworkStandalone; type VisUtil = typeof visUtil; @@ -237,10 +237,55 @@ type VisUtil = typeof visUtil; }); }); - (window as any).visEdges = edges; - (window as any).visLastEvents = {}; - (window as any).visNetwork = network; - (window as any).visNodes = nodes; + // Event queue: + const eventQueue: VisWindow["visEventQueue"] = {} as any; + ([ + "afterDrawing", + "animationFinished", + "beforeDrawing", + "blurEdge", + "blurNode", + "click", + "configChange", + "controlNodeDragEnd", + "controlNodeDragging", + "deselectEdge", + "deselectNode", + "doubleClick", + "dragEnd", + "dragStart", + "dragging", + "hidePopup", + "hold", + "hoverEdge", + "hoverNode", + "initRedraw", + "oncontext", + "release", + "resize", + "select", + "selectEdge", + "selectNode", + "showPopup", + "stabilizationIterationsDone", + "stabilizationProgress", + "stabilized", + "startStabilizing", + "zoom", + ] as const).forEach((eventName): void => { + eventQueue[eventName] = []; + network.on(eventName, (params: any): void => { + eventQueue[eventName].push({ params }); + }); + }); + + Object.assign(window, { + visEdges: edges, + visEventQueue: eventQueue, + visLastEvents: {}, + visNetwork: network, + visNodes: nodes, + }); $status.innerText = "Ready"; })(); diff --git a/cypress/support/commands/index.ts b/cypress/support/commands/index.ts index 6674d12556..5c7e27f36b 100644 --- a/cypress/support/commands/index.ts +++ b/cypress/support/commands/index.ts @@ -1,5 +1,6 @@ export * from "./types"; +export * from "./vis-assert-event"; export * from "./vis-assert-selection"; export * from "./vis-check-ids"; export * from "./vis-click-betweek-points"; diff --git a/cypress/support/commands/types.ts b/cypress/support/commands/types.ts index 48268faaa7..682227d2eb 100644 --- a/cypress/support/commands/types.ts +++ b/cypress/support/commands/types.ts @@ -1,7 +1,8 @@ -import { +import type { DataSetEdges, DataSetNodes, Network, + NetworkEvents, Options, } from "../../../declarations/entry-standalone"; @@ -11,11 +12,20 @@ export * from "../../../standalone"; export interface VisGlobals { edges: DataSetEdges; + eventQueue: Record; lastEvents: Record; network: Network; nodes: DataSetNodes; } +export type VisWindow = { + visEdges: VisGlobals["edges"]; + visEventQueue: VisGlobals["eventQueue"]; + visLastEvents: VisGlobals["lastEvents"]; + visNetwork: VisGlobals["network"]; + visNodes: VisGlobals["nodes"]; +}; + export interface Point { x: number; y: number; diff --git a/cypress/support/commands/vis-assert-event.ts b/cypress/support/commands/vis-assert-event.ts new file mode 100644 index 0000000000..ac0aa9c485 --- /dev/null +++ b/cypress/support/commands/vis-assert-event.ts @@ -0,0 +1,77 @@ +import { NetworkEvents } from "./types"; + +declare global { + namespace Cypress { + interface Chainable { + /** + * Check that given event was fired. + * + * @param name - The name of the event as used by Network.on(). + */ + visShiftAndAssertEventFired(name: NetworkEvents): Chainable; + /** + * Check that given event was fired. + * + * @param name - The name of the event as used by Network.on(). + * @param params - The params of the event as supplied by Network.on(). + */ + visShiftAndAssertEventFiredWithParams( + name: NetworkEvents, + params: any + ): Chainable; + /** + * Check that given event was not fired. + * + * @param name - The name of the event as used by Network.on(). + */ + visShiftAndAssertEventNone( + ...names: readonly NetworkEvents[] + ): Chainable; + } + } +} + +export function visShiftAndAssertEventFired(name: NetworkEvents): void { + cy.visRun( + async ({ eventQueue }): Promise => { + expect(eventQueue[name], `${name} event queue`).to.not.be.empty; + eventQueue[name].shift(); + } + ); +} +Cypress.Commands.add( + "visShiftAndAssertEventFired", + visShiftAndAssertEventFired +); + +export function visShiftAndAssertEventFiredWithParams( + name: NetworkEvents, + params: any +): void { + cy.visRun( + async ({ eventQueue }): Promise => { + expect(eventQueue[name], `${name} event queue`).to.not.be.empty; + const event = eventQueue[name].shift(); + expect(event, `${name} event`) + .to.have.ownProperty("params") + .that.deep.equals(params); + } + ); +} +Cypress.Commands.add( + "visShiftAndAssertEventFiredWithParams", + visShiftAndAssertEventFiredWithParams +); + +export function visShiftAndAssertEventNone( + ...names: readonly NetworkEvents[] +): void { + cy.visRun( + async ({ eventQueue }): Promise => { + for (const name of names) { + expect(eventQueue[name], `${name} event queue`).to.be.empty; + } + } + ); +} +Cypress.Commands.add("visShiftAndAssertEventNone", visShiftAndAssertEventNone); diff --git a/cypress/support/commands/vis-run.ts b/cypress/support/commands/vis-run.ts index ce0aab4419..fc73cdbc22 100644 --- a/cypress/support/commands/vis-run.ts +++ b/cypress/support/commands/vis-run.ts @@ -1,4 +1,4 @@ -import { VisGlobals } from "./types"; +import { VisGlobals, VisWindow } from "./types"; export interface VisRunOptions { /** @@ -19,7 +19,7 @@ declare global { * @param options - Additional options. */ visRun( - callback: (props: VisGlobals) => void, + callback: (props: VisGlobals) => void | Promise, options?: VisRunOptions ): Chainable; } @@ -27,19 +27,22 @@ declare global { } export function visRun( - callback: (props: VisGlobals) => void, + callback: (props: VisGlobals) => void | Promise, { timeout = VIS_DEFAULT_RUN_TIMEOUT }: VisRunOptions = {} ): void { cy.window().then( { timeout }, - ({ - visEdges: edges, - visLastEvents: lastEvents, - visNetwork: network, - visNodes: nodes, - }: any): void => { + async (win: any): Promise => { + const { + visEdges: edges, + visEventQueue: eventQueue, + visLastEvents: lastEvents, + visNetwork: network, + visNodes: nodes, + }: VisWindow = win; + if (edges && lastEvents && network && nodes) { - callback({ edges, lastEvents, network, nodes }); + await callback({ edges, eventQueue, lastEvents, network, nodes }); } else { throw new Error("No page globals were found."); } diff --git a/lib/network/Network.js b/lib/network/Network.js index 306fb3d587..89ec54ee3c 100644 --- a/lib/network/Network.js +++ b/lib/network/Network.js @@ -732,6 +732,7 @@ Network.prototype.selectEdges = function () { }; Network.prototype.unselectAll = function () { this.selectionHandler.unselectAll.apply(this.selectionHandler, arguments); + this.selectionHandler.commitWithoutEmitting.apply(this.selectionHandler); this.redraw(); }; Network.prototype.redraw = function () { diff --git a/lib/network/modules/SelectionHandler.js b/lib/network/modules/SelectionHandler.js index c90c05ae1f..2666baa436 100644 --- a/lib/network/modules/SelectionHandler.js +++ b/lib/network/modules/SelectionHandler.js @@ -15,6 +15,10 @@ class SelectionHandler { constructor(body, canvas) { this.body = body; this.canvas = canvas; + // TODO: Consider firing an event on any change to the selection, not + // only those caused by clicks and taps. It would be easy to implement + // now and (at least to me) it seems like something that could be + // quite useful. this._selectionAccumulator = new SelectionAccumulator(); this.hoverObj = { nodes: {}, edges: {} }; @@ -363,7 +367,6 @@ class SelectionHandler { */ unselectAll() { this._selectionAccumulator.clear(); - this._selectionAccumulator.commit(); } /** @@ -527,6 +530,13 @@ class SelectionHandler { } } + /** + * Commit the selection changes but don't emit any events. + */ + commitWithoutEmitting() { + this._selectionAccumulator.commit(); + } + /** * Select and deselect nodes depending current selection change. * diff --git a/test/network/selection.test.js b/test/network/selection.test.js index dfd8c3feaa..14b2cdd3da 100644 --- a/test/network/selection.test.js +++ b/test/network/selection.test.js @@ -39,6 +39,30 @@ describe("Network", function () { { physics: false } ); + const events = { + deselectEdge: [], + deselectNode: [], + select: [], + selectEdge: [], + selectNode: [], + }; + function resetEvents() { + Object.values(events).forEach((array) => array.splice(0)); + } + + network.on("deselectEdge", (...rest) => events["deselectEdge"].push(rest)); + network.on("deselectNode", (...rest) => events["deselectNode"].push(rest)); + network.on("select", (...rest) => events["select"].push(rest)); + network.on("selectEdge", (...rest) => events["selectEdge"].push(rest)); + network.on("selectNode", (...rest) => events["selectNode"].push(rest)); + + expect(events["deselectEdge"]).to.have.length(0); + expect(events["deselectNode"]).to.have.length(0); + expect(events["select"]).to.have.length(0); + expect(events["selectEdge"]).to.have.length(0); + expect(events["selectNode"]).to.have.length(0); + resetEvents(); + // Select a node with it's edges. network.setSelection({ nodes: ["N_5"] }, { unselectAll: false }); expect(sortArrays(network.getSelection())).to.deep.equal( @@ -48,6 +72,13 @@ describe("Network", function () { }) ); + expect(events["deselectEdge"]).to.have.length(0); + expect(events["deselectNode"]).to.have.length(0); + expect(events["select"]).to.have.length(0); + expect(events["selectEdge"]).to.have.length(0); + expect(events["selectNode"]).to.have.length(0); + resetEvents(); + // Select a node without it's edges. network.setSelection( { nodes: ["N_1"] }, @@ -60,6 +91,13 @@ describe("Network", function () { }) ); + expect(events["deselectEdge"]).to.have.length(0); + expect(events["deselectNode"]).to.have.length(0); + expect(events["select"]).to.have.length(0); + expect(events["selectEdge"]).to.have.length(0); + expect(events["selectNode"]).to.have.length(0); + resetEvents(); + // Select some edges. network.setSelection({ edges: ["E_2-3", "E_3-4"] }, { unselectAll: false }); expect(sortArrays(network.getSelection())).to.deep.equal( @@ -69,6 +107,13 @@ describe("Network", function () { }) ); + expect(events["deselectEdge"]).to.have.length(0); + expect(events["deselectNode"]).to.have.length(0); + expect(events["select"]).to.have.length(0); + expect(events["selectEdge"]).to.have.length(0); + expect(events["selectNode"]).to.have.length(0); + resetEvents(); + // Unselect all. network.unselectAll(); expect(sortArrays(network.getSelection())).to.deep.equal( @@ -77,5 +122,12 @@ describe("Network", function () { edges: [], }) ); + + expect(events["deselectEdge"]).to.have.length(0); + expect(events["deselectNode"]).to.have.length(0); + expect(events["select"]).to.have.length(0); + expect(events["selectEdge"]).to.have.length(0); + expect(events["selectNode"]).to.have.length(0); + resetEvents(); }); });