search

saros-project / saros

Open Source IDE plugin for distributed collaborative software development
https://www.saros-project.org
GNU General Public License v2.0
158 stars 52 forks source link

Fix Saros view update behavior #1130

Closed tobous closed 3 years ago

tobous commented 3 years ago

[FIX][I] #888 Fix Saros view tree model update behavior

Introduces new methods to reload the entire tree model or a specific node while trying to retain the current tree expansion. To do so, the model saves all expanded paths before the reload and restores them after the reload.

Replaces all calls to reloading the model with calls to the new utility methods to ensure that the expansion state is preserved as part of the reload.

Moves the responsibility of reloading the contacts node for session related events (e.g. when hiding a contact from the contacts list when they join the current session) to the contacts node class.

Replaces calls to expanding specific rows of the tree with calls to expanding the path of the element that is actually supposed to be expanded. This removes the guesswork whether the given row actually points to the correct element and ensures that the logic still works when the tree layout is changed.

Resolves #888.

[UI][I] Expand contacts node when first contact is added

Adds new logic explicitly expanding the contacts view if the first contact is added. This can be the case when there were no contacts before or when all contacts were part of the session the user just left. This ensures that the contact node is expanded in such cases.

tobous commented 3 years ago

The entire Saros/I Saros view implementation could use an overhaul, but I did not feel like rewriting at this point in time. So this is just a fix for the apparent/visible issues.

srossbach commented 3 years ago

Isnt there a better way to listen to model changes in the TreeViewer ?

tobous commented 3 years ago

Isnt there a better way to listen to model changes in the TreeViewer ?

As already mentioned, I see this more as a quick fix since I did not want to re-write the logic at this point in time (even tough it would need it). I adjusted the logic to now (hopefully) properly use the mechanisms that were already being used for the updating beforehand.

I don't have any real prior experience working with JTrees, so I can't comment on proper change handling. This way works and it should preserve any user changes made to the tree expansion state. Also I would like to mention that while the variable is named treeView, it has nothing to do with the Eclipse-specific TreeViewer class. This just deals with JTree, DefaultTreeModel, and DefaultTreeNode objects.

From the javadoc [reload()](https://docs.oracle.com/javase/7/docs/api/javax/swing/tree/DefaultTreeModel.html#reload()) should be the proper method to call after any nodes in the tree were changed. Also, we aren't really interested in changes to the model as it should not be able to be changed by any other source. Rather, we are interested in changes to the session and XMPP contacts state, according to which we then update the tree.