search

eclipse-emfcloud / theia-tree-editor

theia-tree-editor
Other
15 stars 12 forks source link

Customize the property used for node selection #58

Closed alxflam closed 2 years ago

alxflam commented 2 years ago

Hi,

the following issue appears if a tree contains two nodes with the same name provided by the corresponding label provider: Of the two nodes, first select the one that is below the other one in the tree. Then change some property of this node in the details view. Notice that the other node is now being selected.

Root cause is that the node retrieval for the last selection is based on the name. Say if there are two nodes named 'Test', doing a modification of the first node leads to the selection of the other 'Test' node (as that was the name of the previously selected node).

Instead, maybe we could pass the name of something like a 'identity' property to the MasterTreeWidget (see select method for the node determination). Then adopters could e.g. ensure a stable ID is created (in their TreeEditor.NodeFactory, e.g. based on the '@id' property) such that the ID is used instead of the name. This would ensure selections are retained even if nodes with the same name are existing.

The behavior is reproducible with the coffee example. (E.g. rename the Push node to Drink and then adapt the duration in the details view - the selection will switch to the node above with the same name).

What do you think?

Greetings, Alex

lucas-koehler commented 2 years ago

Hello @alxflam , thank you very much for the detailed issue. I can see why the issue materializes in the coffee editor but not in the example editor in this repository: The coffee editor updates itself - and thereby the selection - on every change because it gets the updated data from the model server. In the example, this does not happen and is therefore not reproducible.

I agree that the current state is sub-optimal and a method to safely select a tree node is desirable. For me this raises the question whether having a select method on the master tree widget with a custom implementation is the way to go in general: The TreeModel already offers methods to select nodes based on the node itself (and not a name path). Maybe we could use this instead. Do you by chance have any insights on this @alxflam ? @ndoschek do you have experience or insights on the TreeModel?

ndoschek commented 2 years ago

Issue seems to have stalled, please feel free to re-open