qMRMLSubjectHierarchyTreeView::setCurrentItem proposal

Dear Slicer developers,

I would like to get some feedback on a potential fix in qMRMLSubjectHierarchyTreeView: ENH: qMRMLSubjectHierarchyTreeView: call setCurrentIndex rather than … · Slicer/Slicer@1fdee41 · GitHub

In my custom 3d slicer app I load a case which:

  • contains only 2 volumes in the data tree
  • sets the current item of a qMRMLSubjectHierarchyCombobox to the last volume (this combobox is used in my custom module)

Scenario 1:

  • Load the case.
  • Remove the last volume from the data tree.
  • Result: the combobox displays None.

Scenario 2:

  • Load the case.
  • Click on the combobox and click on the last volume (which is already selected)
  • Remove the last volume from the data tree.
  • Result: the combobox displays the first volume.

Analysis

When the last volume is removed from the data tree, this method is called:

QAbstractItemView::rowsAboutToBeRemoved

This method checks the current index:

if the current index is valid then the next index is selected (previous volume => scenario 2)

if the current index is NOT valid then no index is selected (None => scenario 1)

In scenario 1 there is no current index because qMRMLSubjectHierarchyCombobox simply calls qMRMLSubjectHierarchyTreeView which calls only selectionModel()->select which does not set the current index.

As you can see in this commit , this->selectionModel()->setCurrentIndex selects the item AND set the current index.

Do you see any case where an item must be selected without being the current index ?

Do you see any other issue ?

Best regards

Both behaviors are very useful. The developer should be able to choose between them using the noneEnabled flag the same way as in the node combobox.

noneEnabled is already set to True in my qMRMLSubjectHierarchyCombobox.

Unfortunately I do not find any way to create a reproducer in 3D Slicer.

According to me, removing a row of the subject hierarchy tree should always lead to the same behavior in a qMRMLSubjectHierarchyCombobox, it should not change if the user already clicked on an item or not.

Please let me know if this commit could be interesting or if I have to implement a fix only in my custom app.

Regards

Yes, I agree. If this is not the case then it is a bug that should be fixed.

This widget is intended to mirror the behavior of qMRMLNodeComboBox, making it suitable as a drop-in replacement.

If this is the case, then I think it is a bug, and your change is well justified. I can’t think of a case when we want to set selection only in the view without setting the same selection in the model.

Thanks for the investigation!