View Issue Details

IDProjectCategoryView StatusLast Update
0004422Slicer4Module SubjectHierarchypublic2017-08-28 11:12
Reporterdzenanz Assigned Topinter  
PrioritynormalSeveritymajorReproducibilityalways
Status closedResolutionfixed 
Product Version 
Target VersionFixed in VersionSlicer 4.7.0 
Summary0004422: currentItem for multiple SubjectHierarchyComboBoxes is the same
Description

With 2 instances of qMRMLSubjectHierarchyComboBox, the latest selected thing in either is return by currentItem() for both of them. They can show different nodes selected, one might not show any node selected (the initial state).

MWE here https://github.com/dzenanz/Slicer/commit/b8d36f445e7619f2d2f5b941b05f924c1e622dae

TagsNo tags attached.

Activities

pinter

pinter

2017-08-25 12:27

developer   ~0015080

Last edited: 2017-08-25 12:42

View 2 revisions

Thanks for the report!

I did some digging and what you experience is probably* due to the unique way "current item" is handled in subject hierarchy because of the plugins. Let me first explain why this is. Because the subject hierarchy plugins do not have access to the widgets, the central, singleton plugin handler class is used to retrieve the "current item". This current item is the one that just has been right clicked, and the plugins need to 1. offer functions for that item in the context menu, and 2. perform the action on the item that was selected by the user. Worth noting that on multi-selection, currentItem (in the plugin handler) returns the first selected item, and currentItems return all. The combo box has a tree view inside, which, when the current item is asked for, gets the "current item" from this plugin handler.
https://github.com/Slicer/Slicer/blob/master/Modules/Loadable/SubjectHierarchy/Widgets/qMRMLSubjectHierarchyTreeView.cxx#L349
This is a remnant from SH1 (back in the SH1 times, there was only one SH tree view in the SH module, and there were no comboboxes and re-usable tree views), that definitely needs to be changed.

What I think would be best is if the tree view simply got the selected items and returned them. This would solve the problem you reported. The currentItem function would return the first selected item, and currentItems would return all of them.

(*) I did not build your MWE, and in this form will not. There are multiple, much easier ways to provide an example:

  1. An external module which can be built separately and simply added to the additional module directories in Slicer
  2. Even better, just a python code that I can copy-paste into the interactor
dzenanz

dzenanz

2017-08-25 13:23

developer   ~0015081

Thanks for the explanation. Will you work on the fix, or should I?

pinter

pinter

2017-08-25 13:27

developer   ~0015082

I believe I already implemented the fix.
I'll test it thoroughly and then commit. Unfortunately I became sick yesterday so not sure if I can get this done today, but soon!

pinter

pinter

2017-08-25 17:41

developer   ~0015083

I commited a fix
https://github.com/Slicer/Slicer/commit/468067bca94d6bf42b3b01254d3410229a2cf8d3
Please check out tomorrow's nightly and close the issue if it looks good.

dzenanz

dzenanz

2017-08-28 11:03

developer   ~0015086

Now it works as intended. Thanks Csaba!

pinter

pinter

2017-08-28 11:06

developer   ~0015087

Great! Can you please close this issue?

Issue History

Date Modified Username Field Change
2017-08-24 14:21 dzenanz New Issue
2017-08-24 14:21 dzenanz Status new => assigned
2017-08-24 14:21 dzenanz Assigned To => pinter
2017-08-25 12:27 pinter Note Added: 0015080
2017-08-25 12:42 pinter Note Edited: 0015080 View Revisions
2017-08-25 13:23 dzenanz Note Added: 0015081
2017-08-25 13:27 pinter Note Added: 0015082
2017-08-25 17:41 pinter Status assigned => resolved
2017-08-25 17:41 pinter Resolution open => fixed
2017-08-25 17:41 pinter Fixed in Version => Slicer 4.7.0
2017-08-25 17:41 pinter Note Added: 0015083
2017-08-28 11:03 dzenanz Note Added: 0015086
2017-08-28 11:06 pinter Note Added: 0015087
2017-08-28 11:12 dzenanz Status resolved => closed