diff --git a/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor_table.py b/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor_table.py index 235172a63..ab11d7327 100644 --- a/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor_table.py +++ b/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor_table.py @@ -67,7 +67,7 @@ class ParameterEditorTableDialogs: ask_yes_no: Callable[[str, str], bool] = ask_yesno_popup -class ParameterEditorTable(ScrollFrame): # pylint: disable=too-many-ancestors +class ParameterEditorTable(ScrollFrame): # pylint: disable=too-many-ancestors, too-many-instance-attributes """ A class to manage and display the parameter editor table within the GUI. @@ -94,6 +94,15 @@ def __init__( self._last_return_values: dict[tk.Misc, str] = {} self._pending_scroll_to_bottom = False + self._params_list: list[tuple[str, ArduPilotParameter]] = [] + self._total_params: int = 0 + self._current_idx: int = 0 + self._gui_complexity: str = "" + self._is_loading: bool = False + self._add_button_widget: Optional[ttk.Button] = None + self._page_scroll_after_id: Optional[str] = None + self._page_scroll_destroy_binding_registered: bool = False + style = ttk.Style() style.configure("narrow.TButton", padding=0, width=4, border=(0, 0, 0, 0)) @@ -219,44 +228,125 @@ def _apply_scroll_position(self, scroll_to_bottom: bool) -> None: self.canvas.yview_moveto(position) def _update_table(self, params: dict[str, ArduPilotParameter], gui_complexity: str) -> None: - """Update the parameter table with the given parameters.""" - current_param_name: str = "" - show_upload_column = self._should_show_upload_column(gui_complexity) + """ + Update the table from a new parameter set and start incremental rendering. - should_try_to_display_bitmask_parameter_editor_usage = False - try: - for i, (param_name, param) in enumerate(params.items(), 1): - current_param_name = param_name + This method is the entry point used when the table is rebuilt for a + fresh ``params`` mapping. It resets the incremental-load state, stores + the active GUI complexity, renders the first batch of rows immediately + (currently 20 parameters), and then starts the page-scroll/polling + mechanism that loads additional chunks as needed. + """ + self._params_list = list(params.items()) + self._total_params = len(self._params_list) + self._current_idx = 0 + self._gui_complexity = gui_complexity + self._is_loading = False + + self._load_next_chunk(chunk_size=20) + self._page_scroll() + + def _load_next_chunk(self, chunk_size: int = 15) -> None: + """Appends the next batch of widgets to the bottom of the table.""" + if self._is_loading or self._current_idx >= self._total_params: + return - row_widgets: list[tk.Widget] = self._create_column_widgets(param_name, param, show_upload_column) - if self.parameter_editor.should_display_bitmask_parameter_editor_usage(param_name): - should_try_to_display_bitmask_parameter_editor_usage = True - self._grid_column_widgets(row_widgets, i, show_upload_column) + self._is_loading = True + show_upload_column = self._should_show_upload_column(self._gui_complexity) + should_show_bitmask_usage = False - # Add the "Add" button at the bottom of the table - add_button = ttk.Button(self.view_port, text=_("Add"), style="narrow.TButton", command=self._on_parameter_add) - tooltip_msg = _("Add a parameter to the {self.parameter_editor.current_file} file") - show_tooltip(add_button, tooltip_msg.format(**locals())) - add_button.grid(row=len(params) + 2, column=0, sticky="w", padx=0) + start_idx = self._current_idx + end_idx = min(start_idx + chunk_size, self._total_params) + if self._add_button_widget and self._add_button_widget.winfo_exists(): + self._add_button_widget.destroy() + + param_name = "" + try: + for i in range(start_idx, end_idx): + param_name, param = self._params_list[i] + row_widgets = self._create_column_widgets(param_name, param, show_upload_column) + if self.parameter_editor.should_display_bitmask_parameter_editor_usage(param_name): + should_show_bitmask_usage = True + self._grid_column_widgets(row_widgets, i + 1, show_upload_column) except KeyError as e: logging_critical( _("Parameter %s not found in the %s file: %s"), - current_param_name, + param_name, self.parameter_editor.current_file, e, exc_info=True, ) sys_exit(1) + self._current_idx = end_idx + + # Fix layout if all items are loaded + if self._current_idx >= self._total_params: + self._add_button_widget = ttk.Button( + self.view_port, text=_("Add"), style="narrow.TButton", command=self._on_parameter_add + ) + show_tooltip( + self._add_button_widget, + _("Add a parameter to the {self.parameter_editor.current_file} file").format(**locals()), + ) + self._add_button_widget.grid(row=self._total_params + 2, column=0, sticky="w", pady=(10, 0)) + + parent_root = self._get_parent_root() + if parent_root and should_show_bitmask_usage and UsagePopupWindow.should_display("bitmask_parameter_editor"): + display_bitmask_parameters_editor_usage_popup(parent_root) + self._configure_table_columns(show_upload_column) - parent_root = self._get_parent_root() - if ( - parent_root - and should_try_to_display_bitmask_parameter_editor_usage - and UsagePopupWindow.should_display("bitmask_parameter_editor") - ): - display_bitmask_parameters_editor_usage_popup(parent_root) + self.view_port.update_idletasks() + self.canvas.configure(scrollregion=self.canvas.bbox("all")) + self._is_loading = False + + def _cancel_page_scroll_after(self) -> None: + """Cancel any scheduled page-scroll polling callback.""" + after_id = getattr(self, "_page_scroll_after_id", None) + if not after_id: + return + try: + self.after_cancel(after_id) + except tk.TclError: + pass + finally: + self._page_scroll_after_id = None + + def _on_destroy_cancel_page_scroll(self, _event: Optional[tk.Event] = None) -> None: + """Stop page-scroll polling when the widget is being destroyed.""" + self._cancel_page_scroll_after() + + def _page_scroll(self) -> None: + """Load more items when reaching the bottom.""" + self._page_scroll_after_id = None + + # 1. ALWAYS check if the widget exists FIRST before doing Tkinter operations + if not self.winfo_exists(): + self._cancel_page_scroll_after() + return + + # 2. Now it is safe to bind events + if not getattr(self, "_page_scroll_destroy_binding_registered", False): + self.bind("", self._on_destroy_cancel_page_scroll, add="+") + self._page_scroll_destroy_binding_registered = True + + # 3. Check if we are done loading + if self._current_idx >= self._total_params: + self._cancel_page_scroll_after() + return + + try: + if self.canvas.yview()[1] > 0.85: + self._load_next_chunk(chunk_size=15) + except tk.TclError: + return + + if self.winfo_exists() and self._current_idx < self._total_params: + try: + self._page_scroll_after_id = self.after(150, self._page_scroll) + except tk.TclError: + self._page_scroll_after_id = None def _create_column_widgets(self, param_name: str, param: ArduPilotParameter, show_upload_column: bool) -> list[tk.Widget]: """Create all column widgets for a parameter row.""" diff --git a/ardupilot_methodic_configurator/frontend_tkinter_scroll_frame.py b/ardupilot_methodic_configurator/frontend_tkinter_scroll_frame.py index 02c920e98..1b061145d 100644 --- a/ardupilot_methodic_configurator/frontend_tkinter_scroll_frame.py +++ b/ardupilot_methodic_configurator/frontend_tkinter_scroll_frame.py @@ -85,8 +85,11 @@ def on_canvas_configure(self, event: tk.Event) -> None: self.canvas.itemconfig(self.canvas_window, width=canvas_width) def on_mouse_wheel(self, event: tk.Event) -> None: # cross platform scroll wheel event + if not hasattr(self, "canvas") or not self.canvas.winfo_exists(): + return # The widget was destroyed, ignore the scroll event canvas_height = self.canvas.winfo_height() - rows_height = self.canvas.bbox("all")[3] + bbox = self.canvas.bbox("all") + rows_height = bbox[3] if bbox is not None else 0 if rows_height > canvas_height: # only scroll if the rows overflow the frame if platform_system() == "Windows": diff --git a/ardupilot_methodic_configurator/frontend_tkinter_show.py b/ardupilot_methodic_configurator/frontend_tkinter_show.py index 3675b5c9a..d971765c2 100644 --- a/ardupilot_methodic_configurator/frontend_tkinter_show.py +++ b/ardupilot_methodic_configurator/frontend_tkinter_show.py @@ -477,33 +477,13 @@ def __init__( # pylint: disable=too-many-arguments, too-many-positional-argumen self.timers: dict[str, Optional[str]] = {} # Bind the and events to show and hide the tooltip - if platform_system() == "Darwin": - # On macOS, defer tooltip creation slightly to avoid flashing while - # moving through dense tables and controls. - if tag_name and isinstance(self.widget, tk.Text): - self.widget.tag_bind(tag_name, "", self.schedule_show, "+") - self.widget.tag_bind(tag_name, "", self.destroy_hide, "+") - else: - self.widget.bind("", self.schedule_show, "+") - self.widget.bind("", self.destroy_hide, "+") + # Defer tooltip creation slightly to avoid flashing while moving through dense tables. + if tag_name and isinstance(self.widget, tk.Text): + self.widget.tag_bind(tag_name, "", self.schedule_show, "+") + self.widget.tag_bind(tag_name, "", self.destroy_hide, "+") else: - if tag_name and isinstance(self.widget, tk.Text): - self.widget.tag_bind(tag_name, "", self.show, "+") - self.widget.tag_bind(tag_name, "", self.hide, "+") - else: - self.widget.bind("", self.show, "+") - self.widget.bind("", self.hide, "+") - # On non-macOS, create the tooltip immediately and show/hide it on events - self.tooltip = cast("tk.Toplevel", self.toplevel_class(widget)) - self.tooltip.wm_overrideredirect(boolean=True) - tooltip_label = ttk.Label( - self.tooltip, text=text, background="#ffffe0", relief="solid", borderwidth=1, justify=tk.LEFT - ) - tooltip_label.pack() - self.tooltip.withdraw() # Initially hide the tooltip - # Bind to tooltip to prevent hiding when mouse is over it - self.tooltip.bind("", self._cancel_hide) - self.tooltip.bind("", self.hide) + self.widget.bind("", self.schedule_show, "+") + self.widget.bind("", self.destroy_hide, "+") self.widget.bind("", self._on_widget_destroy, "+") @@ -517,15 +497,6 @@ def _cancel_timer(self, name: str) -> None: def _cancel_show(self) -> None: self._cancel_timer("show") - def show(self, event: Optional[tk.Event] = None) -> None: # noqa: ARG002 # pylint: disable=unused-argument - """On non-macOS, tooltip already exists, show it on events.""" - self._cancel_hide() - self._hide_active_tooltip() - if self.tooltip: - self.position_tooltip() - self.tooltip.deiconify() - Tooltip._active_tooltip = self - def _cancel_hide(self, event: Optional[tk.Event] = None) -> None: # noqa: ARG002 # pylint: disable=unused-argument self._cancel_timer("hide") @@ -547,14 +518,14 @@ def _hide_active_tooltip(self) -> None: Tooltip._active_tooltip.force_hide() Tooltip._active_tooltip = None - def schedule_show(self, event: Optional[tk.Event] = None) -> None: # noqa: ARG002 # pylint: disable=unused-argument + def schedule_show(self, _event: Optional[tk.Event] = None) -> None: """Delay tooltip creation slightly to avoid flicker during pointer movement.""" self._cancel_hide() self._cancel_show() self.timers["show"] = self.widget.after(TOOLTIP_SHOW_DELAY_MS, self.create_show) - def create_show(self, event: Optional[tk.Event] = None) -> None: # noqa: ARG002 # pylint: disable=unused-argument - """On macOS, only create the tooltip when the mouse enters the widget.""" + def create_show(self, _event: Optional[tk.Event] = None) -> None: + """Create and show the tooltip when the pointer is still over the widget after the delay.""" self._cancel_show() self._cancel_hide() @@ -589,7 +560,7 @@ def create_show(self, event: Optional[tk.Event] = None) -> None: # noqa: ARG002 "noActivates", ) self.tooltip.configure(bg="#ffffe0") - except AttributeError: # Catches protected member access error + except (AttributeError, tk.TclError): # Catches protected member access error self.tooltip.wm_attributes("-alpha", 1.0) # Ensure opacity self.tooltip.wm_attributes("-topmost", True) # Keep on top # noqa: FBT003 self.tooltip.configure(bg="#ffffe0") @@ -652,30 +623,14 @@ def position_tooltip(self) -> None: # Silently ignore - tooltip will be recreated on next hover if needed pass - def hide(self, event: Optional[tk.Event] = None) -> None: # noqa: ARG002 # pylint: disable=unused-argument - """Hide the tooltip after a delay on non-macOS.""" - self._cancel_hide() - self.timers["hide"] = self.widget.after(TOOLTIP_HIDE_DELAY_MS, self._do_hide) - - def _do_hide(self) -> None: - """Actually hide or destroy the tooltip depending on platform.""" - if self.tooltip: - self.tooltip.withdraw() - if Tooltip._active_tooltip is self: - Tooltip._active_tooltip = None - self.timers.pop("hide", None) - def force_hide(self) -> None: - """Immediately hide or destroy the tooltip, depending on platform.""" + """Immediately destroy the tooltip globally across all OSs.""" self._cancel_show() self._cancel_hide() self._cancel_timer("alpha") if self.tooltip: - if platform_system() == "Darwin": - self.tooltip.destroy() - self.tooltip = None - else: - self.tooltip.withdraw() + self.tooltip.destroy() + self.tooltip = None if Tooltip._active_tooltip is self: Tooltip._active_tooltip = None diff --git a/tests/bdd_frontend_tkinter_show.py b/tests/bdd_frontend_tkinter_show.py index 313b99f71..62233289f 100755 --- a/tests/bdd_frontend_tkinter_show.py +++ b/tests/bdd_frontend_tkinter_show.py @@ -23,7 +23,6 @@ import pytest from ardupilot_methodic_configurator.frontend_tkinter_show import ( - TOOLTIP_HIDE_DELAY_MS, TOOLTIP_SHOW_DELAY_MS, MonitorBounds, Tooltip, @@ -747,19 +746,15 @@ def test_tooltip_initialization_non_macos(self, mock_widget: MagicMock, mock_top with ( patch("ardupilot_methodic_configurator.frontend_tkinter_show.platform_system", return_value="Linux"), patch("tkinter.Toplevel", return_value=mock_toplevel), - patch("tkinter.ttk.Label") as mock_label, + patch("tkinter.ttk.Label"), ): tooltip = Tooltip(mock_widget, "Test text") - # Check that Toplevel was created - assert tooltip.tooltip == mock_toplevel + # Check that Toplevel was NOT created yet (Lazy Loading) + assert tooltip.tooltip is None # Check bindings - mock_widget.bind.assert_any_call("", tooltip.show, "+") - mock_widget.bind.assert_any_call("", tooltip.hide, "+") - # Check tooltip setup - mock_toplevel.wm_overrideredirect.assert_called_with(boolean=True) - mock_label.assert_called_once() - mock_toplevel.withdraw.assert_called_once() + mock_widget.bind.assert_any_call("", tooltip.schedule_show, "+") + mock_widget.bind.assert_any_call("", tooltip.destroy_hide, "+") def test_tooltip_initialization_macos(self, mock_widget: MagicMock) -> None: """ @@ -822,10 +817,10 @@ def test_tooltip_position_tooltip(self, mock_widget, mock_toplevel) -> None: ), ): tooltip = Tooltip(mock_widget, "Test text") + mock_widget.winfo_containing.return_value = mock_widget # Tell the test the mouse is hovering + tooltip.create_show() - tooltip.position_tooltip() - - # Check that geometry was set + # Check that geometry was set (create_show did this automatically!) mock_toplevel.geometry.assert_called_once() # The call should be with calculated position call_args = mock_toplevel.geometry.call_args[0][0] @@ -844,9 +839,10 @@ def test_tooltip_show_non_macos(self, mock_widget, mock_toplevel) -> None: patch("tkinter.ttk.Label"), patch("ardupilot_methodic_configurator.frontend_tkinter_show.platform_system", return_value="Linux"), ): + mock_widget.winfo_containing.return_value = mock_widget tooltip = Tooltip(mock_widget, "Test text") - tooltip.show() + tooltip.create_show() mock_toplevel.deiconify.assert_called_once() @@ -864,10 +860,12 @@ def test_tooltip_hide(self, mock_widget, mock_toplevel) -> None: patch("ardupilot_methodic_configurator.frontend_tkinter_show.platform_system", return_value="Linux"), ): tooltip = Tooltip(mock_widget, "Test text") + tooltip.tooltip = mock_toplevel # Mock that it was created + tooltip.destroy_hide() - tooltip.hide() - - mock_widget.after.assert_called_once_with(TOOLTIP_HIDE_DELAY_MS, tooltip._do_hide) + # As it destroys immediately, check if active_tooltip is cleared + assert Tooltip._active_tooltip is None + mock_toplevel.destroy.assert_called_once() def test_tooltip_cancel_hide(self, mock_widget) -> None: """ @@ -990,9 +988,9 @@ def test_tooltip_no_position_when_tooltip_none(self, mock_widget) -> None: # Assert: No exception, no positioning attempted - def test_tooltip_do_hide_withdraws_on_non_macos(self, mock_widget, mock_toplevel) -> None: + def test_tooltip_force_hide_destroys_globally(self, mock_widget, mock_toplevel) -> None: """ - Test tooltip _do_hide withdraws tooltip on non-macOS. + Test tooltip force_hide destroys the tooltip globally across all OSs and clears the active tooltip. GIVEN: Tooltip on non-macOS platform WHEN: _do_hide is called @@ -1003,11 +1001,14 @@ def test_tooltip_do_hide_withdraws_on_non_macos(self, mock_widget, mock_toplevel patch("tkinter.ttk.Label"), patch("ardupilot_methodic_configurator.frontend_tkinter_show.platform_system", return_value="Linux"), ): + mock_widget.winfo_containing.return_value = mock_widget tooltip = Tooltip(mock_widget, "Test text") + tooltip.create_show() # Must create it before we can destroy it - tooltip._do_hide() + tooltip.force_hide() - mock_toplevel.withdraw.assert_called() + mock_toplevel.destroy.assert_called_once() + assert tooltip.tooltip is None def test_tooltip_create_show_avoids_redundant_creation(self, mock_widget, mock_toplevel) -> None: """ @@ -1067,10 +1068,13 @@ def test_tooltip_with_custom_toplevel_class(self, mock_widget, mock_toplevel) -> patch("ardupilot_methodic_configurator.frontend_tkinter_show.platform_system", return_value="Linux"), patch("tkinter.ttk.Label"), ): + mock_widget.winfo_containing.return_value = mock_widget tooltip = Tooltip(mock_widget, "Test text", toplevel_class=custom_toplevel_class) + tooltip.create_show() # Trigger lazy load + # Assert: Custom class was used - custom_toplevel_class.assert_called_once_with(mock_widget) + custom_toplevel_class.assert_called_once_with(mock_widget, bg="#ffffe0") assert tooltip.tooltip == mock_toplevel def test_tooltip_position_below_false(self, mock_widget, mock_toplevel) -> None: @@ -1091,9 +1095,10 @@ def test_tooltip_position_below_false(self, mock_widget, mock_toplevel) -> None: ), ): tooltip = Tooltip(mock_widget, "Test text", position_below=False) - tooltip.position_tooltip() + mock_widget.winfo_containing.return_value = mock_widget # Tell the test the mouse is hovering + tooltip.create_show() - # Assert: Geometry set (indicates positioning occurred) + # Assert: Geometry set mock_toplevel.geometry.assert_called_once() def test_tooltip_create_show_uses_macos_styling(self, mock_widget, mock_toplevel) -> None: