Node Editors : Enhancing Node Manipulation

Feature requests
, ,
21

@Tricotou, my comments aren’t about losing functionality we had. On the contrary, this is a great addition to the tools and something I asked for years ago. My concern with the current implementation is twofold.

  • We are giving up two modifier keys for the same feature, just with inverted selection. In looking at all of the node-based tools we are creating and where the tooling is headed, I don’t feel great about losing an extra single key modifier (alt) for any future features. Obviously, we have combination modifiers still available like shift + alt, but using two single key modifiers for the same feature seems not to be the best future facing choice.
  • When you mention not wanting to zoom out to see what you are moving with the feature, you are expecting that users will be ok with manipulating nodes that they cannot see. If you are zoomed into a section of the graph and can see 6 nodes and using the feature affects 20 other nodes that are off screen, the user can’t make an informed decision about where to place those nodes. This hits the other main pillar of our design efforts I often repeat to myself - “Don’t assume user intent.” If we are manipulating the user’s graph without them seeing it, we are opening the door for frustration.

So if I were to suggest a change for this feature, this is the behavior I would suggest:

  1. ctrl + click works just as it does today, group selection without any concept of what is already selected.
  2. shift + click will select every node in the hierarchy traveling toward the last selected node.
    a. If shift + click is used and there was no other selected node, just that node is selected.
    b. If shift + click does not find a path to the last selected node, just add that node only to previous selection.
    c. Shift + click and ctrl + click both add to the selection group so can be used interchangeably. Perhaps shift + clicking a chain and ctrl + clicking two additional node or vice versa.
  3. To address your concern about needing to zoom out to select all the way to the begging or end of the node chain, we could add a simple behavior. Click to select the first node, then shift + double click to select in the intended direction to the end of the node chain. In this way, you don’t need to zoom out to select all, it’s the same number of clicks, and it is initiated by the user to say, “I want to select the entire chain” and we are not assuming intent.

The shift + double click may not seem intuitive, and I agree that it can be awkward as that is how Maya has you quick select an entire loop of edges. However, what we gain from this is a reliance on a well know pattern which is largely unknown knowns for a lot of people (I had to ask a Mac user to verify their selection pattern was the same as Windows because I had memory of it being the same, but couldn’t test and that user had to test to confirm). Being able to rely on patterns that people have forgotten that they already know is just tapping into muscle memory, which makes me think that this needed feature will be that much more intuitive if we follow that pattern.

Basically, I am trying to find that middle ground which will satisfy the most scenarios without assuming user intent and adding the least amount of new patterns to learn. I think if you can modify your feature with this in mind, you will solve a lot of problems in graph organization that we have faced for a long time.

1 Like
22

Hello @PatrickRyan

OK ! I think I come back with good news !

I edited the behavior to do like so :

  • No more use of ALT key
  • Select with SHIFT adds to the selection, just like CTRL
  • Click in the middle of canvas will unselect all only if you don’t move, allowing to move the graph with selected nodes (could be discussed)

Then, here are the nice stuff :
GraphNode._searchMiddle : Select A → SHIFT Select B will search for a link inbetween (otherwise just select A&B)

GraphNode._expand : If 2 nodes A & B are neightboors, select A, SHIFT and DOUBLE CLICK B → will select all in direction of B, following links (toward children, or toward parents, depending on relation between A & B)

I hope you will be happy with this ! Please have a try and tell me :slight_smile:

23

Here is a playground to test the snapshot :

(Select the node material in treeview and hit edit button :pencil: )

24

tested and approved!

1 Like
25

I’m leaving the PR draft for the moment because I would like to investigate about some inconsistency on select / deselect… But testing an older version, it seems that there is something already weird, so not from this feature. For example the unselect with CTRL while selecting nodes is… Somehow random.

1 Like
26

@Tricotou, I am with you in that I stumbled on the inconsistent deselect with ctrl. From what I am seeing, this is the repro.

  • hold down control and don’t release it for the duration of the repro
  • click a node
  • click outside the node and drag a selection on the same node (it will stay selected, but you will see a quick flash of the white highlight on the node)
  • click again on the selected node and it will deselect

I don’t doubt that this is an old bug that we only found because we were testing your PR. This is what the behavior should be: Shift + click works as you have it now. Ctrl + click will select an unselected node regardless of what is selected. Ctrl + click on a selected node will deselect the node regardless of what is selected. In that way, shift is only adding to the selection, and ctrl is a toggle for selection. The only weirdness now is that ctrl + click + drag will create a selection box and shift + click + drag will create a frame. That is old behavior, so we won’t change it, but it does break the selection dynamic when mind mapping the keys.

Overall, this is a great addition to the tool. Thank you for doing it!

27

Hello @PatrickRyan

So, yeah I confirm the problem is not from this feature, the previous NME has the same. In fact I found a much simpler repro :

  • Nodes will unselect with CTRL only once.

It’s that simple, works every time ^^ Just select and unselect some nodes with CTRL… On each node the unselect will work only one time. Like if it has memory.

I’m having a look at the code right now, and there is (to my mind) something very weird in the structure :

  • In the graphNode.ts (Node Level) in the pointer events functions such as _onDown or _onMove, is made the usage of the condition evt.ctrlKey. And logic will eventually change the select state of some nodes :
} else if (evt.ctrlKey) {
    this.setIsSelected(false, false);
}
  • But in graphCanvas.tsx at a upper level, there is an observable which listens to these selection changes, and takes it into account, based on the truth state ofthis._multiKeyIsPressed, which is evt.ctrlKey || evt.metaKey.
props.stateManager.onSelectionChangedObservable.add((options) => {
    // [...]
    if (selection instanceof GraphNode) {
        if (this._multiKeyIsPressed || this._shiftKeyIsPressed || forceKeepSelection) {
            if (!this._selectedFrameAndNodesConflict(this._selectedFrames, [selection]) && !this._selectedNodes.includes(selection)) {
                this._selectedNodes.push(selection);
            }
        } else {
            this._selectedFrames = [];
            this._selectedNodes = [selection];
            this._selectedLink = null;
            this._selectedPort = null;
        }
    }
    // [...]
}
  • Maybe it’s ok, by the way, but at first sight, seems confusing…
28

Oupsie! I’ll fix that right now

29

oh well I saw you did it in your PR
Thanks mate!

30

Ahah yes, pushed 30 seconds ago xD

@PatrickRyan I finally found the bug :

  • NodeGraph objects had a isSelected property, unset by default (undefined)
  • onSelect : returns (ignore) if isSelected===true (never happened)
  • onDeselect : returns (ignore) if isSelected===false (never happened !)

And then ONLY the unselect with ctrlKey was setting isSelected=false …

2 Likes
31

NME snapshot is not ready yet, but you can already test from the playground (edit node :pencil:)

It works smooooooooth I’m so happy :slight_smile:

Setting the PR back to ready :white_check_mark:

2 Likes
32

This is an amazing quality of life addition, @Tricotou! Thank you so much for contributing!

1 Like