Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the nodes draggable in the view #7916

Open
Tracked by #7897
abey79 opened this issue Oct 28, 2024 · 2 comments
Open
Tracked by #7897

Make the nodes draggable in the view #7916

abey79 opened this issue Oct 28, 2024 · 2 comments
Labels
feat-graph-view Everything related to the graph view

Comments

@abey79
Copy link
Member

abey79 commented Oct 28, 2024

Manually dragging the nodes in the graph view is useful both for fine-tuning the presentation and to interact with the (future) auto-layout algorithm for analysis purposes.

A correct implementation should transcribe the drag-and-drop interaction in the view into Position2D component overrides. However, for this to happen, we need per-instance override semantics.

Possible solutions:

  • Any of the per-instance override strategy listed in: Per-instance component override #7920
  • The per-instance override could be implemented at view level by having an ManuallySet: [bool] component, to track which instances of the Position2D instances should be considered. Insanely hacky, but doable without changing our data model.
  • A slightly cleaner, more extensible variant would be to use tagged component to maintain multiple overrides for Position2D. One would be tagged with manually_set. In the future, another one could be dynamic_auto_layout, to store the intermediate states of the force-based auto-layout, should we want to expose them to the user across several frames.
  • (edit) Yet another approach would be to have a ManualNodePositions: { node_id: Utf8, position: Vec2D } view property (not override) might be a better fit to the GraphView, because the overridden position would be related to a give node id, which is much more stable than a node instance index (a given node id might be logged under different instance index at different point in time). This would allow us to entirely side-step the "per-instance-override" question entirely.
  • other?

Another aspect of this is how poor the current override UI is with non-mono-component (although in this case this is not a limiting factor strictly speaking, as the instance edit would happen via in-view UI interaction).

@abey79 abey79 added the feat-graph-view Everything related to the graph view label Oct 28, 2024
@teh-cmc
Copy link
Member

teh-cmc commented Oct 28, 2024

A correct implementation should transcribe the drag-and-drop interaction in the view into Position2D component overrides. However, for this to happen, we need per-instance override semantics.

(I'm not familiar with the data model behind the new graph stuff)

Why do we need per-instance overrides at all? I.e. why is overriding all the positions in the position batch everytime any node is moved around not an option?

@abey79
Copy link
Member Author

abey79 commented Oct 28, 2024

(I'm not familiar with the data model behind the new graph stuff)

Consider the GraphNode archetype has an optional Position2D component. Fallback to some kind of auto-layout (which could initially be as stupid as "arrange all nodes on a circle"). We can ignore edges here.

Why do we need per-instance overrides at all?

I think this is required for the cases where the user logged time-varying Position2D. When one node is moved around in the UI, it's now-overridden position should apply regardless of the position of the time cursor, and should only affect that particular node. For that to happen, the un-overridden instance should be left "empty" in the override, because there is no "previous value" to duplicate (the previous value being time-varying).

(With that said, I now realise that there is an alternative approach that's hacky w.r.t to rerun data model, but closer to the graph view semantics we already decided. I'm adding it to the list.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat-graph-view Everything related to the graph view
Projects
None yet
Development

No branches or pull requests

2 participants