Skip to content

New: Add removal functions - #96

Merged
AlexIchenskiy merged 4 commits into
new/simulator-fixesfrom
new/add-removal-functions
Mar 18, 2024
Merged

New: Add removal functions#96
AlexIchenskiy merged 4 commits into
new/simulator-fixesfrom
new/add-removal-functions

Conversation

@AlexIchenskiy

Copy link
Copy Markdown
Collaborator

This PR introduces functions for removing all data, nodes (the same functionality as removing all the data), or edges by reusing existing remove function. It can be used as follows:

orb.data.removeAll();
orb.data.removeAllNodes();
orb.data.removeAllEdges();

@AlexIchenskiy AlexIchenskiy self-assigned this Mar 13, 2024

@tonilastre tonilastre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks great! There is just one comment on the callback data.

Comment thread src/models/graph.ts Outdated

this._settings?.onRemoveData?.(data);
if (this._settings && this._settings.onRemoveData) {
this._settings.onRemoveData(data);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a user (internal or external) I would expect to receive in the callback all nodes and edges that have been removed. You are forwarding the data that you've received which doesn't necessarily mean that those have been removed, e.g.

In my graph I have these nodes and edges

  • Nodes: 1, 2, 3
  • Edges A (1-2), B (2-1), C(1-3)

I call remove with nodes = 1, 3, 5 and edges = C, D. But actually nodes 1, 3 will be removed and all edges will be removed (A, B, C). So instead of receiving [1, 3, 5], [C, D] in the callback, I would expect to receive [1, 3], [A, B, C].

Comment thread src/models/graph.ts Outdated
// Merge edges removed by removing nodes and by removing edges, ensuring there are no duplicate edge IDs.
removedData.edgeIds = removedData.edgeIds.concat(
removedEdgeIds.filter((edgeId) => removedData.edgeIds.indexOf(edgeId) < 0),
);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the most elegant solution I've found so far, but I'm open to suggestions for any more performant/readable alternatives

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be slow, so I would suggest creating utility function, e.g. dedupArrays that receive a generic array, or multiple arrays and dedups their values - using set or map.

E.g.

dedupArrays([1, 2, 3], [1, 2]) -> [1, 2, 3])
dedupArrays([1, 2, 3], [1, 2], [5, 6]) -> [1, 2, 3, 5, 6])
dedupArrays([1, 2, 3], [1, 2], [5, 6], [], [6, 7]) -> [1, 2, 3, 5, 6, 7])

Comment thread src/models/graph.ts Outdated
}

private _removeNodes(nodeIds: any[]) {
private _removeNodes(nodeIds: any[]): { nodeIds: any[]; edgeIds: any[] } {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This return type is cool. I would maybe extract it into an interface/type. Also, just for a sake of consistency, I would maybe consider returning the same type on _removeEdges - right now, nodeIds will be always empty, but in the future if we add "removeConnectingNodesfor edges, then the API still holds and can use the returned object with bothnodeIdsandedgeIds`.

Comment thread src/models/graph.ts Outdated
// Merge edges removed by removing nodes and by removing edges, ensuring there are no duplicate edge IDs.
removedData.edgeIds = removedData.edgeIds.concat(
removedEdgeIds.filter((edgeId) => removedData.edgeIds.indexOf(edgeId) < 0),
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be slow, so I would suggest creating utility function, e.g. dedupArrays that receive a generic array, or multiple arrays and dedups their values - using set or map.

E.g.

dedupArrays([1, 2, 3], [1, 2]) -> [1, 2, 3])
dedupArrays([1, 2, 3], [1, 2], [5, 6]) -> [1, 2, 3, 5, 6])
dedupArrays([1, 2, 3], [1, 2], [5, 6], [], [6, 7]) -> [1, 2, 3, 5, 6, 7])

@tonilastre tonilastre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I've added a small detail for a better code quality.

Comment thread src/models/graph.ts Outdated
Comment on lines +282 to +285
const removedData = {
nodeIds: dedupArrays(removedNodesData.nodeIds, removedEdgesData.nodeIds),
edgeIds: dedupArrays(removedNodesData.edgeIds, removedEdgesData.edgeIds),
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a small detail here. Whenever you create a new object or an array, always add a type. Types will help you to raise warning if you for example change type in the future.

const removedData: IGraphObjectsIds = {
...

Also, this object should be within if on line 290 because you want to create this object only if settings exists and settings.onRemoveData exists, otherwise time and memory is wasted if settings is not called at all.

@AlexIchenskiy
AlexIchenskiy merged commit e9a4632 into new/simulator-fixes Mar 18, 2024
@AlexIchenskiy
AlexIchenskiy deleted the new/add-removal-functions branch March 18, 2024 13:30
AlexIchenskiy added a commit that referenced this pull request Mar 27, 2024
* New: Add new simulator (#56)

* New: Add simulator scenarios for manual testing
* New: Refactor simulator (WIP)
* New: Add progress overlay, Update descriptions
* Fix: Introduce new simulator event, Fix main-thread behavior
* Fix: Rearrange class methods based on visibility
* Fix: Improve naming
* Chore(release): 0.2.0
* Fix: Tweak simulator, adjust API slightly
* Fix: Temporarily patch some physics behavior
* Fix: Adjust re-heating parameters
* Fix: tweak physics behavior -> immediately stop sim when disabling
* Chore: Remove the beta from release branches

---------

Co-authored-by: dlozic <davidlozic@gmail.com>

* New: Add zoom and recenter functions (#74)

* New: Add zoom and recenter functions

* Fix: Reduce excessive recentering (#75)

* Fix: Remove excessive recenterings
* Fix: Remove unused code

* Fix: New simulator (#92)

* Chore: Refactor naming

* New: Add new events

* Chore: Refactor code styling

* Docs: Remove unused flags

* Chore: Remove unused simulation functions

* Chore: Refactor view render function calls

* Chore: Add missing tests

* New: Add removal functions (#96)

* New: Add removal functions

* Fix: Add missing callback data

* Chore: Refactor remove return values

* Chore: Refactor remove function type usage

* Fix: Default settings for node placement (#98)

* New: Add properties setters and getters (#93)

* New: Add node properties setters

* New: Add edge properties setters

* New: Add properties getters

* New: Add patch for nodes and edges

* Fix: Make getters return copies

* Fix: Edge factory listeners copying

* Fix: Jest outdated tests

* Fix: Github actions node version

* Chore: Refactor observer interface

* Chore: Refactor node/edge constructor settings

* Chore: Refactor node/edge function grouping

* Chore: Refactor node/edge function grouping

* Fix: Listeners behaviour

* Chore: Refactor property copying

* Chore: Refactor subject implementation

* Fix: Set position behaviour on node drag

* Chore: Upgrade node version

* Chore: Refactor function type check

* Fix: Remove listener behaviour

* Chore: Refactor util naming

* Chore: Remove unused type assertion

* Chore: Refactor position setter options

* Chore: Refactor property patch function

* Fix: Set map node position behaviour

* Chore: Refactor simulator data patching

* Chore: Change observers to callbacks

* New: Add state setters with options (#95)

* New: Add state setters with options

* Chore: Remove leftover comments

* Chore: Refactor state setter logic

* Chore: Refactor state types

* Fix: Rename merged function usage

* Fix: Merged variable naming

* Chore: Fix tests

---------

Co-authored-by: dlozic <davidlozic@gmail.com>
Co-authored-by: Oleksandr Ichenskyi <55350107+AlexIchenskiy@users.noreply.github.com>
Co-authored-by: AlexIchenskiy <aichenskiy@gmail.com>
tonilastre added a commit that referenced this pull request Jul 28, 2026
… tooling

* Fix: Fix multiple maps issue (#33)

* Chore: Update package.json version

* New: Change the API to handle OrbView and OrbMapView (#34)

* New: Change the API to handle OrbView and OrbMapView

* New: Change the API for select/hover strategies

* Chore: Release/1.0.0

* New: Add support to get selected/hovered nodes and edges (#61)

* New: Added support to get selected nodes and edges

* New: Added support to get hovered nodes and edges

---------

Co-authored-by: Abhinav Singh Parmar <abhinavparmar147@gmail.com>

* New: Add support for enabling and disabling dragging of nodes (fixes #62) (#69)

* New: Add feature to enable/disable node dragging (fixes #62)

* New: Added support to modify interaction from setSettings

* New: Updated documentation for interaction property

* New: Add feature to enable/disable zoom (fixes #62)

* NEW: Updated documentation for interaction property

* NEW: Updated documentation to include isDragEnabled

* New: Add support for custom edge line style (#77)

* New Added support for custom edges

* Refactor: Streamline edge rendering code and optimize line style handling

* New: Add support for handling device pixel ratio (#45)

* Chore: Move container and canvas creation from the view to the renderer

* New: Add devicePixelRatio render property and handler

* Fix: Add default DPR for older browsers

* Chore: Remove useless check for automatic DPR

* New: Add new simulator (#56) (#57)

* New: Add new simulator (#56)

* New: Add simulator scenarios for manual testing
* New: Refactor simulator (WIP)
* New: Add progress overlay, Update descriptions
* Fix: Introduce new simulator event, Fix main-thread behavior
* Fix: Rearrange class methods based on visibility
* Fix: Improve naming
* Chore(release): 0.2.0
* Fix: Tweak simulator, adjust API slightly
* Fix: Temporarily patch some physics behavior
* Fix: Adjust re-heating parameters
* Fix: tweak physics behavior -> immediately stop sim when disabling
* Chore: Remove the beta from release branches

---------

Co-authored-by: dlozic <davidlozic@gmail.com>

* New: Add zoom and recenter functions (#74)

* New: Add zoom and recenter functions

* Fix: Reduce excessive recentering (#75)

* Fix: Remove excessive recenterings
* Fix: Remove unused code

* Fix: New simulator (#92)

* Chore: Refactor naming

* New: Add new events

* Chore: Refactor code styling

* Docs: Remove unused flags

* Chore: Remove unused simulation functions

* Chore: Refactor view render function calls

* Chore: Add missing tests

* New: Add removal functions (#96)

* New: Add removal functions

* Fix: Add missing callback data

* Chore: Refactor remove return values

* Chore: Refactor remove function type usage

* Fix: Default settings for node placement (#98)

* New: Add properties setters and getters (#93)

* New: Add node properties setters

* New: Add edge properties setters

* New: Add properties getters

* New: Add patch for nodes and edges

* Fix: Make getters return copies

* Fix: Edge factory listeners copying

* Fix: Jest outdated tests

* Fix: Github actions node version

* Chore: Refactor observer interface

* Chore: Refactor node/edge constructor settings

* Chore: Refactor node/edge function grouping

* Chore: Refactor node/edge function grouping

* Fix: Listeners behaviour

* Chore: Refactor property copying

* Chore: Refactor subject implementation

* Fix: Set position behaviour on node drag

* Chore: Upgrade node version

* Chore: Refactor function type check

* Fix: Remove listener behaviour

* Chore: Refactor util naming

* Chore: Remove unused type assertion

* Chore: Refactor position setter options

* Chore: Refactor property patch function

* Fix: Set map node position behaviour

* Chore: Refactor simulator data patching

* Chore: Change observers to callbacks

* New: Add state setters with options (#95)

* New: Add state setters with options

* Chore: Remove leftover comments

* Chore: Refactor state setter logic

* Chore: Refactor state types

* Fix: Rename merged function usage

* Fix: Merged variable naming

* Chore: Fix tests

---------

Co-authored-by: dlozic <davidlozic@gmail.com>
Co-authored-by: Oleksandr Ichenskyi <55350107+AlexIchenskiy@users.noreply.github.com>
Co-authored-by: AlexIchenskiy <aichenskiy@gmail.com>

* New: Add zoom in and out functions (#100)

* Fix: Skip unnecessary listener notify on set style

* Fix: remove unnecessary rerender on state change

* Chore: Update documentation

* Fix: Node/edge getter performance issue

* Chore: Add data change docs example

* Fix: Docs typos

* Fix: Disable source map generation (#105)

* New: Add tree layout (#107)

* New: Add new layouts

* New: Add layout options

* Chore: Make layout dynamically changeable

* Chore: Update docs

* Fix: Naming typo

* Chore: Refactor layouts

* New: Enable layout node add/remove

* Chore: Improve behavior for recurrent nodes

* Chore: Move some simulator settings to layout

* Chore: Refactor code quality and performance

* Fix: Layout behavior on change

* Chore: Add recenter on layout change

* Fix: Layout change behavior

* Fix: Simulation behavior on data deletion

* Chore: Add recenter on layout change

* Fix: Change layout engine logic (#108)

* Fix: Change layout engine logic

* New: Add simulation cancellation logic

* Chore: Remove leftover code

* Fix: Hierarchical layout recenter logic

* Chore: Refactor package versions and code logic

* Fix: Refactor function types

* New: Add multiselect (#110)

* New: Add multiselect

* Chore: Simplify logic

* Chore: Update package.json

* New: Add SVG export (#111)

* New: Add SVG export

* Chore: Refactor code quality

* New: Add WebGL renderer (#109)

* New: Add WebGL renderer

* New: Add naive WebGL force layout computation

* New: Add WebGL improved node/shape geometry options

* New: Add labels and node images

* Fix: GPU drag behavior

* Chore: Remove leftover comments

* Fix: WebGL renderer style invalidating

* Chore: Refactor code quality

* Chore: Add WebGL example and docs

* New: Add docs page (#112)

* New: Add docs page

* New: Add more docs examples

* Update .github/workflows/docs.yml

---------

Co-authored-by: Toni <toni.lastre@memgraph.io>

* Chore: Finish up the release process

* Chore: Fix sync between package.json files

* Chore: Add new package-lock.json

---------

Co-authored-by: David <davidlozic@gmail.com>
Co-authored-by: Abhinv Singh Parmar <abhi171b010@gmail.com>
Co-authored-by: Abhinav Singh Parmar <abhinavparmar147@gmail.com>
Co-authored-by: Abhinv Singh Parmar <abhinav.parmar@infosys.com>
Co-authored-by: Oleksandr Ichenskyi <55350107+AlexIchenskiy@users.noreply.github.com>
Co-authored-by: AlexIchenskiy <aichenskiy@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants