Skip to content

Commit 97202d0

Browse files
committed
refactor(ui): use self.center_window_on_screen() instead of BaseWindow.center_window_on_screen()
Address GitHub Copilot PR #1387 review findings: - Replace BaseWindow.center_window_on_screen(self.root) with self.center_window_on_screen(self.root) in all 8 subclass windows to respect polymorphism and allow instance-level mock patching - Strengthen withdrawn-parent centering test: reset mocks after __init__, assert exactly one call via update_progress_bar(), and verify window.progress_window is the argument passed - Add test_uses_rendered_size_when_window_is_mapped to assert that winfo_width/height (rendered size) is preferred over winfo_reqwidth/ winfo_reqheight (content size) when the window is already mapped
1 parent 47c35b9 commit 97202d0

10 files changed

Lines changed: 55 additions & 10 deletions

ardupilot_methodic_configurator/frontend_tkinter_component_editor_base.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ def _setup_window(self) -> None:
153153
_("Amilcar Lucas's - ArduPilot methodic configurator ") + self.version + _(" - Vehicle Component Editor")
154154
)
155155
self.root.geometry(self.calculate_scaled_geometry(WINDOW_WIDTH_PIX, 600))
156-
BaseWindow.center_window_on_screen(self.root)
156+
self.center_window_on_screen(self.root)
157157
self.root.protocol("WM_DELETE_WINDOW", self.on_closing)
158158

159159
def _setup_styles(self) -> None:

ardupilot_methodic_configurator/frontend_tkinter_connection_selection.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ def __init__(
317317
super().__init__()
318318
self.root.title(_("AMC {version} - Flight controller connection").format(version=__version__)) # Set the window title
319319
self.root.geometry(self.calculate_scaled_geometry(520, 462)) # Set the window size
320-
BaseWindow.center_window_on_screen(self.root)
320+
self.center_window_on_screen(self.root)
321321
self.default_baudrate = default_baudrate
322322

323323
# Explain why we are here

ardupilot_methodic_configurator/frontend_tkinter_flightcontroller_info.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ def __init__(self, flight_controller: FlightController, vehicle_dir: Path) -> No
7676
super().__init__()
7777
self.root.title(_("AMC {version} - Flight Controller Info").format(version=__version__)) # Set the window title
7878
self.root.geometry(self.calculate_scaled_geometry(500, 420)) # Adjust the window size as needed
79-
BaseWindow.center_window_on_screen(self.root)
79+
self.center_window_on_screen(self.root)
8080

8181
self.presenter = FlightControllerInfoPresenter(flight_controller, vehicle_dir)
8282

ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ def __init__(
254254
_("Amilcar Lucas's - ArduPilot methodic configurator ") + __version__ + _(" - Parameter file editor and uploader")
255255
)
256256
self.root.geometry(self.calculate_scaled_geometry(990, 630)) # Set the window width and height
257-
BaseWindow.center_window_on_screen(self.root)
257+
self.center_window_on_screen(self.root)
258258

259259
# Bind the close_connection_and_quit function to the window close event
260260
self.root.protocol("WM_DELETE_WINDOW", self.close_connection_and_quit)

ardupilot_methodic_configurator/frontend_tkinter_project_creator.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ def template_selection_callback(_widget: "DirectorySelectionWidgets") -> str:
138138
self.root.geometry(
139139
f"{round(800 * self.dpi_scaling_factor)}x{round(window_height * self.dpi_scaling_factor)}"
140140
) # Set the window size
141-
BaseWindow.center_window_on_screen(self.root)
141+
self.center_window_on_screen(self.root)
142142

143143
for setting_name, metadata in settings_metadata.items():
144144
checkbox = ttk.Checkbutton(

ardupilot_methodic_configurator/frontend_tkinter_project_opener.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ def __init__(self, project_manager: VehicleProjectManager) -> None:
5656
)
5757

5858
self.root.geometry(self.calculate_scaled_geometry(600, 450)) # Set the window size
59-
BaseWindow.center_window_on_screen(self.root)
59+
self.center_window_on_screen(self.root)
6060

6161
# Explain why we are here
6262
introduction_text = self.project_manager.get_introduction_message()

ardupilot_methodic_configurator/frontend_tkinter_software_update.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ def __init__(self, version_info: str, download_callback: Optional[Callable[[], b
2828
)
2929
)
3030
self.root.geometry(self.calculate_scaled_geometry(700, 600))
31-
BaseWindow.center_window_on_screen(self.root)
31+
self.center_window_on_screen(self.root)
3232
self.download_callback = download_callback
3333
self.root.protocol("WM_DELETE_WINDOW", self.on_cancel)
3434

ardupilot_methodic_configurator/frontend_tkinter_template_overview.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ def _configure_window(self) -> None:
114114

115115
# Scale window geometry for HiDPI displays
116116
self.root.geometry(self.calculate_scaled_geometry(1200, 600))
117-
BaseWindow.center_window_on_screen(self.root)
117+
self.center_window_on_screen(self.root)
118118

119119
def _initialize_ui_components(self) -> None:
120120
"""Initialize UI components with proper scaling."""

tests/test_frontend_tkinter_base_window.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1757,6 +1757,42 @@ def test_calls_update_idletasks_before_positioning(self) -> None:
17571757
winfo_idx = call_order.index("winfo_reqwidth")
17581758
assert update_idx < winfo_idx, "update_idletasks should be called before dimension queries"
17591759

1760+
def test_uses_rendered_size_when_window_is_mapped(self) -> None:
1761+
"""
1762+
Uses actual rendered window size (winfo_width/height) when window is already mapped.
1763+
1764+
GIVEN: A window that is already mapped and reports actual rendered dimensions via winfo_width/height
1765+
WHEN: Centering the window on screen
1766+
THEN: The rendered size should be used (not winfo_reqwidth/height) for correct centering
1767+
"""
1768+
with patch("ardupilot_methodic_configurator.frontend_tkinter_base_window.get_monitors") as mock_monitors:
1769+
# Arrange: Single 1920x1080 monitor at origin
1770+
monitor = MagicMock()
1771+
monitor.x = 0
1772+
monitor.y = 0
1773+
monitor.width = 1920
1774+
monitor.height = 1080
1775+
mock_monitors.return_value = [monitor]
1776+
1777+
# Mock window: winfo_width/height return actual rendered size (> 1),
1778+
# winfo_reqwidth/height return different (content-based) values
1779+
mock_window = MagicMock()
1780+
mock_window.winfo_width.return_value = 450 # actual rendered width
1781+
mock_window.winfo_height.return_value = 350 # actual rendered height
1782+
mock_window.winfo_reqwidth.return_value = 300 # content size (should NOT be used)
1783+
mock_window.winfo_reqheight.return_value = 200 # content size (should NOT be used)
1784+
mock_window.winfo_pointerx.return_value = 960
1785+
mock_window.winfo_pointery.return_value = 540
1786+
1787+
# Act: Center window on screen
1788+
BaseWindow.center_window_on_screen(mock_window)
1789+
1790+
# Assert: Rendered sizes (450x350) are used, not content sizes (300x200)
1791+
# Center calculation using rendered size: x = (1920 - 450) / 2 = 735
1792+
# y = (1080 - 350) / 2 = 365
1793+
mock_window.geometry.assert_called_once_with("+735+365")
1794+
mock_window.update.assert_called_once()
1795+
17601796

17611797
if __name__ == "__main__":
17621798
pytest.main([__file__])

tests/test_frontend_tkinter_progress_window.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -277,11 +277,20 @@ def test_user_sees_fc_init_progress_centered_on_screen_when_parent_is_withdrawn(
277277
only_show_when_update_progress_called=True,
278278
)
279279

280+
# Ignore any centering that may have happened during __init__
281+
mock_center_screen.reset_mock()
282+
mock_center_parent.reset_mock()
283+
284+
# First time the window is actually shown via update_progress_bar
280285
window.update_progress_bar(10, 100)
281286

282-
# Screen centering should be used, not parent-relative
283-
assert mock_center_screen.call_count >= 1
287+
# Screen centering should be used exactly once, not parent-relative, when showing via update_progress_bar
288+
mock_center_screen.assert_called_once()
284289
mock_center_parent.assert_not_called()
285290

291+
# The created progress window should be the one being centered
292+
args, _ = mock_center_screen.call_args
293+
assert window.progress_window in args
294+
286295
with contextlib.suppress(Exception):
287296
window.destroy()

0 commit comments

Comments
 (0)