Fix: multi-selection with the same-parent rule & drop multiple pods into a scope at once

[li-xin-yi]

1. Fix the multi-selection hotkey issue for Mac users: Mac users can press meta key to multi-select nodes as what ctrl does for other users
2. Highlight the border in a darker blue than focused when selected. Un-selection logic is also fixed
3. Apply the "same-parent rule": when a user selects more than one node in either ctrl or shift way, it checks if all selected nodes are of the same parent, otherwise, it will be difficult to drag them into the same scope or delete/cut them together. If the rule is violated when a new node is selected, we un-select all previously selected nodes and select only the new one.
4. Multiple pods can be dragged into a scope at once when selected: when dragging more than one node and the target is a scope, all selected nodes are dropped into the scope.
5. Auto-bind node to parent scope: when dragging stop with the mouse in a target scope, even if the selected nodes are not completely inside the scope (or completely not inside the scope), they will be moved into it completely at once.

Known issues:

1. Auto-binding only works properly when the target scope is larger than all child nodes, or it may look strange.
2. If you multi-select pods in ctrl way, please only press ctrl when selecting, release it right after the node is selected, or the last one will be dragged with all previously selected nodes regardless of the same-parent rule. This is an intrinsic bug in reactflow, the drag behavior is not totally controlled by selection. Don't abuse ctrl.
3. pod[id].parent can't be updated correctly or in time in collaboration mode: a user may drag a node into a scope and update everything as expected on their own side, but the collaborator users can only view the change on front-end side, store.pods on their side doesn't update. It may cause some issues when the collaborators delete involved scopes. It's not very hard to fix, use data.parent field to record their parent, and update with useEffect when it changes. Actually, I propose this approach to update store.pods for almost every important fields of a pod to guarantee all users maintain the data up-to-date. However, I don't have enough time to refactor such a big project. Left it to do when I am available.

[lihebi]

Great PR, thank you, Xinyi!

[lihebi]

If we don't call setPodPosition, how would the store be updated? An immediate issue I observe: if you move a pod, the dirty status is not set, and thus the cloud sync is not triggered. However, when I refreshed, the new position indeed persisted. I guess this persistence is from the yjs side, is it? If so, does it mean that there will be some inconsistency between yjs server and database?

[lihebi]

It is indeed inconsistent between yjs server and the database. If you restart the yjs socket container, you can see all the positions are wrong. The positions are not saved to the database.

[li-xin-yi]

Yes, it only updates nodesMap, notice that nodesMap is shared by all online users via the yjs server, they pull the update from nodesMap and set nodes in canvas according to it immediately. When nodesMap is connected during initialization, it first syncs with nodesMap, it only pulls data from DB when no data in nodesMap. In the current stage, the position changes are not written to the DB.

I agree with your concern. We should recover those lines. That is actually what I mentioned in known issue 3, in the past, we don't do any position update in store.pods[id] when the dragging is from other users. Fortunately, those lines toggles DB update on one user-side. But other users should also be subscribed to the change and update their store.pods, or the data inconsistency issue become more terrible.

[lihebi]

I agree with your concern. We should recover those lines

I've recovered the setPodPosition call in #137.

a user may drag a node into a scope and update everything as expected on their own side, but the collaborator users can only view the change on front-end side, store.pods on their side doesn't update.

If I understand correctly, when a user drags a node into a scope:

• yjs.nodesMap will be updated (thus the graph itself is updated)
• store.pods[id].parent (and store.pods[id].children) won't be updated

If so, we need to fix it, otherwise, the scoped runtime won't work correctly. I think your proposed useEffect method is the way to go. If you have the bandwidth, please go ahead. Otherwise, I can do it as well.

in the past, we don't do any position update in store.pods[id] when the dragging is from other users

I feel this is OK. We don't need to update store.pods[id].position, because it is not used to render the nodes. On refresh, it will be loaded from DB correctly.

[li-xin-yi]

If I understand correctly, when a user drags a node into a scope:

• yjs.nodesMap will be updated (thus the graph itself is updated)
• store.pods[id].parent (and store.pods[id].children) won't be updated
  If so, we need to fix it, otherwise, the scoped runtime won't work correctly. I think your proposed useEffect method is > the way to go. If you have the bandwidth, please go ahead. Otherwise, I can do it as well.

Yes, you got it exactly. Thanks a lot.

I feel this is OK. We don't need to update store.pods[id].position, because it is not used to render the nodes. On refresh, it will be loaded from DB correctly.

Imagine a situation:

1. a pod is dragged from a place to another place by a user other than myself, store.pod[id].position is not updated on my side, which is (oldX, oldY), but it should be (newX, newY)
2. later, I change something inside this pod, e.g., name, code, calling setPodName or setPodContent, which updates only name or content, keeps position as (oldX, oldY), and mark the pod as dirty.
3. remoteUpdateAllPods is called periodically to write this dirty pod to the database, which is {name: newName, content: newContent, position: (oldX, oldY)}

That's what I mean by saying data inconsistency issue.

Besides, I'm now considering copy-paste multi-selected pods, I may use serialized store.pod[id] info in clipboard, their relative positions may also be kept after pasting.

[lihebi]

Ah, I see. Great example! We indeed need a careful new design for syncing all important fields of store.pods.

[lihebi]

So store.pods[id].content should also have this same inconsistency issue (as in your example step 1,2,3), right?

[li-xin-yi]

So store.pods[id].content should also have this same inconsistency issue (as in your example step 1,2,3), right?

I think the consistency of code itself is okay. Because we use setPodContent in editor's onChange, it should be called every time the value updates, even if the update comes from the yjs server. Of course, it may cause redundant writing to the DB, but the performance overhead in it is not so critical in the current stage.

I'm mainly worrying about the delayed update in position and parent info.

BTW, I come up with a possible solution to mitigate the redundant DB update overhead caused by nodesMap changes: add a field in node's data, as lastModifier to track who pushed the update of this pod to the yjs server just now, in useEffect that handles node data update, we only call doRemoteUpdate (or mark dirty) when the lastModifier is not myself, otherwise, only update fields store.pod[id]. Certainly, we only consider applying the performance optimization when the cost to access DB is much larger than other operations. For the current stage, I think the speed is acceptable.

[lihebi]

Because we use setPodContent in editor's onChange, it should be called every time the value updates, even if the update comes from the yjs server.

I see. Yeah, essentially we implement the useEffect update on pods.content. We should do the same for position and parent as well.

Of course, it may cause redundant writing to the DB, but the performance overhead in it is not so critical in the current stage.

Agree, no need to worry about this.

_BTW, I come up with a possible solution to mitigate the redundant DB update overhead caused by nodesMap changes: add a field in node's data, as lastModifier to track who pushed the update

Great idea; let's implement it later in this way.