Page MenuHomePhabricator

[Selection widgets] Enabling auto selection emits a signal that might not yet be connected
Closed, ResolvedPublic

Description

We recently updated some important plugins to work with the new selection concept and new selection widgets (see T24775: Refactor plugins to use the new node selection widgets).
Then we had a discussion about the default selection mode of the selection widgets (see T27498: [Selection widgets] Define default for auto selection / listening mechanism).

We are now facing an non intuitive situation where a developer of a plugin has to know that calling SelectionWidget->SetAutoSelectNewNodes(true); needs to happen after something like connect(SelectionWidget, &QmitkSingleNodeSelectionWidget::CurrentSelectionChanged, this, &SomePlugin::OnCurrentSelectionChanged); (which is sometimes put into a function called CreateConnections or SetupConnections).

The reason for that is that SetAutoSelectNewNodes creates a new selection and eventually emits the signal CurrentSelectionChanged, which has not been connected, if the call happens before the connect.

I want to discuss this and talk about possible solutions or our default way to do this. We now have some views that simply put the call to SetAutoSelectNewNodes at the end of (typically) CreateQtPartControl. This is absolutely valid but I'm not sure if this is the best option.

Revisions and Commits

rMITK MITK
Restricted Differential Revision
Restricted Differential Revision
Restricted Differential Revision

Event Timeline

kalali triaged this task as Normal priority.Aug 14 2020, 5:51 PM
kalali created this task.
kalali added a project: Request for Discussion.
floca edited projects, added Cleared, Restricted Project; removed Request for Discussion.Sep 1 2020, 5:53 PM

Discussion result:
As you cannot hook into the Qt connect mechanism, I see no "simple" chance to automatically manage that under the hood. The most pragmatic solution is to simply add a remark to the class (QmitkSingleNodeSelectionWidget) and method (SetAutoSelectNewNodes()) that on should activate this feature *after* connecting the events to ensure already the first autoselect has an effect.

Thank you, I wanted to post the discussion result here as well. I will fix this.

kalali added a revision: Restricted Differential Revision.Sep 3 2020, 2:02 PM
kalali removed a project: Restricted Project.

Closed with changes of D398 as discussed: Remark for developers.