Skip to content

Commit 7d1dc41

Browse files
OmkarSarkar204amilcarlucas
authored andcommitted
fix(frontend_tkinter_show): timer and on-demand load along with fast loading on large parameter table
Signed-off-by: Omkar Sarkar <omkarsarkar24@gmail.com>
1 parent 0a2d63b commit 7d1dc41

2 files changed

Lines changed: 45 additions & 85 deletions

File tree

ardupilot_methodic_configurator/frontend_tkinter_show.py

Lines changed: 13 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -477,33 +477,13 @@ def __init__( # pylint: disable=too-many-arguments, too-many-positional-argumen
477477
self.timers: dict[str, Optional[str]] = {}
478478

479479
# Bind the <Enter> and <Leave> events to show and hide the tooltip
480-
if platform_system() == "Darwin":
481-
# On macOS, defer tooltip creation slightly to avoid flashing while
482-
# moving through dense tables and controls.
483-
if tag_name and isinstance(self.widget, tk.Text):
484-
self.widget.tag_bind(tag_name, "<Enter>", self.schedule_show, "+")
485-
self.widget.tag_bind(tag_name, "<Leave>", self.destroy_hide, "+")
486-
else:
487-
self.widget.bind("<Enter>", self.schedule_show, "+")
488-
self.widget.bind("<Leave>", self.destroy_hide, "+")
480+
# Defer tooltip creation slightly to avoid flashing while moving through dense tables.
481+
if tag_name and isinstance(self.widget, tk.Text):
482+
self.widget.tag_bind(tag_name, "<Enter>", self.schedule_show, "+")
483+
self.widget.tag_bind(tag_name, "<Leave>", self.destroy_hide, "+")
489484
else:
490-
if tag_name and isinstance(self.widget, tk.Text):
491-
self.widget.tag_bind(tag_name, "<Enter>", self.show, "+")
492-
self.widget.tag_bind(tag_name, "<Leave>", self.hide, "+")
493-
else:
494-
self.widget.bind("<Enter>", self.show, "+")
495-
self.widget.bind("<Leave>", self.hide, "+")
496-
# On non-macOS, create the tooltip immediately and show/hide it on events
497-
self.tooltip = cast("tk.Toplevel", self.toplevel_class(widget))
498-
self.tooltip.wm_overrideredirect(boolean=True)
499-
tooltip_label = ttk.Label(
500-
self.tooltip, text=text, background="#ffffe0", relief="solid", borderwidth=1, justify=tk.LEFT
501-
)
502-
tooltip_label.pack()
503-
self.tooltip.withdraw() # Initially hide the tooltip
504-
# Bind to tooltip to prevent hiding when mouse is over it
505-
self.tooltip.bind("<Enter>", self._cancel_hide)
506-
self.tooltip.bind("<Leave>", self.hide)
485+
self.widget.bind("<Enter>", self.schedule_show, "+")
486+
self.widget.bind("<Leave>", self.destroy_hide, "+")
507487

508488
self.widget.bind("<Destroy>", self._on_widget_destroy, "+")
509489

@@ -517,15 +497,6 @@ def _cancel_timer(self, name: str) -> None:
517497
def _cancel_show(self) -> None:
518498
self._cancel_timer("show")
519499

520-
def show(self, event: Optional[tk.Event] = None) -> None: # noqa: ARG002 # pylint: disable=unused-argument
521-
"""On non-macOS, tooltip already exists, show it on events."""
522-
self._cancel_hide()
523-
self._hide_active_tooltip()
524-
if self.tooltip:
525-
self.position_tooltip()
526-
self.tooltip.deiconify()
527-
Tooltip._active_tooltip = self
528-
529500
def _cancel_hide(self, event: Optional[tk.Event] = None) -> None: # noqa: ARG002 # pylint: disable=unused-argument
530501
self._cancel_timer("hide")
531502

@@ -547,14 +518,14 @@ def _hide_active_tooltip(self) -> None:
547518
Tooltip._active_tooltip.force_hide()
548519
Tooltip._active_tooltip = None
549520

550-
def schedule_show(self, event: Optional[tk.Event] = None) -> None: # noqa: ARG002 # pylint: disable=unused-argument
521+
def schedule_show(self, _event: Optional[tk.Event] = None) -> None:
551522
"""Delay tooltip creation slightly to avoid flicker during pointer movement."""
552523
self._cancel_hide()
553524
self._cancel_show()
554525
self.timers["show"] = self.widget.after(TOOLTIP_SHOW_DELAY_MS, self.create_show)
555526

556-
def create_show(self, event: Optional[tk.Event] = None) -> None: # noqa: ARG002 # pylint: disable=unused-argument
557-
"""On macOS, only create the tooltip when the mouse enters the widget."""
527+
def create_show(self, _event: Optional[tk.Event] = None) -> None:
528+
"""Create and show the tooltip when the pointer is still over the widget after the delay."""
558529
self._cancel_show()
559530
self._cancel_hide()
560531

@@ -589,7 +560,7 @@ def create_show(self, event: Optional[tk.Event] = None) -> None: # noqa: ARG002
589560
"noActivates",
590561
)
591562
self.tooltip.configure(bg="#ffffe0")
592-
except AttributeError: # Catches protected member access error
563+
except (AttributeError, tk.TclError): # Catches protected member access error
593564
self.tooltip.wm_attributes("-alpha", 1.0) # Ensure opacity
594565
self.tooltip.wm_attributes("-topmost", True) # Keep on top # noqa: FBT003
595566
self.tooltip.configure(bg="#ffffe0")
@@ -652,30 +623,14 @@ def position_tooltip(self) -> None:
652623
# Silently ignore - tooltip will be recreated on next hover if needed
653624
pass
654625

655-
def hide(self, event: Optional[tk.Event] = None) -> None: # noqa: ARG002 # pylint: disable=unused-argument
656-
"""Hide the tooltip after a delay on non-macOS."""
657-
self._cancel_hide()
658-
self.timers["hide"] = self.widget.after(TOOLTIP_HIDE_DELAY_MS, self._do_hide)
659-
660-
def _do_hide(self) -> None:
661-
"""Actually hide or destroy the tooltip depending on platform."""
662-
if self.tooltip:
663-
self.tooltip.withdraw()
664-
if Tooltip._active_tooltip is self:
665-
Tooltip._active_tooltip = None
666-
self.timers.pop("hide", None)
667-
668626
def force_hide(self) -> None:
669-
"""Immediately hide or destroy the tooltip, depending on platform."""
627+
"""Immediately destroy the tooltip globally across all OSs."""
670628
self._cancel_show()
671629
self._cancel_hide()
672630
self._cancel_timer("alpha")
673631
if self.tooltip:
674-
if platform_system() == "Darwin":
675-
self.tooltip.destroy()
676-
self.tooltip = None
677-
else:
678-
self.tooltip.withdraw()
632+
self.tooltip.destroy()
633+
self.tooltip = None
679634
if Tooltip._active_tooltip is self:
680635
Tooltip._active_tooltip = None
681636

tests/bdd_frontend_tkinter_show.py

Lines changed: 32 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import pytest
2424

2525
from ardupilot_methodic_configurator.frontend_tkinter_show import (
26-
TOOLTIP_HIDE_DELAY_MS,
2726
TOOLTIP_SHOW_DELAY_MS,
2827
MonitorBounds,
2928
Tooltip,
@@ -747,19 +746,15 @@ def test_tooltip_initialization_non_macos(self, mock_widget: MagicMock, mock_top
747746
with (
748747
patch("ardupilot_methodic_configurator.frontend_tkinter_show.platform_system", return_value="Linux"),
749748
patch("tkinter.Toplevel", return_value=mock_toplevel),
750-
patch("tkinter.ttk.Label") as mock_label,
749+
patch("tkinter.ttk.Label"),
751750
):
752751
tooltip = Tooltip(mock_widget, "Test text")
753752

754-
# Check that Toplevel was created
755-
assert tooltip.tooltip == mock_toplevel
753+
# Check that Toplevel was NOT created yet (Lazy Loading)
754+
assert tooltip.tooltip is None
756755
# Check bindings
757-
mock_widget.bind.assert_any_call("<Enter>", tooltip.show, "+")
758-
mock_widget.bind.assert_any_call("<Leave>", tooltip.hide, "+")
759-
# Check tooltip setup
760-
mock_toplevel.wm_overrideredirect.assert_called_with(boolean=True)
761-
mock_label.assert_called_once()
762-
mock_toplevel.withdraw.assert_called_once()
756+
mock_widget.bind.assert_any_call("<Enter>", tooltip.schedule_show, "+")
757+
mock_widget.bind.assert_any_call("<Leave>", tooltip.destroy_hide, "+")
763758

764759
def test_tooltip_initialization_macos(self, mock_widget: MagicMock) -> None:
765760
"""
@@ -822,10 +817,10 @@ def test_tooltip_position_tooltip(self, mock_widget, mock_toplevel) -> None:
822817
),
823818
):
824819
tooltip = Tooltip(mock_widget, "Test text")
820+
mock_widget.winfo_containing.return_value = mock_widget # Tell the test the mouse is hovering
821+
tooltip.create_show()
825822

826-
tooltip.position_tooltip()
827-
828-
# Check that geometry was set
823+
# Check that geometry was set (create_show did this automatically!)
829824
mock_toplevel.geometry.assert_called_once()
830825
# The call should be with calculated position
831826
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:
844839
patch("tkinter.ttk.Label"),
845840
patch("ardupilot_methodic_configurator.frontend_tkinter_show.platform_system", return_value="Linux"),
846841
):
842+
mock_widget.winfo_containing.return_value = mock_widget
847843
tooltip = Tooltip(mock_widget, "Test text")
848844

849-
tooltip.show()
845+
tooltip.create_show()
850846

851847
mock_toplevel.deiconify.assert_called_once()
852848

@@ -864,10 +860,12 @@ def test_tooltip_hide(self, mock_widget, mock_toplevel) -> None:
864860
patch("ardupilot_methodic_configurator.frontend_tkinter_show.platform_system", return_value="Linux"),
865861
):
866862
tooltip = Tooltip(mock_widget, "Test text")
863+
tooltip.tooltip = mock_toplevel # Mock that it was created
864+
tooltip.destroy_hide()
867865

868-
tooltip.hide()
869-
870-
mock_widget.after.assert_called_once_with(TOOLTIP_HIDE_DELAY_MS, tooltip._do_hide)
866+
# As it destroys immediately, check if active_tooltip is cleared
867+
assert Tooltip._active_tooltip is None
868+
mock_toplevel.destroy.assert_called_once()
871869

872870
def test_tooltip_cancel_hide(self, mock_widget) -> None:
873871
"""
@@ -990,24 +988,27 @@ def test_tooltip_no_position_when_tooltip_none(self, mock_widget) -> None:
990988

991989
# Assert: No exception, no positioning attempted
992990

993-
def test_tooltip_do_hide_withdraws_on_non_macos(self, mock_widget, mock_toplevel) -> None:
991+
def test_tooltip_force_hide_destroys_globally(self, mock_widget, mock_toplevel) -> None:
994992
"""
995-
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.
996994
997-
GIVEN: Tooltip on non-macOS platform
998-
WHEN: _do_hide is called
999-
THEN: Tooltip is withdrawn
995+
GIVEN: Tooltip on any platform
996+
WHEN: force_hide is called
997+
THEN: Tooltip is destroyed and active tooltip is cleared
1000998
"""
1001999
with (
10021000
patch("tkinter.Toplevel", return_value=mock_toplevel),
10031001
patch("tkinter.ttk.Label"),
10041002
patch("ardupilot_methodic_configurator.frontend_tkinter_show.platform_system", return_value="Linux"),
10051003
):
1004+
mock_widget.winfo_containing.return_value = mock_widget
10061005
tooltip = Tooltip(mock_widget, "Test text")
1006+
tooltip.create_show() # Must create it before we can destroy it
10071007

1008-
tooltip._do_hide()
1008+
tooltip.force_hide()
10091009

1010-
mock_toplevel.withdraw.assert_called()
1010+
mock_toplevel.destroy.assert_called_once()
1011+
assert tooltip.tooltip is None
10111012

10121013
def test_tooltip_create_show_avoids_redundant_creation(self, mock_widget, mock_toplevel) -> None:
10131014
"""
@@ -1067,10 +1068,13 @@ def test_tooltip_with_custom_toplevel_class(self, mock_widget, mock_toplevel) ->
10671068
patch("ardupilot_methodic_configurator.frontend_tkinter_show.platform_system", return_value="Linux"),
10681069
patch("tkinter.ttk.Label"),
10691070
):
1071+
mock_widget.winfo_containing.return_value = mock_widget
10701072
tooltip = Tooltip(mock_widget, "Test text", toplevel_class=custom_toplevel_class)
10711073

1074+
tooltip.create_show() # Trigger lazy load
1075+
10721076
# Assert: Custom class was used
1073-
custom_toplevel_class.assert_called_once_with(mock_widget)
1077+
custom_toplevel_class.assert_called_once_with(mock_widget, bg="#ffffe0")
10741078
assert tooltip.tooltip == mock_toplevel
10751079

10761080
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:
10911095
),
10921096
):
10931097
tooltip = Tooltip(mock_widget, "Test text", position_below=False)
1094-
tooltip.position_tooltip()
1098+
mock_widget.winfo_containing.return_value = mock_widget # Tell the test the mouse is hovering
1099+
tooltip.create_show()
10951100

1096-
# Assert: Geometry set (indicates positioning occurred)
1101+
# Assert: Geometry set
10971102
mock_toplevel.geometry.assert_called_once()
10981103

10991104
def test_tooltip_create_show_uses_macos_styling(self, mock_widget, mock_toplevel) -> None:

0 commit comments

Comments
 (0)