Fix window moving off-screen issue by enforcing screen bounds by Supan-Roy · Pull Request #119 · erez-c137/NetSpeedTray

Supan-Roy · 2026-03-27T13:24:19Z

Problem

The Settings dialog could be moved off-screen, especially above the top edge, which hides the title bar and makes window controls difficult to access.

Solution

Implemented position clamping for both the tray widget and the Settings dialog:

Added continuous bounds enforcement for widget position monitoring
Added dialog-level clamping on:
- restore
- resize/reposition
- show
- move events
Enforced a hard top bound (y >= 0) to ensure the title bar always remains visible and accessible

Result

Windows remain fully visible on-screen
Title bar controls (minimize, maximize, close) are always reachable
Improved resilience against off-screen placement after dragging or restoring saved positions

Testing

Added/updated unit tests for:

Top-edge clamping behavior
Skip-clamping while dragging

All related positioning and settings tests pass locally

Copilot

Pull request overview

This PR aims to prevent the tray widget and Settings dialog from becoming inaccessible by enforcing on-screen bounds and adding tests to validate the behavior.

Changes:

Added _clamp_dialog_to_screen() to the Settings dialog and invoked it on restore, reposition/resize logic, show, and move events.
Added continuous window-bounds enforcement in PositionManager via a new timer and clamping helper methods.
Added/updated unit tests to cover top-edge clamping and “skip while dragging” behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File	Description
`src/netspeedtray/views/settings/dialog.py`	Adds dialog-level clamping logic and hooks clamping into key lifecycle/events.
`src/netspeedtray/core/position_manager.py`	Introduces a periodic bounds-enforcement timer and clamping utilities for the tray widget.
`src/netspeedtray/tests/unit/test_settings.py`	Adds a unit test asserting the Settings dialog top-edge clamp behavior.
`src/netspeedtray/tests/unit/test_position_manager.py`	Adds unit tests for bounds enforcement and skipping enforcement while dragging.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-27T13:29:14Z

+        # Hard top clamp: never allow title bar above the top edge.
+        min_y = max(0, available.top())


min_y = max(0, available.top()) will mis-handle multi-monitor layouts where the chosen screen has a negative top coordinate (monitor placed above the primary). In that case a dialog can be fully visible with y < 0, but this clamp forces it to y >= 0 and may move it onto the wrong screen. Prefer clamping to available.top() (and/or to a small margin within that screen) rather than forcing a global 0-based coordinate system.

Suggested change

# Hard top clamp: never allow title bar above the top edge.

min_y = max(0, available.top())

# Hard top clamp: never allow title bar above the top edge of this screen.

min_y = available.top()

Copilot · 2026-03-27T13:29:15Z

+
+        self.manager._enforce_window_bounds()
+
+        self.mock_widget.move.assert_called_with(120, 0)


This test hard-codes that clamping should force y to 0, which isn't true on multi-monitor setups where the active screen's top is negative (monitor above primary). Once clamping is based on the target screen's geometry/availableGeometry, this assertion should be updated to compare against that screen's top instead of a global 0.

Copilot · 2026-03-27T13:29:15Z

+    settings_dialog.move(120, -200)
+    settings_dialog._clamp_dialog_to_screen()
+
+    assert settings_dialog.pos().y() >= 0


This assertion assumes the top of the usable desktop is always y >= 0, which fails on multi-monitor configurations where the dialog is shown on a screen with a negative availableGeometry().top() (monitor positioned above the primary). Consider asserting against the active screen's top bound (or mocking screen geometry in the test) rather than hard-coding 0.

Suggested change

assert settings_dialog.pos().y() >= 0

# Determine the top edge of the active screen's available geometry

window_handle = settings_dialog.windowHandle()

screen = window_handle.screen() if window_handle is not None else QApplication.primaryScreen()

top_bound = screen.availableGeometry().top() if screen is not None else 0

assert settings_dialog.pos().y() >= top_bound

Copilot · 2026-03-27T13:29:15Z

+        # Keep the title bar reachable on Windows even if monitor coordinates are odd.
+        return ScreenPosition(validated.x, max(0, validated.y))


Hard-clamping Y with max(0, validated.y) will break multi-monitor setups where a valid on-screen position has a negative Y coordinate (e.g., a monitor placed above the primary). ScreenUtils.validate_position() already clamps to screen.geometry().top(), so this extra clamp is both redundant on single-monitor and a functional regression on multi-monitor. Prefer returning ScreenPosition(validated.x, validated.y) (or clamp against the chosen screen's top) instead of forcing >= 0.

Suggested change

# Keep the title bar reachable on Windows even if monitor coordinates are odd.

return ScreenPosition(validated.x, max(0, validated.y))

# Use the validated position directly; ScreenUtils already clamps to the screen's geometry.

return ScreenPosition(validated.x, validated.y)

Copilot · 2026-03-27T13:29:15Z

+        if not self._window_bounds_timer.isActive():
+            # Keep the widget reachable even if external factors move it off-screen.
+            self._window_bounds_timer.start(500)


Starting a new bounds-enforcement timer at 500ms will wake the event loop 2x/second for the lifetime of the app while the widget is visible. This can have unnecessary CPU/battery impact; consider a longer interval (e.g., a few seconds), making it configurable, and/or only enabling it when free-move is enabled or when an out-of-bounds condition has been detected.

Copilot · 2026-03-27T13:29:16Z

    def start_monitoring(self) -> None:
        """Starts the periodic tray watcher."""
        if not self._tray_watcher_timer.isActive():


start_monitoring now starts both the tray watcher and the new window-bounds timer, but the docstring still says it only starts the tray watcher. Update the docstring to reflect the actual behavior so callers understand what monitoring is being enabled.

Copilot · 2026-03-27T13:29:16Z

    def stop_monitoring(self) -> None:
        """Stops the periodic tray watcher."""
        self._tray_watcher_timer.stop()


stop_monitoring now stops both the tray watcher and the window-bounds timer, but the docstring still describes only the tray watcher. Update the docstring to avoid misleading future changes/callers.

erez-c137 · 2026-05-20T06:43:11Z

Hi @Supan-Roy — thanks for the contribution, and apologies for the delay reviewing this.

Closing because the branch has drifted too far from main to recover, and a current re-review against main shows the proposed fix is no longer needed in the form it was written. Details so future contributions don't hit the same wall:

Branch drift
The diff is now 1956 insertions / 5114 deletions. It would erase v1.3.1, which shipped substantial work: the update checker, RDP isolation utils, hardware monitoring settings page + tests, a smaller app icon, DPI scaling for the widget-size clamp (commit 74161b8, the fix for #114), and more.

Bugs in the position_manager.py changes
Two regressions slipped in during the apparent AI-assisted rebase:

position_manager.py:316 uses DEFAULT_TRAY_OFFSET_X as the fallback for tray_offset_y — typo.
The DPI-scaled width/height cap is reverted to a raw pixel value — would regress Make the display space wider in High DPI? #114 on high-DPI displays.

The settings/dialog.py changes import NetworkController (current main has StatsController) and HardwarePage (which this same PR deletes), so it won't even import.

The stated bug is already mostly handled on main
Current views/settings/dialog.py has:

Centering on availableGeometry() at init (dialog.py:113-121)
Saved-position clamping on restore (dialog.py:268-269)
Auto-reposition on layout changes via _adjust_size_and_reposition() (dialog.py:313-323)

The only remaining gap (clamping while the user actively drags the dialog above the top edge) is recoverable (Alt+F4 + reopen heals it via the restore-time clamp) and isn't referenced by any open issue.

Where help is actually wanted
The real user pain right now is in main-widget positioning on multi-monitor setups:

In two monitr setup it take resolution from second and move gadget on first #138 — widget appears on wrong monitor / wrong resolution
Misbehaiour when used on second screen #133 — free_move position not retained across sessions on multi-monitor
tray window is cut from top and bottom #137 — widget cut top/bottom with third-party taskbar modifiers

These touch core/position_manager.py and the free_move save/restore path. If you'd like to take a swing at one, please branch fresh from current main and reference the issue number — happy to review.

Thanks again for getting involved with the project.

Fix window moving off-screen issue by enforcing screen bounds

ab897ce

Copilot AI review requested due to automatic review settings March 27, 2026 13:24

Copilot started reviewing on behalf of Supan-Roy March 27, 2026 13:24 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

erez-c137 closed this May 20, 2026

erez-c137 mentioned this pull request May 20, 2026

feat: Add customizable arrow-to-speed spacing control #120

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix window moving off-screen issue by enforcing screen bounds#119

Fix window moving off-screen issue by enforcing screen bounds#119
Supan-Roy wants to merge 1 commit into
erez-c137:mainfrom
Supan-Roy:fix/window-offscreen

Supan-Roy commented Mar 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

erez-c137 commented May 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		# Hard top clamp: never allow title bar above the top edge.
		min_y = max(0, available.top())


		self.manager._enforce_window_bounds()

		self.mock_widget.move.assert_called_with(120, 0)

-    assert settings_dialog.pos().y() >= 0
+    # Determine the top edge of the active screen's available geometry
+    window_handle = settings_dialog.windowHandle()
+    screen = window_handle.screen() if window_handle is not None else QApplication.primaryScreen()
+    top_bound = screen.availableGeometry().top() if screen is not None else 0
+    assert settings_dialog.pos().y() >= top_bound

		# Keep the title bar reachable on Windows even if monitor coordinates are odd.
		return ScreenPosition(validated.x, max(0, validated.y))

Uh oh!

Conversation

Supan-Roy commented Mar 27, 2026

Problem

Solution

Result

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

erez-c137 commented May 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants