Add new inspector for favorite nodes
Audited13e7f96912aa
Actions

Description

Add new inspector for favorite nodes

Add buttons to the selection dialog and the inspector to favor and
disfavor specific data nodes.

Details

Auditors

floca
kalali

Provenance

kalali	Authored on Jan 14 2020, 6:35 PM
kalali	Pushed on Jan 14 2020, 6:40 PM

Parents

rMITK258838df6a80: Merge branch 'T26992-FixRenderingTestHelper'

Branches

Unknown

Tags

Unknown

Event Timeline

kalali committed rMITK13e7f96912aa: Add new inspector for favorite nodes (authored by kalali).Jan 14 2020, 6:35 PM

Some remarks as written in T24659#191634

/Modules/QtWidgets/include/QmitkAbstractDataStorageInspector.h
77	I needed to add this here so that I can manually update a view if the bool-property of the datanode has been changed. The problem is, that the selection node should not depend on the `QmitkDataStorageFavoriteNodesInspector` so it needs a more general access to trigger the update. All other inspectors do not implement this function so in their case nothing happens. Would be better to somehow let the favoriteNodesInspector know that another inspector (opened in another tab) made changes to some nodes (their properties) so that the view is updated if the favoriteNodesInspector is opened (the tab is selected).
/Modules/QtWidgets/src/QmitkDataStorageFavoriteNodesInspector.cpp
56	Here I am able to directly update the storage model to use the newly deselected nodes are removed from the model and are not shown in the list view. Here a node-property observer-pattern would also work, since we know on which nodes we have to listen. But as this does only work for nodes-to-be-removed and not with nodes we want to add (nodes we do not know but which will be added as favorites in the future), I did not choose to use such a pattern.
/Plugins/org.mitk.gui.qt.common/src/QmitkNodeSelectionDialog.cpp
173	Problem here is that the nodes' properties are changed but the favoriteNodesInspector does not receive these changes. So if you switch to the favoriteNodesInspector-tab, the recently changed favorite nodes are not displayed. I cannot directly access the favoriteNodesInspector and somehow call an update, since the dialog knows nothing about any favoriteNodesInspector.

kalali added an auditor: floca.Jan 14 2020, 6:58 PM

This commit now requires audit.Jan 14 2020, 6:58 PM

floca raised a concern with this commit.Jan 14 2020, 9:40 PM

floca added a subscriber: floca.

floca added inline comments.

/Modules/QtWidgets/include/QmitkAbstractDataStorageInspector.h
77	Cannot this be covered by using ChangedNodeEvents for DataStorageModels in general to trigger an update as soon as a node is changed? This should also lead to updated views/inspectors. Or did I miss the point?
/Plugins/org.mitk.gui.qt.common/src/QmitkNodeSelectionDialog.cpp
173	Ok, so ChangedNodeEvent does not work? But if it is the case, then I would tend to not introduce this work arround but refere to the task T22322, where we have already identified that any property modification should also modify the node -> fixing this task would also fix this problem. thus I would keep the current weakness of the inspector, document it in a task as a "known issue", refering to T22322 to fix it. In our current nomal workflows it should a big deal anyway.

This commit now has outstanding concerns.Jan 14 2020, 9:40 PM

floca added inline comments.Jan 14 2020, 9:43 PM

/Modules/QtWidgets/src/QmitkDataStorageFavoriteNodesInspector.cpp
24	As the caption is currently the only information it should be "Remove selection from favorites". Or you add a hint that explains it.

kalali marked an inline comment as done.Jan 15 2020, 11:47 AM

kalali added inline comments.

/Modules/QtWidgets/include/QmitkAbstractDataStorageInspector.h
77	See explanation below.
/Plugins/org.mitk.gui.qt.common/src/QmitkNodeSelectionDialog.cpp
173	`ChangedNodeEvent` could work but we disabled it in the `QmitkDataStorageDefaultListModel`, since the 'NodeChanged'-event is currently sent far too often. We could allow this now or add another `QmitkDataStorageFavoriteNodesListModel` that reacts to this event. However, this means that the refreshing of the view will happen on many node changes, such as visibility, `select`-property etc. This might not be a problem if the favoriteNodesInspector is only used inside the NodeSelectionDialog since here we typically don't have these node-changes (like change visibility, rendering properties etc.). The problem you mentioned is related to the the renderer-specific properties and is not related to this exact problem, as far as I understood.

Discussed it offline.

All concerns with this commit have now been addressed.Jan 15 2020, 1:44 PM

kalali marked 2 inline comments as done.Jan 15 2020, 6:25 PM

kalali added inline comments.

/Modules/QtWidgets/include/QmitkAbstractDataStorageInspector.h
77	We decided to go with the `ChangedNodeEvents` (see explanation below).
/Plugins/org.mitk.gui.qt.common/src/QmitkNodeSelectionDialog.cpp
173	We decided to use the `ChangedNodeEvent` and observe the behavior. If it is too time consuming, since we send too many property-changes, we have to rethink the concept. For now we will take the path of least resistance.