Page MenuHomePhabricator

dropped node inserted at wrong position in the data manager
Closed, ResolvedPublic

Description

If you drag and drop a node or nodes in the data manager (e.g. to reorder the layers of nodes) then the nodes are inserted in a wrong position.

The cause of the problem is that the position is not adjusted after the images are removed from the parent. E.g. if you drag one image which is before the point where you want to insert it, then it will be inserted one item below as it should be, since the indexes change during its removal. However, if the image is dragged upwards, it will be inserted in the expected position.

I have a fix for this and can send a pull request through github.

Event Timeline

The fix:

https://github.com/NifTK/MITK/commit/cfeed4eb98462e00fe0e8fdae3ec7988ae334beb

I can rebase the branch on the MITK master and send a pull request.

@Michael Is that something you already know of? Does it make sense to integrate it during the next bug-squashing day?

We were discussing whether or not to fix this for the next release and a couple of questions cam up:

  1. Is this fix already included in your code base? What is your experience?
  2. The current behaviour depends on the direction the node is coming from, but usually when moving a node up one wants to position it before whatever node one is releasing it on and vice versa. One problem in especial is moving a node to the first position. The current behaviour is to release it on the top node. The new behaviour might require you to move it to the thin area between the top node and the top of the list. How is that working for you?
  3. We considered leaving the current behaviour, but adding a line marking the position where the node will be added. Would this work for you?

I used the attached patch to test the behaviour of your proposed changes.

If I drag and drop a node on another node on the same hierarchy level however, the order is not changed at all.

Is this your desired behaviour?

In my opinion the dragged node should be placed before the node it is dragged on (as hitting the thin area between the top node and the top of the list is the most difficult).

Actually, we use another commit as well. If you drop a node upon another, it will make it its derived node.

https://github.com/NifTK/MITK/commit/c0855b84484dfdc9f4f2d3c009cdacaf5d524e99

This was the actual feature that we needed. You are right that it is a bit hard to position a node in between two other nodes. I do not know what would be a good compromise.

We do not want to have the "dropping node on others makes them child nodes" as our default behaviour, as a node may have multiple parents in the data storage and it is unclear how to cleanly handle this. Options include:

  1. Only make it a child node if it has no parents
  2. Replace all parents with the dropped-upon node
  3. Add the dropped-upon node as a parents beside the already existing ones

Problems:

  1. Is very unintuitive and will confuse users
  2. Probably the best option, but prevents more complex relations between nodes
  3. Might not change the display in the data manager as the node is only shown once

However, we could create a toggle in the data manager preference pages to allow switching between our default behaviour (node is placed on the same level after the node in the direction of movement) and yours (becomes a child node).

Would this work for you?

Yes, a preference for the data manager would be good.

That's true that a node can have several sources in the data storage, but I do not know if the data manager (tree model) actually supports this, or if yes, how this situation is rendered in the tree view. Is the same node rendered in as many instances as many sources it has, once beneath each of them? If yes, this can be confusing for the user as well, so I do not think that the drag-and-drop would make this worse. It is not a critique, I do not know a better way of displaying the nodes in the data manager. Just telling that using a tree model is kind of assuming that the relationship is hierarchical, or reflecting only part of the graph.

What regards our use case, our nodes are definitely in a hierarchy (no more than one parent per node), and the drag and drop is used only for the images that are opened by File/Open and put on the top level.

I think, (2) is the best option. If you want to keep the current parents (3), maybe dragging with keeping the Ctrl key pressed could be used (Cmd on Mac, I think).

(1) is not good, as you could not move a node from one parent to another. Although we do not need that feature, I am pretty sure there would be users who would try it and got confused. :)

User thomass has pushed new remote branch:

bug-15488-dropped-node-wrong-position

User thomass has pushed new remote branch:

bug-15488-intuitive-dropped-node-handling

@espak Do the changes merged in rMITKc3d09f20 fit your requirements? You can switch between hierarchy changes being allowed or not in the datamanager preference page (bottom checkbox).