Handle child cells in registerCellsAddedHandler #140
Conversation
cauebs
changed the title
cauebs
force-pushed
the
fb-ASIM-4848-clone-network-elements
branch
2 times, most recently
from
dcfaf1e
to
97f80ce
Compare
cauebs
force-pushed
the
fb-ASIM-4848-clone-network-elements
branch
2 times, most recently
from
8d44d36
to
b3afed3
Compare
cauebs
force-pushed
the
fb-ASIM-4848-clone-network-elements
branch
from
e8ddd07
to
52cffb8
Compare
cauebs
force-pushed
the
fb-ASIM-4848-clone-network-elements
branch
from
bf7498e
to
52cffb8
Compare
cauebs
force-pushed
the
fb-ASIM-4848-clone-network-elements
branch
from
f61329d
to
2f786c7
Compare
src/qmxgraph/page/api.js
Outdated
| * @param {number[]} cellIds Ids of cells to be cloned. | ||
| * @param {boolean} ignoreMissingCells Ids of non existent cells are ignored instead of | ||
| * raising an error. | ||
| * @param {boolean} allowInvalidEdges Specifies if invalid edges should be cloned. |
There was a problem hiding this comment.
Can you elaborate what are "invalid edges"?
There was a problem hiding this comment.
I confess I'm not quite sure. I'm just surfacing the underlying mxGraph API. But since I'm already changing the behaviour by implicitly adding the cells to the graph on cloneCells (which mxGraph doesn't do), then maybe I could just ommit that parameter since it's not useful to us.
There was a problem hiding this comment.
Update: reading the mxGraph's source code just now, it seems that "invalid edges" is all about the edges' terminal nodes. It checks for dangling edges, loops, edges missing both terminal nodes, etc.
| raising an error. | ||
| :param allow_invalid_edges: Specifies if invalid edges should be cloned. | ||
| """ | ||
| with wait_signals_called(self._events_bridge.on_cells_added): |
There was a problem hiding this comment.
I just noticed that if cell_ids is empty this will hang.
Given this scenario also affects other code around I think we can ignore this for now.
| assert added_handler.call_args_list == [ | ||
| mocker.call(cell_id) for cell_id in (cell_ids + new_cell_ids) | ||
| ] |
There was a problem hiding this comment.
Just to be sure.
Does this causes the add event to be called one time for each cell?
There was a problem hiding this comment.
It shouldn't, you're right. In the current implementation cellsAdded is only called once for each call to cloneCells.
The thing is I haven't run the tests yet, because I'm having trouble building this project's conda environment, and the CI is still broken.
| """ | ||
| graph = graph_cases('2v_1e') | ||
|
|
||
| vertices = graph.get_vertices() |
There was a problem hiding this comment.
I also often do this:
| vertices = graph.get_vertices() | |
| vert1, vert2 = graph.get_vertices() |
cauebs
force-pushed
the
fb-ASIM-4848-clone-network-elements
branch
from
60dfff4
to
f65e168
Compare
Replace spread operator with more supported apply method
cauebs
force-pushed
the
fb-ASIM-4848-clone-network-elements
branch
from
d767d30
to
bf0d5b1
Compare
for more information, see https://pre-commit.ci
| var graph = this._graphEditor.graph; | ||
|
|
||
| // Creates a dictionary for fast lookups | ||
| var dict = new mxDictionary(); | ||
| var tmp = []; |
There was a problem hiding this comment.
tabs for indentation?
Better check your IDE XP
There was a problem hiding this comment.
Copied from mxgraph. I will convert it to spaces 👍
There was a problem hiding this comment.
pre-commit.ci already did that.
Needed for ASIM-4848. Main PR in https://github.com/ESSS/alfasim/pull/38/.
When cells are copied by ctrl+dragging, a callback is executed as the new cells are added, but decorations weren't passed along with nodes and edges (and perhaps tables).
The callback implementation for when cells are removed included some logic for recursing down into child cells, and it seems that was also required for when they're added. This PR factors that logic out from
registerCellsRemovedHandlerso it can be reused inregisterCellsAddedHandler.Props to @prusse-martin for basically co-authoring this.