From 71c110c96dbaf71ebf35c1a144cf56f04c9c7b0c Mon Sep 17 00:00:00 2001 From: Omkar Sarkar Date: Wed, 22 Apr 2026 11:46:02 +0530 Subject: [PATCH 1/5] fix: timer and on-demand load along with fast loading on large parameter table Signed-off-by: Omkar Sarkar --- ...frontend_tkinter_parameter_editor_table.py | 86 +++++++++++++------ .../frontend_tkinter_show.py | 79 ++++++----------- 2 files changed, 90 insertions(+), 75 deletions(-) diff --git a/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor_table.py b/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor_table.py index 235172a63..a8b0e5d68 100644 --- a/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor_table.py +++ b/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor_table.py @@ -219,44 +219,82 @@ 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) + """Initialize scroll load.""" + 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 - 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 + self._is_loading = True + show_upload_column = self._should_show_upload_column(self._gui_complexity) + should_show_bitmask_usage = False - 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) + start_idx = self._current_idx + end_idx = min(start_idx + chunk_size, self._total_params) - # 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) + if hasattr(self, "_add_button_widget") and self._add_button_widget.winfo_exists(): + self._add_button_widget.destroy() + 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 _page_scroll(self) -> None: + """Load more items when reaching the bottom.""" + if not self.winfo_exists() or self._current_idx >= self._total_params: + return + + try: + if self.canvas.yview()[1] > 0.85: + self._load_next_chunk(chunk_size=15) + except tk.TclError: + pass + + self.after(150, self._page_scroll) 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_show.py b/ardupilot_methodic_configurator/frontend_tkinter_show.py index 3675b5c9a..4846c7eae 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,14 +497,14 @@ 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 show(self, event: Optional[tk.Event] = None) -> None: # 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") @@ -589,7 +569,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,18 +632,18 @@ 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 hide(self, event: Optional[tk.Event] = None) -> None: # 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 _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.""" @@ -671,11 +651,8 @@ def force_hide(self) -> None: 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 From a1bb4d93b2dc5e051628c6bbea98dc57e827c5be Mon Sep 17 00:00:00 2001 From: Omkar Sarkar Date: Wed, 22 Apr 2026 21:30:35 +0530 Subject: [PATCH 2/5] fix: cleanup and scroll noatt fixed Signed-off-by: Omkar Sarkar --- .../frontend_tkinter_parameter_editor_table.py | 12 ++++++++++-- .../frontend_tkinter_scroll_frame.py | 2 ++ .../frontend_tkinter_show.py | 5 ++--- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor_table.py b/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor_table.py index a8b0e5d68..b8aebd8d3 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,13 @@ 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 + style = ttk.Style() style.configure("narrow.TButton", padding=0, width=4, border=(0, 0, 0, 0)) @@ -241,9 +248,10 @@ def _load_next_chunk(self, chunk_size: int = 15) -> None: start_idx = self._current_idx end_idx = min(start_idx + chunk_size, self._total_params) - if hasattr(self, "_add_button_widget") and self._add_button_widget.winfo_exists(): + 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] diff --git a/ardupilot_methodic_configurator/frontend_tkinter_scroll_frame.py b/ardupilot_methodic_configurator/frontend_tkinter_scroll_frame.py index 02c920e98..4774995aa 100644 --- a/ardupilot_methodic_configurator/frontend_tkinter_scroll_frame.py +++ b/ardupilot_methodic_configurator/frontend_tkinter_scroll_frame.py @@ -85,6 +85,8 @@ 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] diff --git a/ardupilot_methodic_configurator/frontend_tkinter_show.py b/ardupilot_methodic_configurator/frontend_tkinter_show.py index 4846c7eae..36df3d8d9 100644 --- a/ardupilot_methodic_configurator/frontend_tkinter_show.py +++ b/ardupilot_methodic_configurator/frontend_tkinter_show.py @@ -646,13 +646,12 @@ def position_tooltip(self) -> None: # self.timers.pop("hide", None) def force_hide(self) -> None: - """Immediately hide or destroy the tooltip, depending on platform.""" + """Immediately hide the tooltip globally across all OSs.""" self._cancel_show() self._cancel_hide() self._cancel_timer("alpha") if self.tooltip: - self.tooltip.destroy() - self.tooltip = None + self.tooltip.withdraw() # Withdraw instead of destroy to prevent UI jank if Tooltip._active_tooltip is self: Tooltip._active_tooltip = None From eb4b640630128484e41cf68db3ceb62b91c2897a Mon Sep 17 00:00:00 2001 From: Omkar Sarkar Date: Wed, 22 Apr 2026 22:17:54 +0530 Subject: [PATCH 3/5] fix: MacOs tooltip destroy Signed-off-by: Omkar Sarkar --- ardupilot_methodic_configurator/frontend_tkinter_show.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ardupilot_methodic_configurator/frontend_tkinter_show.py b/ardupilot_methodic_configurator/frontend_tkinter_show.py index 36df3d8d9..eab3a72e2 100644 --- a/ardupilot_methodic_configurator/frontend_tkinter_show.py +++ b/ardupilot_methodic_configurator/frontend_tkinter_show.py @@ -646,12 +646,13 @@ def position_tooltip(self) -> None: # self.timers.pop("hide", None) def force_hide(self) -> None: - """Immediately hide the tooltip globally across all OSs.""" + """Immediately destroy the tooltip globally across all OSs.""" self._cancel_show() self._cancel_hide() self._cancel_timer("alpha") if self.tooltip: - self.tooltip.withdraw() # Withdraw instead of destroy to prevent UI jank + self.tooltip.destroy() + self.tooltip = None if Tooltip._active_tooltip is self: Tooltip._active_tooltip = None From d2e9c58fb2b1d36383f6affb908ab84576e27846 Mon Sep 17 00:00:00 2001 From: Omkar Sarkar Date: Fri, 24 Apr 2026 14:31:19 +0530 Subject: [PATCH 4/5] fix: updated test and pylint issue Signed-off-by: Omkar Sarkar --- .../frontend_tkinter_show.py | 26 +--------- tests/bdd_frontend_tkinter_show.py | 52 ++++++++++--------- 2 files changed, 30 insertions(+), 48 deletions(-) diff --git a/ardupilot_methodic_configurator/frontend_tkinter_show.py b/ardupilot_methodic_configurator/frontend_tkinter_show.py index eab3a72e2..0418e1871 100644 --- a/ardupilot_methodic_configurator/frontend_tkinter_show.py +++ b/ardupilot_methodic_configurator/frontend_tkinter_show.py @@ -497,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: # 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") @@ -527,13 +518,13 @@ 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 + def create_show(self, _event: Optional[tk.Event] = None) -> None: """On macOS, only create the tooltip when the mouse enters the widget.""" self._cancel_show() self._cancel_hide() @@ -632,19 +623,6 @@ 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: # 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 destroy the tooltip globally across all OSs.""" self._cancel_show() diff --git a/tests/bdd_frontend_tkinter_show.py b/tests/bdd_frontend_tkinter_show.py index 313b99f71..db53f1d2c 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,7 +988,7 @@ 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. @@ -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,11 +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) - assert tooltip.tooltip == mock_toplevel + custom_toplevel_class.assert_called_once() def test_tooltip_position_below_false(self, mock_widget, mock_toplevel) -> None: """ @@ -1091,9 +1094,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: From ef4326f4545208f3545dfb6cace77a0bd269690c Mon Sep 17 00:00:00 2001 From: Omkar Sarkar Date: Tue, 28 Apr 2026 23:01:05 +0530 Subject: [PATCH 5/5] fix: adressed copilot Signed-off-by: Omkar Sarkar --- ...frontend_tkinter_parameter_editor_table.py | 52 +++++++++++++++++-- .../frontend_tkinter_scroll_frame.py | 3 +- .../frontend_tkinter_show.py | 2 +- tests/bdd_frontend_tkinter_show.py | 5 +- 4 files changed, 54 insertions(+), 8 deletions(-) diff --git a/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor_table.py b/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor_table.py index b8aebd8d3..ab11d7327 100644 --- a/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor_table.py +++ b/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor_table.py @@ -100,6 +100,8 @@ def __init__( 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)) @@ -226,7 +228,15 @@ 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: - """Initialize scroll load.""" + """ + Update the table from a new parameter set and start incremental rendering. + + 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 @@ -291,18 +301,52 @@ def _load_next_chunk(self, chunk_size: int = 15) -> None: 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.""" - if not self.winfo_exists() or self._current_idx >= self._total_params: + 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: - pass + return - self.after(150, self._page_scroll) + 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 4774995aa..1b061145d 100644 --- a/ardupilot_methodic_configurator/frontend_tkinter_scroll_frame.py +++ b/ardupilot_methodic_configurator/frontend_tkinter_scroll_frame.py @@ -88,7 +88,8 @@ def on_mouse_wheel(self, event: tk.Event) -> None: # cross platform scroll whee 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 0418e1871..d971765c2 100644 --- a/ardupilot_methodic_configurator/frontend_tkinter_show.py +++ b/ardupilot_methodic_configurator/frontend_tkinter_show.py @@ -525,7 +525,7 @@ def schedule_show(self, _event: Optional[tk.Event] = None) -> None: self.timers["show"] = self.widget.after(TOOLTIP_SHOW_DELAY_MS, self.create_show) def create_show(self, _event: Optional[tk.Event] = None) -> None: - """On macOS, only create the tooltip when the mouse enters the widget.""" + """Create and show the tooltip when the pointer is still over the widget after the delay.""" self._cancel_show() self._cancel_hide() diff --git a/tests/bdd_frontend_tkinter_show.py b/tests/bdd_frontend_tkinter_show.py index db53f1d2c..62233289f 100755 --- a/tests/bdd_frontend_tkinter_show.py +++ b/tests/bdd_frontend_tkinter_show.py @@ -990,7 +990,7 @@ def test_tooltip_no_position_when_tooltip_none(self, mock_widget) -> 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 @@ -1074,7 +1074,8 @@ def test_tooltip_with_custom_toplevel_class(self, mock_widget, mock_toplevel) -> tooltip.create_show() # Trigger lazy load # Assert: Custom class was used - custom_toplevel_class.assert_called_once() + 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: """