Skip to content

Commit ef4326f

Browse files
fix: adressed copilot
Signed-off-by: Omkar Sarkar <omkarsarkar24@gmail.com>
1 parent d2e9c58 commit ef4326f

4 files changed

Lines changed: 54 additions & 8 deletions

File tree

ardupilot_methodic_configurator/frontend_tkinter_parameter_editor_table.py

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,8 @@ def __init__(
100100
self._gui_complexity: str = ""
101101
self._is_loading: bool = False
102102
self._add_button_widget: Optional[ttk.Button] = None
103+
self._page_scroll_after_id: Optional[str] = None
104+
self._page_scroll_destroy_binding_registered: bool = False
103105

104106
style = ttk.Style()
105107
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:
226228
self.canvas.yview_moveto(position)
227229

228230
def _update_table(self, params: dict[str, ArduPilotParameter], gui_complexity: str) -> None:
229-
"""Initialize scroll load."""
231+
"""
232+
Update the table from a new parameter set and start incremental rendering.
233+
234+
This method is the entry point used when the table is rebuilt for a
235+
fresh ``params`` mapping. It resets the incremental-load state, stores
236+
the active GUI complexity, renders the first batch of rows immediately
237+
(currently 20 parameters), and then starts the page-scroll/polling
238+
mechanism that loads additional chunks as needed.
239+
"""
230240
self._params_list = list(params.items())
231241
self._total_params = len(self._params_list)
232242
self._current_idx = 0
@@ -291,18 +301,52 @@ def _load_next_chunk(self, chunk_size: int = 15) -> None:
291301
self.canvas.configure(scrollregion=self.canvas.bbox("all"))
292302
self._is_loading = False
293303

304+
def _cancel_page_scroll_after(self) -> None:
305+
"""Cancel any scheduled page-scroll polling callback."""
306+
after_id = getattr(self, "_page_scroll_after_id", None)
307+
if not after_id:
308+
return
309+
try:
310+
self.after_cancel(after_id)
311+
except tk.TclError:
312+
pass
313+
finally:
314+
self._page_scroll_after_id = None
315+
316+
def _on_destroy_cancel_page_scroll(self, _event: Optional[tk.Event] = None) -> None:
317+
"""Stop page-scroll polling when the widget is being destroyed."""
318+
self._cancel_page_scroll_after()
319+
294320
def _page_scroll(self) -> None:
295321
"""Load more items when reaching the bottom."""
296-
if not self.winfo_exists() or self._current_idx >= self._total_params:
322+
self._page_scroll_after_id = None
323+
324+
# 1. ALWAYS check if the widget exists FIRST before doing Tkinter operations
325+
if not self.winfo_exists():
326+
self._cancel_page_scroll_after()
327+
return
328+
329+
# 2. Now it is safe to bind events
330+
if not getattr(self, "_page_scroll_destroy_binding_registered", False):
331+
self.bind("<Destroy>", self._on_destroy_cancel_page_scroll, add="+")
332+
self._page_scroll_destroy_binding_registered = True
333+
334+
# 3. Check if we are done loading
335+
if self._current_idx >= self._total_params:
336+
self._cancel_page_scroll_after()
297337
return
298338

299339
try:
300340
if self.canvas.yview()[1] > 0.85:
301341
self._load_next_chunk(chunk_size=15)
302342
except tk.TclError:
303-
pass
343+
return
304344

305-
self.after(150, self._page_scroll)
345+
if self.winfo_exists() and self._current_idx < self._total_params:
346+
try:
347+
self._page_scroll_after_id = self.after(150, self._page_scroll)
348+
except tk.TclError:
349+
self._page_scroll_after_id = None
306350

307351
def _create_column_widgets(self, param_name: str, param: ArduPilotParameter, show_upload_column: bool) -> list[tk.Widget]:
308352
"""Create all column widgets for a parameter row."""

ardupilot_methodic_configurator/frontend_tkinter_scroll_frame.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,8 @@ def on_mouse_wheel(self, event: tk.Event) -> None: # cross platform scroll whee
8888
if not hasattr(self, "canvas") or not self.canvas.winfo_exists():
8989
return # The widget was destroyed, ignore the scroll event
9090
canvas_height = self.canvas.winfo_height()
91-
rows_height = self.canvas.bbox("all")[3]
91+
bbox = self.canvas.bbox("all")
92+
rows_height = bbox[3] if bbox is not None else 0
9293

9394
if rows_height > canvas_height: # only scroll if the rows overflow the frame
9495
if platform_system() == "Windows":

ardupilot_methodic_configurator/frontend_tkinter_show.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -525,7 +525,7 @@ def schedule_show(self, _event: Optional[tk.Event] = None) -> None:
525525
self.timers["show"] = self.widget.after(TOOLTIP_SHOW_DELAY_MS, self.create_show)
526526

527527
def create_show(self, _event: Optional[tk.Event] = None) -> None:
528-
"""On macOS, only create the tooltip when the mouse enters the widget."""
528+
"""Create and show the tooltip when the pointer is still over the widget after the delay."""
529529
self._cancel_show()
530530
self._cancel_hide()
531531

tests/bdd_frontend_tkinter_show.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -990,7 +990,7 @@ def test_tooltip_no_position_when_tooltip_none(self, mock_widget) -> None:
990990

991991
def test_tooltip_force_hide_destroys_globally(self, mock_widget, mock_toplevel) -> None:
992992
"""
993-
Test tooltip _do_hide withdraws tooltip on non-macOS.
993+
Test tooltip force_hide destroys the tooltip globally across all OSs and clears the active tooltip.
994994
995995
GIVEN: Tooltip on non-macOS platform
996996
WHEN: _do_hide is called
@@ -1074,7 +1074,8 @@ def test_tooltip_with_custom_toplevel_class(self, mock_widget, mock_toplevel) ->
10741074
tooltip.create_show() # Trigger lazy load
10751075

10761076
# Assert: Custom class was used
1077-
custom_toplevel_class.assert_called_once()
1077+
custom_toplevel_class.assert_called_once_with(mock_widget, bg="#ffffe0")
1078+
assert tooltip.tooltip == mock_toplevel
10781079

10791080
def test_tooltip_position_below_false(self, mock_widget, mock_toplevel) -> None:
10801081
"""

0 commit comments

Comments
 (0)