diff --git a/bec_widgets/widgets/control/scan_control/scan_control.py b/bec_widgets/widgets/control/scan_control/scan_control.py index bb6d69d2..45078546 100644 --- a/bec_widgets/widgets/control/scan_control/scan_control.py +++ b/bec_widgets/widgets/control/scan_control/scan_control.py @@ -132,7 +132,8 @@ class ScanControl(BECWidget, QWidget): self._last_scan_fetch_watchdog.setSingleShot(True) self._last_scan_fetch_watchdog.setInterval(self.LAST_SCAN_FETCH_TIMEOUT_MS) self._last_scan_fetch_watchdog.timeout.connect(self._on_last_scan_parameters_timeout) - self._selected_scan: str | None = None + self._selected_scan: str = "" + self._scan_name_typed = False # Create and set main layout self._init_UI() @@ -197,7 +198,9 @@ class ScanControl(BECWidget, QWidget): self.layout.addWidget(self.scan_control_group) # Connect signals - self.comboBox_scan_selection.currentTextChanged.connect(self.on_scan_selection_changed) + self.comboBox_scan_selection.currentIndexChanged.connect(self._on_scan_index_changed) + self.comboBox_scan_selection.currentTextChanged.connect(self._on_scan_text_changed) + self.comboBox_scan_selection.lineEdit().textEdited.connect(self._on_scan_name_typed) self.comboBox_scan_selection.lineEdit().editingFinished.connect( self.validate_scan_selection ) @@ -267,9 +270,13 @@ class ScanControl(BECWidget, QWidget): ] def _update_scan_selector(self) -> None: - """Apply the configured scan filter while preserving the current selection.""" - current_scan = self.comboBox_scan_selection.currentText() - if current_scan: + """Apply the configured scan filter while preserving the confirmed selection. + + The combo box text may hold a half-typed, unconfirmed name, so preservation and + change detection are keyed on the confirmed selection, never on the raw text. + """ + confirmed_scan = self._selected_scan + if confirmed_scan: self.save_current_scan_parameters() # Read the raw filter: ``None`` means "unset", which the property never reports. allowed_scans = self.config.allowed_scans @@ -289,15 +296,26 @@ class ScanControl(BECWidget, QWidget): render_scan_tooltip_html(scan_name, self._scan_docstring(scan_name)), Qt.ItemDataRole.ToolTipRole, ) - if current_scan in visible_scans: - self.comboBox_scan_selection.setCurrentText(current_scan) + if confirmed_scan in visible_scans: + self.comboBox_scan_selection.setCurrentText(confirmed_scan) self.scan_info_button.setEnabled(bool(visible_scans)) self._update_run_button_state() self._update_selected_scan_tooltip() - if self.comboBox_scan_selection.currentText() != current_scan: + # Repopulation under QSignalBlocker suppressed any style updates. + self._update_validity_style(self.is_valid_scan(self.comboBox_scan_selection.currentText())) + if not visible_scans: + self._clear_scan_selection() + elif self.comboBox_scan_selection.currentText() != confirmed_scan: self.on_scan_selection_changed(self.comboBox_scan_selection.currentText()) + def _clear_scan_selection(self) -> None: + """Drop the confirmed selection after the filter left the selector empty.""" + if not self._selected_scan: + return + self._selected_scan = "" + self.reset_layout() + def _update_run_button_state(self) -> None: """Start requires a selected scan and valid metadata.""" has_scan = bool(self.comboBox_scan_selection.currentText()) @@ -380,31 +398,44 @@ class ScanControl(BECWidget, QWidget): self._hide_scan_selector_settings_button = bool(hide) self.scan_selector_settings_button.setVisible(not self._hide_scan_selector_settings_button) - def on_scan_selection_changed(self, scan_name: str): - """Callback for the scan selection combo box. + def _on_scan_index_changed(self, index: int): + """The scan was selected from the dropdown, with the arrow keys or from code.""" + self.on_scan_selection_changed(self.comboBox_scan_selection.itemText(index)) - The combo box is editable, so the text changes with every keystroke. Only a name - that matches one of the listed scans switches the widget to that scan; any other - text is flagged as invalid input and reverted once editing is finished. + def _on_scan_name_typed(self, _scan_name: str): + """The scan name is being typed by the user and is not confirmed yet.""" + self._scan_name_typed = True + + def _on_scan_text_changed(self, scan_name: str): + """Callback for any text change of the editable scan selection combo box.""" + self._update_validity_style(self.is_valid_scan(scan_name)) + if self._scan_name_typed: + # A typed name only switches the scan once it is confirmed, so that the scan + # parameters are not rebuilt on every keystroke. + self._scan_name_typed = False + return + self.on_scan_selection_changed(scan_name) + + def on_scan_selection_changed(self, scan_name: str): + """Switches the widget to the given scan and ignores names that are not listed. Args: - scan_name(str): Current text of the scan selection combo box. + scan_name(str): Name of the scan to switch to. Resolved case-insensitively to + the listed scan name. """ - if not self.is_valid_scan(scan_name): - self._update_validity_style(False) + index = self.comboBox_scan_selection.findText(scan_name, Qt.MatchFixedString) + if index < 0: return - self._update_validity_style(True) + scan_name = self.comboBox_scan_selection.itemText(index) if scan_name == self._selected_scan: return - if self._selected_scan is not None: + if self._selected_scan: # Store the parameters of the scan we are leaving before its boxes are removed. self._save_scan_parameters(self._selected_scan) self._selected_scan = scan_name # Selecting by typing only changes the text; keep the current index in sync. - self.comboBox_scan_selection.setCurrentIndex( - self.comboBox_scan_selection.findText(scan_name) - ) + self.comboBox_scan_selection.setCurrentIndex(index) self._update_selected_scan_tooltip() self.scan_selected.emit(scan_name) self.restore_scan_parameters(scan_name) @@ -412,34 +443,44 @@ class ScanControl(BECWidget, QWidget): @SafeSlot() def validate_scan_selection(self): """ - Resolve the typed text to a valid scan once the user finished editing. + Confirms or discards the typed scan name once the user finished editing. - A name that differs only in case is completed to the listed scan, anything else + A listed scan is selected, also if the typed name differs in case, anything else falls back to the last valid selection. """ - scan_name = self.comboBox_scan_selection.currentText() - if self.is_valid_scan(scan_name): + index = self.comboBox_scan_selection.findText( + self.comboBox_scan_selection.currentText(), Qt.MatchFixedString + ) + if index < 0: + # Fall back to the confirmed selection - unless the filter removed it from the + # selector meanwhile, in which case a hidden name must not be re-displayed. + self.comboBox_scan_selection.setCurrentText( + self._selected_scan if self.is_valid_scan(self._selected_scan) else "" + ) return - index = self.comboBox_scan_selection.findText(scan_name, Qt.MatchFixedString) - if index >= 0: - self.comboBox_scan_selection.setCurrentIndex(index) - return - self.comboBox_scan_selection.setCurrentText(self._selected_scan or "") + self.comboBox_scan_selection.setCurrentIndex(index) + self.on_scan_selection_changed(self.comboBox_scan_selection.itemText(index)) def is_valid_scan(self, scan_name: str) -> bool: """Returns True if the given name is one of the scans listed in the combo box. + Case-insensitive, matching the completer and the confirmation rule: a name that + will be accepted on confirm must not be styled as invalid while it is typed. + Args: scan_name(str): Name of the scan to check. """ - return bool(scan_name) and self.comboBox_scan_selection.findText(scan_name) >= 0 + return ( + bool(scan_name) + and self.comboBox_scan_selection.findText(scan_name, Qt.MatchFixedString) >= 0 + ) def _update_validity_style(self, is_valid: bool): """Highlights the scan selection combo box while it holds an unknown scan name.""" - if is_valid: - self.comboBox_scan_selection.setStyleSheet("") - return - self.comboBox_scan_selection.setStyleSheet("QComboBox { border: 1px solid red; }") + style = "" if is_valid else "QComboBox { border: 1px solid red; }" + # Setting a style sheet repolishes the widget, so only do it on an actual change. + if self.comboBox_scan_selection.styleSheet() != style: + self.comboBox_scan_selection.setStyleSheet(style) @SafeSlot() @SafeSlot(bool) @@ -449,7 +490,8 @@ class ScanControl(BECWidget, QWidget): """ if not self.last_scan_button.isEnabled(): return - current_scan = self.comboBox_scan_selection.currentText() + # A scan name that is typed but not confirmed yet must not be fetched. + current_scan = self.current_scan if not current_scan: # e.g. an empty selector after a filter change - nothing to restore return @@ -605,7 +647,9 @@ class ScanControl(BECWidget, QWidget): if generation != self._last_scan_fetch_generation: # result of a timed-out or superseded fetch return - if self.comboBox_scan_selection.currentText() != scan_name: + # Compare against the confirmed selection: the combo box text may hold a scan name + # that is still being typed while the fetch completes. + if self.current_scan != scan_name: logger.debug(f"Discarding fetched parameters for {scan_name}: scan selection changed") return @@ -636,8 +680,11 @@ class ScanControl(BECWidget, QWidget): @SafeProperty(str) def current_scan(self): - """Returns the scan name for the currently selected scan.""" - return self.comboBox_scan_selection.currentText() + """Returns the scan name for the currently selected scan. + + A scan name that is typed but not confirmed yet is not reported here. + """ + return self._selected_scan @current_scan.setter def current_scan(self, scan_name: str): @@ -646,9 +693,10 @@ class ScanControl(BECWidget, QWidget): Args: scan_name(str): Name of the scan to set as current. """ - if not self.is_valid_scan(scan_name): - return - self.comboBox_scan_selection.setCurrentText(scan_name) + # Switch directly instead of relying on setCurrentText signal side effects: when + # the user has typed the exact target name without confirming it, the text does + # not change and no signal would fire, silently swallowing the request. + self.on_scan_selection_changed(scan_name) @SafeSlot(str) def set_current_scan(self, scan_name: str): @@ -922,7 +970,7 @@ class ScanControl(BECWidget, QWidget): def save_current_scan_parameters(self): """Saves the current scan parameters to the scan control config for further use.""" - self._save_scan_parameters(self.comboBox_scan_selection.currentText()) + self._save_scan_parameters(self.current_scan) def _save_scan_parameters(self, scan_name: str): """Saves the parameters currently shown in the group boxes under the given scan name. @@ -944,9 +992,9 @@ class ScanControl(BECWidget, QWidget): @SafeSlot(popup_error=True) def run_scan(self): """Starts the selected scan with the given parameters.""" - # The scan name may still be edited when the run button is clicked. + # The scan name may be typed but not confirmed when the run button is clicked. self.validate_scan_selection() - scan_name = self.comboBox_scan_selection.currentText() + scan_name = self.current_scan if not scan_name: return args, kwargs = self.get_scan_parameters() diff --git a/tests/unit_tests/test_scan_control.py b/tests/unit_tests/test_scan_control.py index 037a623e..37f299fe 100644 --- a/tests/unit_tests/test_scan_control.py +++ b/tests/unit_tests/test_scan_control.py @@ -962,12 +962,24 @@ def test_scan_selection_is_editable_with_completer(scan_control): assert completions == {"line_scan", "grid_scan"} -def test_typing_scan_name_selects_scan(scan_control, qtbot): +def _type_scan_name(qtbot, scan_control, scan_name: str): + """Replaces the content of the scan selection by typing the given name.""" + line_edit = scan_control.comboBox_scan_selection.lineEdit() + line_edit.clear() + qtbot.keyClicks(line_edit, scan_name) + + +def test_typing_scan_name_switches_scan_only_once_confirmed(scan_control, qtbot): combo = scan_control.comboBox_scan_selection + + _type_scan_name(qtbot, scan_control, "grid_scan") + + # typing alone does not rebuild the scan parameters assert scan_control.current_scan == "line_scan" + assert scan_control._metadata_form._scan_name == "line_scan" with qtbot.waitSignal(scan_control.scan_selected) as blocker: - combo.setEditText("grid_scan") + qtbot.keyClick(combo.lineEdit(), Qt.Key_Return) assert blocker.args == ["grid_scan"] assert scan_control.current_scan == "grid_scan" @@ -975,10 +987,19 @@ def test_typing_scan_name_selects_scan(scan_control, qtbot): assert combo.styleSheet() == "" +def test_selecting_scan_from_dropdown_switches_scan(scan_control): + combo = scan_control.comboBox_scan_selection + + combo.setCurrentIndex(combo.findText("grid_scan")) + + assert scan_control.current_scan == "grid_scan" + assert scan_control._metadata_form._scan_name == "grid_scan" + + def test_typing_unknown_scan_name_is_rejected(scan_control, qtbot): combo = scan_control.comboBox_scan_selection - combo.setEditText("grid") + _type_scan_name(qtbot, scan_control, "grid") # an incomplete name does not switch the scan, it is only flagged while editing assert scan_control._metadata_form._scan_name == "line_scan" @@ -987,47 +1008,61 @@ def test_typing_unknown_scan_name_is_rejected(scan_control, qtbot): qtbot.keyClick(combo.lineEdit(), Qt.Key_Return) assert combo.currentText() == "line_scan" + assert scan_control.current_scan == "line_scan" assert combo.styleSheet() == "" def test_typing_scan_name_with_different_case_is_completed(scan_control, qtbot): combo = scan_control.comboBox_scan_selection - combo.setEditText("GRID_SCAN") + _type_scan_name(qtbot, scan_control, "GRID_SCAN") qtbot.keyClick(combo.lineEdit(), Qt.Key_Return) assert combo.currentText() == "grid_scan" - assert scan_control._metadata_form._scan_name == "grid_scan" + assert scan_control.current_scan == "grid_scan" -def test_typing_scan_name_stores_previous_scan_parameters(scan_control): +def test_switching_scan_stores_previous_scan_parameters(scan_control): for kwarg_box in scan_control.kwarg_boxes: for widget in kwarg_box.widgets: if widget.arg_name == "exp_time": WidgetIO.set_value(widget, 3.0) - scan_control.comboBox_scan_selection.setEditText("grid_scan") + scan_control.comboBox_scan_selection.setCurrentText("grid_scan") assert scan_control.previous_scan == "line_scan" assert scan_control.config.scans["line_scan"].kwargs["exp_time"] == 3.0 -def test_run_scan_discards_unfinished_scan_name(scan_control): - combo = scan_control.comboBox_scan_selection - combo.setEditText("grid_scan") - combo.setEditText("grid_sca") +def test_run_scan_confirms_typed_scan_name(scan_control, qtbot): + _type_scan_name(qtbot, scan_control, "grid_scan") scans = SimpleNamespace(grid_scan=MagicMock()) with ( patch.object(scan_control, "scans", scans), - patch.object(scan_control, "get_scan_parameters", lambda: ((), {})), + patch.object(scan_control, "get_scan_parameters", lambda *_: ((), {})), ): scan_control.run_scan() - assert combo.currentText() == "grid_scan" + assert scan_control.current_scan == "grid_scan" scans.grid_scan.assert_called_once_with() +def test_run_scan_discards_unfinished_scan_name(scan_control, qtbot): + combo = scan_control.comboBox_scan_selection + _type_scan_name(qtbot, scan_control, "grid_sca") + + scans = SimpleNamespace(line_scan=MagicMock()) + with ( + patch.object(scan_control, "scans", scans), + patch.object(scan_control, "get_scan_parameters", lambda *_: ((), {})), + ): + scan_control.run_scan() + + assert combo.currentText() == "line_scan" + scans.line_scan.assert_called_once_with() + + def test_scan_switch_runs_cleanup_on_previous_inputs(scan_control): """Switching scans tears down the old group boxes; the BECWidget inputs inside (device comboboxes) must go through close() so their cleanup runs, instead of @@ -1891,3 +1926,90 @@ def test_restore_last_scan_parameters_memo_survives_overlapping_workers( args, kwargs = scan_control.get_scan_parameters(bec_object=False) assert args == ["samx", 0.0, 2.0] assert kwargs["steps"] == 10 + + +def test_filter_change_during_typing_keeps_confirmed_scan(scan_control, qtbot): + """A filter update arriving mid-typing must preserve the confirmed selection, not the + half-typed text.""" + combo = scan_control.comboBox_scan_selection + combo.setCurrentIndex(combo.findText("grid_scan")) + assert scan_control.current_scan == "grid_scan" + + _type_scan_name(qtbot, scan_control, "gri") # unconfirmed + scan_control.allowed_scans = ["line_scan", "grid_scan"] + + assert scan_control.current_scan == "grid_scan" + assert combo.currentText() == "grid_scan" + assert combo.styleSheet() == "" + assert scan_control._metadata_form._scan_name == "grid_scan" + + +def test_filter_change_during_typing_of_other_listed_scan(scan_control, qtbot): + """Typed text that happens to equal another listed scan must not survive a filter + update as a display/state divergence.""" + combo = scan_control.comboBox_scan_selection + combo.setCurrentIndex(combo.findText("grid_scan")) + _type_scan_name(qtbot, scan_control, "line_scan") # exact name, unconfirmed + + scan_control.allowed_scans = ["line_scan", "grid_scan"] + + # display and confirmed state agree again + assert combo.currentText() == scan_control.current_scan == "grid_scan" + + +def test_filter_removing_all_scans_clears_selection(scan_control, mocked_client, qtbot): + """An (intentionally supported) filter of only unavailable scans empties the selector; + the confirmed selection and its parameter boxes must not survive as orphans.""" + scan_control.allowed_scans = ["scan_that_does_not_exist_yet"] + + combo = scan_control.comboBox_scan_selection + assert combo.count() == 0 + assert scan_control.current_scan == "" + assert scan_control.arg_box is None + assert not scan_control.button_run_scan.isEnabled() + + # the restore button must not fetch for an empty selection + get_last = MagicMock(wraps=mocked_client.connector.get_last) + with patch.object(mocked_client.connector, "get_last", get_last): + scan_control.last_scan_button.click() + qtbot.wait(200) + get_last.assert_not_called() + assert scan_control.last_scan_button.isEnabled() + + # typing garbage and confirming must not resurrect the hidden scan + _type_scan_name(qtbot, scan_control, "junk") + qtbot.keyClick(combo.lineEdit(), Qt.Key_Return) + assert combo.currentText() == "" + + # clearing the filter brings the scans back and a scan can be selected again + scan_control.allowed_scans = None + assert combo.count() == 2 + combo.setCurrentIndex(combo.findText("grid_scan")) + assert scan_control.current_scan == "grid_scan" + + +def test_current_scan_setter_applies_over_identical_typed_text(scan_control, qtbot): + """Setting current_scan while the user has typed the exact same name (unconfirmed) + must still switch the scan - setCurrentText alone would emit no signal.""" + _type_scan_name(qtbot, scan_control, "grid_scan") # unconfirmed, still on line_scan + assert scan_control.current_scan == "line_scan" + + scan_control.set_current_scan("grid_scan") + + assert scan_control.current_scan == "grid_scan" + assert scan_control._metadata_form._scan_name == "grid_scan" + assert scan_control.comboBox_scan_selection.currentIndex() == ( + scan_control.comboBox_scan_selection.findText("grid_scan") + ) + + +def test_case_insensitive_typing_is_not_flagged_invalid(scan_control, qtbot): + """Validity styling must match the confirmation rule, which is case-insensitive.""" + combo = scan_control.comboBox_scan_selection + _type_scan_name(qtbot, scan_control, "GRID_SCAN") + + assert combo.styleSheet() == "" # not red: this name will be accepted on confirm + + qtbot.keyClick(combo.lineEdit(), Qt.Key_Return) + assert scan_control.current_scan == "grid_scan" + assert combo.currentText() == "grid_scan"