From 7dc0fe736be9ac59d8f532b4711b55192c19d434 Mon Sep 17 00:00:00 2001 From: AlexIchenskiy Date: Thu, 14 Mar 2024 11:46:45 +0100 Subject: [PATCH 1/5] Chore: Refactor position setter options --- src/models/graph.ts | 4 ++-- src/models/node.ts | 22 ++++++++++++++++------ src/views/orb-view.ts | 2 +- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/models/graph.ts b/src/models/graph.ts index 64fa524..a8e699f 100644 --- a/src/models/graph.ts +++ b/src/models/graph.ts @@ -200,7 +200,7 @@ export class Graph extends Subject imp for (let i = 0; i < positions.length; i++) { const node = this._nodes.getOne(positions[i].id); if (node) { - node.setPosition(positions[i], true); + node.setPosition(positions[i], { isNotifySkipped: true }); } } } @@ -422,7 +422,7 @@ export class Graph extends Subject imp const existingNode = this.getNodeById(nodes[i].id); if (existingNode) { existingNode.setData(nodes[i]); - existingNode.setPosition(nodes[i], true); + existingNode.setPosition(nodes[i], { isNotifySkipped: true }); continue; } diff --git a/src/models/node.ts b/src/models/node.ts index 1900c72..ddded59 100644 --- a/src/models/node.ts +++ b/src/models/node.ts @@ -34,6 +34,10 @@ export interface INodeMapCoordinates { lng: number; } +export interface INodeSetPositionOptions { + isNotifySkipped: boolean; +} + export enum NodeShapeType { CIRCLE = 'circle', DOT = 'dot', @@ -113,10 +117,13 @@ export interface INode extends ISubjec setData(callback: (node: INode) => N): void; patchData(data: Partial): void; patchData(callback: (node: INode) => Partial): void; - setPosition(position: INodeCoordinates | INodeMapCoordinates | INodePosition, call: boolean): void; + setPosition( + position: INodeCoordinates | INodeMapCoordinates | INodePosition, + options?: INodeSetPositionOptions, + ): void; setPosition( callback: (node: INode) => INodeCoordinates | INodeMapCoordinates | INodePosition, - call: boolean, + options?: INodeSetPositionOptions, ): void; setStyle(style: INodeStyle): void; setStyle(callback: (node: INode) => INodeStyle): void; @@ -457,10 +464,13 @@ export class Node extends Subject impl this.notifyListeners(); } - setPosition(position: INodeCoordinates | INodeMapCoordinates | INodePosition, isInner?: boolean): void; + setPosition( + position: INodeCoordinates | INodeMapCoordinates | INodePosition, + options?: INodeSetPositionOptions, + ): void; setPosition( callback: (node: INode) => INodeCoordinates | INodeMapCoordinates | INodePosition, - isInner?: boolean, + options?: INodeSetPositionOptions, ): void; setPosition( arg: @@ -468,7 +478,7 @@ export class Node extends Subject impl | INodeMapCoordinates | INodePosition | ((node: INode) => INodeCoordinates | INodeMapCoordinates | INodePosition), - isInner?: boolean, + options?: INodeSetPositionOptions, ) { let position: INodeCoordinates | INodeMapCoordinates | INodePosition; if (isFunction(arg)) { @@ -493,7 +503,7 @@ export class Node extends Subject impl }; } - if (!isInner) { + if (!options?.isNotifySkipped) { this.notifyListeners({ id: this.id, ...position }); } } diff --git a/src/views/orb-view.ts b/src/views/orb-view.ts index d2ee65f..4773064 100644 --- a/src/views/orb-view.ts +++ b/src/views/orb-view.ts @@ -286,7 +286,7 @@ export class OrbView for (let i = 0; i < nodes.length; i++) { const position = this._settings.getPosition(nodes[i]); if (position) { - nodes[i].setPosition({ id: nodes[i].getId(), ...position }, true); + nodes[i].setPosition({ id: nodes[i].getId(), ...position }, { isNotifySkipped: true }); } } } From 84921da81f9b316150ab7b98d46d70ad97d7c262 Mon Sep 17 00:00:00 2001 From: AlexIchenskiy Date: Thu, 14 Mar 2024 13:03:50 +0100 Subject: [PATCH 2/5] Chore: Refactor property patch function --- src/utils/object.utils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils/object.utils.ts b/src/utils/object.utils.ts index fbb7a67..d6dfcf9 100644 --- a/src/utils/object.utils.ts +++ b/src/utils/object.utils.ts @@ -122,9 +122,9 @@ const copyPlainObject = (obj: Record): Record => { }; export const patchProperties = (target: T, source: T): void => { - const keys = Object.keys(source as Object); + const keys = Object.keys(source as Object) as (keyof T)[]; for (let i = 0; i < keys.length; i++) { - (target[keys[i] as keyof T] as T[keyof T]) = source[keys[i] as keyof T] as T[keyof T]; + target[keys[i]] = source[keys[i]]; } }; From f9e13a06b4afc7d8a21a534bb8a6281ac5e38b4c Mon Sep 17 00:00:00 2001 From: AlexIchenskiy Date: Thu, 14 Mar 2024 13:20:07 +0100 Subject: [PATCH 3/5] Fix: Set map node position behaviour --- src/models/node.ts | 46 ++++++------------------------------- src/utils/observer.utils.ts | 4 ++-- src/views/orb-map-view.ts | 2 +- 3 files changed, 10 insertions(+), 42 deletions(-) diff --git a/src/models/node.ts b/src/models/node.ts index ddded59..1b48741 100644 --- a/src/models/node.ts +++ b/src/models/node.ts @@ -29,11 +29,6 @@ export interface INodeCoordinates { y: number; } -export interface INodeMapCoordinates { - lat: number; - lng: number; -} - export interface INodeSetPositionOptions { isNotifySkipped: boolean; } @@ -117,12 +112,9 @@ export interface INode extends ISubjec setData(callback: (node: INode) => N): void; patchData(data: Partial): void; patchData(callback: (node: INode) => Partial): void; + setPosition(position: INodeCoordinates | INodePosition, options?: INodeSetPositionOptions): void; setPosition( - position: INodeCoordinates | INodeMapCoordinates | INodePosition, - options?: INodeSetPositionOptions, - ): void; - setPosition( - callback: (node: INode) => INodeCoordinates | INodeMapCoordinates | INodePosition, + callback: (node: INode) => INodeCoordinates | INodePosition, options?: INodeSetPositionOptions, ): void; setStyle(style: INodeStyle): void; @@ -198,15 +190,6 @@ export class Node extends Subject impl clearPosition() { this._position.x = undefined; this._position.y = undefined; - const data = this.getData(); - - if ('lng' in this._data) { - this.setData({ ...data, lng: undefined }); - } - - if ('lat' in this._data) { - this.setData({ ...data, lat: undefined }); - } this.notifyListeners(); } @@ -464,25 +447,18 @@ export class Node extends Subject impl this.notifyListeners(); } + setPosition(position: INodeCoordinates | INodePosition, options?: INodeSetPositionOptions): void; setPosition( - position: INodeCoordinates | INodeMapCoordinates | INodePosition, + callback: (node: INode) => INodeCoordinates | INodePosition, options?: INodeSetPositionOptions, ): void; setPosition( - callback: (node: INode) => INodeCoordinates | INodeMapCoordinates | INodePosition, - options?: INodeSetPositionOptions, - ): void; - setPosition( - arg: - | INodeCoordinates - | INodeMapCoordinates - | INodePosition - | ((node: INode) => INodeCoordinates | INodeMapCoordinates | INodePosition), + arg: INodeCoordinates | INodePosition | ((node: INode) => INodeCoordinates | INodePosition), options?: INodeSetPositionOptions, ) { - let position: INodeCoordinates | INodeMapCoordinates | INodePosition; + let position: INodeCoordinates | INodePosition; if (isFunction(arg)) { - position = (arg as (node: INode) => INodeCoordinates | INodeMapCoordinates)(this); + position = (arg as (node: INode) => INodeCoordinates)(this); } else { position = arg; } @@ -495,14 +471,6 @@ export class Node extends Subject impl } } - if ('lat' in position && 'lng' in position) { - this._data = { - ...this._data, - lat: position.lat, - lng: position.lng, - }; - } - if (!options?.isNotifySkipped) { this.notifyListeners({ id: this.id, ...position }); } diff --git a/src/utils/observer.utils.ts b/src/utils/observer.utils.ts index 9083b5a..b190fdd 100644 --- a/src/utils/observer.utils.ts +++ b/src/utils/observer.utils.ts @@ -1,6 +1,6 @@ -import { INodeCoordinates, INodeMapCoordinates, INodePosition } from '../models/node'; +import { INodeCoordinates, INodePosition } from '../models/node'; -export type IObserverDataPayload = INodePosition | INodeCoordinates | INodeMapCoordinates; +export type IObserverDataPayload = INodePosition | INodeCoordinates; export interface IObserver { update(data?: IObserverDataPayload): void; diff --git a/src/views/orb-map-view.ts b/src/views/orb-map-view.ts index 1bfd416..a2888cf 100644 --- a/src/views/orb-map-view.ts +++ b/src/views/orb-map-view.ts @@ -456,7 +456,7 @@ export class OrbMapView } const layerPoint = this._leaflet.latLngToLayerPoint([coordinates.lat, coordinates.lng]); - nodes[i].setPosition(layerPoint, true); + nodes[i].setPosition(layerPoint, { isNotifySkipped: true }); } } From e5743588b50dc0ef5bc76dcb9bafbf959b33918b Mon Sep 17 00:00:00 2001 From: AlexIchenskiy Date: Mon, 18 Mar 2024 08:40:19 +0100 Subject: [PATCH 4/5] Chore: Refactor simulator data patching --- src/simulator/engine/d3-simulator-engine.ts | 29 ++++++++++++--------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/simulator/engine/d3-simulator-engine.ts b/src/simulator/engine/d3-simulator-engine.ts index ce11950..384b4d1 100644 --- a/src/simulator/engine/d3-simulator-engine.ts +++ b/src/simulator/engine/d3-simulator-engine.ts @@ -315,15 +315,21 @@ export class D3SimulatorEngine extends Emitter { patchData(data: Partial) { if (data.nodes) { data.nodes = this._fixDefinedNodes(data.nodes); - const nodeIds = this._nodes.map((node) => node.id); + const nodeIds: { [id: number]: number } = {}; + + for (let i = 0; i < this._nodes.length; i++) { + nodeIds[this._nodes[i].id] = i; + } + for (let i = 0; i < data.nodes.length; i += 1) { - if (nodeIds.includes(data.nodes[i].id)) { - this._nodeIndexByNodeId = { - ...this._nodeIndexByNodeId, - ...data.nodes[i], - }; - const index = this._nodes.findIndex((node) => data.nodes && node.id === data.nodes[i].id); + const nodeId: any = data.nodes[i].id; + + if (nodeId in nodeIds) { + const index = nodeIds[nodeId]; + this._nodeIndexByNodeId[nodeId] = index; this._nodes[index] = data.nodes[i]; + } else { + this._nodes.push(data.nodes[i]); } } } @@ -337,11 +343,10 @@ export class D3SimulatorEngine extends Emitter { if (data.nodes) { data.nodes = this._fixDefinedNodes(data.nodes); for (let i = 0; i < data.nodes.length; i += 1) { - if (this._nodeIndexByNodeId[data.nodes[i].id]) { - this._nodeIndexByNodeId = { - ...this._nodeIndexByNodeId, - ...data.nodes[i], - }; + const nodeId = data.nodes[i].id; + + if (this._nodeIndexByNodeId[nodeId]) { + this._nodeIndexByNodeId[nodeId] = i; } else { this._nodes.push(data.nodes[i]); } From 0ee733d5e9c194583c92f9fc319451efbf07cbae Mon Sep 17 00:00:00 2001 From: AlexIchenskiy Date: Mon, 18 Mar 2024 12:32:08 +0100 Subject: [PATCH 5/5] Chore: Change observers to callbacks --- src/models/graph.ts | 19 +++++++++++-------- src/utils/observer.utils.ts | 7 +++---- src/views/orb-map-view.ts | 10 ++++------ src/views/orb-view.ts | 10 ++++------ 4 files changed, 22 insertions(+), 24 deletions(-) diff --git a/src/models/graph.ts b/src/models/graph.ts index a8e699f..4eafc30 100644 --- a/src/models/graph.ts +++ b/src/models/graph.ts @@ -17,7 +17,7 @@ export type IEdgeFilter = (edge: IEdge export type INodeFilter = (node: INode) => boolean; -export interface IGraph extends IObserver, ISubject { +export interface IGraph extends ISubject { getNodes(filterBy?: INodeFilter): INode[]; getEdges(filterBy?: IEdgeFilter): IEdge[]; getNodeCount(): number; @@ -74,6 +74,7 @@ export class Graph extends Subject imp if (settings && settings.listeners) { this.listeners = settings.listeners; } + this.notifyListeners = this.notifyListeners.bind(this); this.setup({ nodes, edges }); } @@ -377,16 +378,18 @@ export class Graph extends Subject imp return nearestEdge; } - update(data?: IObserverDataPayload): void { + // Arrow function is used because they inherit the context from the enclosing scope + // which is important for the callback to notify listeners as expected + private _update: IObserver = (data?: IObserverDataPayload): void => { this.notifyListeners(data); - } + }; private _insertNodes(nodes: N[]) { const newNodes: INode[] = new Array>(nodes.length); for (let i = 0; i < nodes.length; i++) { newNodes[i] = NodeFactory.create( { data: nodes[i] }, - { onLoadedImage: () => this._settings?.onLoadedImages?.(), listeners: [this] }, + { onLoadedImage: () => this._settings?.onLoadedImages?.(), listeners: [this._update] }, ); } this._nodes.setMany(newNodes); @@ -407,7 +410,7 @@ export class Graph extends Subject imp endNode, }, { - listeners: [this], + listeners: [this._update], }, ), ); @@ -429,7 +432,7 @@ export class Graph extends Subject imp newNodes.push( NodeFactory.create( { data: nodes[i] }, - { onLoadedImage: () => this._settings?.onLoadedImages?.(), listeners: [this] }, + { onLoadedImage: () => this._settings?.onLoadedImages?.(), listeners: [this._update] }, ), ); } @@ -457,7 +460,7 @@ export class Graph extends Subject imp endNode, }, { - listeners: [this], + listeners: [this._update], }, ); newEdges.push(edge); @@ -490,7 +493,7 @@ export class Graph extends Subject imp endNode, }, { - listeners: [this], + listeners: [this._update], }, ); edge.setState(existingEdge.getState()); diff --git a/src/utils/observer.utils.ts b/src/utils/observer.utils.ts index b190fdd..e1d32ac 100644 --- a/src/utils/observer.utils.ts +++ b/src/utils/observer.utils.ts @@ -2,9 +2,8 @@ import { INodeCoordinates, INodePosition } from '../models/node'; export type IObserverDataPayload = INodePosition | INodeCoordinates; -export interface IObserver { - update(data?: IObserverDataPayload): void; -} +// Using callbacks here to ensure that the Observer update is abstracted from the user +export type IObserver = (data?: IObserverDataPayload) => void; export interface ISubject { listeners: IObserver[]; @@ -42,7 +41,7 @@ export class Subject implements ISubject { notifyListeners(data?: IObserverDataPayload): void { for (let i = 0; i < this.listeners.length; i++) { - this.listeners[i].update(data); + this.listeners[i](data); } } } diff --git a/src/views/orb-map-view.ts b/src/views/orb-map-view.ts index a2888cf..5ad3555 100644 --- a/src/views/orb-map-view.ts +++ b/src/views/orb-map-view.ts @@ -66,9 +66,7 @@ export type IOrbMapViewSettingsUpdate IOrbMapViewSettingsInit >; -export class OrbMapView - // eslint-disable-next-line prettier/prettier - implements IObserver, IOrbView> { +export class OrbMapView implements IOrbView> { private _container: HTMLElement; private _resizeObs: ResizeObserver; private _graph: IGraph; @@ -92,7 +90,7 @@ export class OrbMapView this.render(); } }, - listeners: [this], + listeners: [this._update], }); this._graph.setDefaultStyle(getDefaultGraphStyle()); this._events = new OrbEmitter(); @@ -230,9 +228,9 @@ export class OrbMapView this._canvas.outerHTML = ''; } - update(): void { + private _update: IObserver = (): void => { this.render(); - } + }; private _initCanvas() { const canvas = document.createElement('canvas'); diff --git a/src/views/orb-view.ts b/src/views/orb-view.ts index 4773064..832a812 100644 --- a/src/views/orb-view.ts +++ b/src/views/orb-view.ts @@ -47,9 +47,7 @@ export type IOrbViewSettingsInit = Omi 'render' > & { render?: Partial }; -export class OrbView - // eslint-disable-next-line prettier/prettier - implements IObserver, IOrbView> { +export class OrbView implements IOrbView> { private _container: HTMLElement; private _resizeObs: ResizeObserver; private _graph: IGraph; @@ -103,7 +101,7 @@ export class OrbView this.render(); } }, - listeners: [this], + listeners: [this._update], }); this._graph.setDefaultStyle(getDefaultGraphStyle()); this._events = new OrbEmitter(); @@ -598,7 +596,7 @@ export class OrbView } }; - update(data?: IObserverDataPayload): void { + private _update: IObserver = (data?: IObserverDataPayload): void => { if (data && 'x' in data && 'y' in data && 'id' in data) { this._simulator.patchData({ nodes: [ @@ -616,7 +614,7 @@ export class OrbView }); } this.render(); - } + }; private _initCanvas(): HTMLCanvasElement { const canvas = document.createElement('canvas');