Skip to content
This repository has been archived by the owner on Dec 28, 2021. It is now read-only.

Deselect visualization when its node gets removed #1556

Merged
merged 8 commits into from
May 13, 2021
Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#### Visual Environment

- [Delete key will delete selected nodes][1538].
- [It is possible to move around after deleting a selected visualization][1556].
s9ferech marked this conversation as resolved.
Show resolved Hide resolved

#### EnsoGL (rendering engine)

Expand Down
1 change: 1 addition & 0 deletions src/rust/ide/view/graph-editor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1403,6 +1403,7 @@ impl GraphEditorModel {
/// implementation.
fn remove_node(&self, node_id:impl Into<NodeId>) {
let node_id = node_id.into();
self.frp.source.on_visualization_select.emit(Switch::Off(node_id));
Copy link
Member

Choose a reason for hiding this comment

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

Emiting frp events from model functions is rather a bad pattern, as it is really hard to understand where the events come from. Instead, check where this model function is called in the FRP network and handle it directly there.

This should be handled next to the lines:

output.source.on_visualization_select <+ selected.constant(Switch::On(node_id));
output.source.on_visualization_select <+ deselected.constant(Switch::Off(node_id));

so it would be easy to track when visualization is deselectected when reading the code (all deselecting will be coded in one place then)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed this, but it cannot be handled next to the lines that you suggested, because we need only one connection per graph editor, but those lines are run for every node.

self.nodes.remove(&node_id);
self.nodes.selected.remove_item(&node_id);
}
Expand Down