Fix: MainWindow not fitting on small screens with high DPI scaling#1380
Fix: MainWindow not fitting on small screens with high DPI scaling#1380Maditer wants to merge 9 commits intoValkirie:mainfrom
Conversation
…, the logical resolution is 960×600. The window's MinWidth/MinHeight was 1280×720, making it impossible to display fully. Lowered minimums to 800×450 and added screen-size clamping at startup.
📝 WalkthroughWalkthroughThe changes adjust the main window's default and minimum dimensions in XAML, and introduce screen-aware positioning logic in the code-behind. The code-behind now computes screen bounds from the primary display and constrains window dimensions to fit within those bounds, accounting for a 40-pixel taskbar allowance. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@HandheldCompanion/Views/Windows/MainWindow.xaml.cs`:
- Around line 232-233: The window size is always overwritten by persisted
settings (Height/Width set from ManagerFactory.settingsManager.GetDouble) so the
new XAML defaults (1080x620) never apply on first run; fix by either updating
the backing defaults in the generated Settings (update the default
MainWindowHeight/MainWindowWidth values in Properties/Settings.Designer.cs to
1080 and 620) or change the MainWindow constructor logic to only call
ManagerFactory.settingsManager.GetDouble("MainWindowHeight"/"MainWindowWidth")
and assign to Height/Width when a persisted value exists (e.g., check for
presence/IsSet or a sentinel before overriding the XAML Height/Width).
- Around line 229-242: The code clamps window size/position against
SystemParameters.PrimaryScreenWidth/Height and forces non-negative Left/Top;
change this to locate the target monitor via Screen.AllScreens using the saved
MainWindowLeft/MainWindowTop (from
ManagerFactory.settingsManager.GetDouble("MainWindowLeft") / "MainWindowTop") or
default to Screen.PrimaryScreen, then use that monitor.WorkingArea (not a
hardcoded 40px) to clamp Width, Height, Left, and Top; ensure Width/Height are
min/maxed against WorkingArea.Width/Height and Left/Top are clamped between
WorkingArea.Left and WorkingArea.Right-Width (and similarly for Top) so
positions on monitors left/above primary are preserved (follow the same pattern
used in OverlayQuickTools.xaml.cs).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 00b920f2-ef7b-4a6d-8e77-d6831b20ae37
📒 Files selected for processing (2)
HandheldCompanion/Views/Windows/MainWindow.xamlHandheldCompanion/Views/Windows/MainWindow.xaml.cs
| double screenWidth = SystemParameters.PrimaryScreenWidth; | ||
| double screenHeight = SystemParameters.PrimaryScreenHeight; | ||
|
|
||
| Height = (int)Math.Max(MinHeight, ManagerFactory.settingsManager.GetDouble("MainWindowHeight")); | ||
| Width = (int)Math.Max(MinWidth, ManagerFactory.settingsManager.GetDouble("MainWindowWidth")); | ||
| Left = Math.Min(SystemParameters.PrimaryScreenWidth - MinWidth, ManagerFactory.settingsManager.GetDouble("MainWindowLeft")); | ||
| Top = Math.Min(SystemParameters.PrimaryScreenHeight - MinHeight, ManagerFactory.settingsManager.GetDouble("MainWindowTop")); | ||
|
|
||
| // Clamp window size to fit the available screen (critical for high-DPI small screens) | ||
| if (Width > screenWidth) | ||
| Width = screenWidth; | ||
| if (Height > screenHeight - 40) // leave room for taskbar | ||
| Height = screenHeight - 40; | ||
|
|
||
| Left = Math.Min(screenWidth - Width, Math.Max(0, ManagerFactory.settingsManager.GetDouble("MainWindowLeft"))); | ||
| Top = Math.Min(screenHeight - Height, Math.Max(0, ManagerFactory.settingsManager.GetDouble("MainWindowTop"))); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the MainWindow.xaml.cs file
find . -name "MainWindow.xaml.cs" -type f 2>/dev/nullRepository: Valkirie/HandheldCompanion
Length of output: 122
🏁 Script executed:
# Read the relevant section around lines 229-242
cat -n HandheldCompanion/Views/Windows/MainWindow.xaml.cs | sed -n '220,250p'Repository: Valkirie/HandheldCompanion
Length of output: 1654
🏁 Script executed:
# Check if there are any imports related to multi-monitor handling or screen positioning
rg -A 5 -B 5 "PrimaryScreenWidth|PrimaryScreenHeight|SystemParameters" HandheldCompanion/Views/Windows/MainWindow.xaml.cs | head -100Repository: Valkirie/HandheldCompanion
Length of output: 684
🏁 Script executed:
# Search for any existing multi-monitor or Screen class usage in the codebase
rg -i "Screen\.|GetMonitor|WorkingArea|DeviceScreen" --type csRepository: Valkirie/HandheldCompanion
Length of output: 16914
🏁 Script executed:
# Check if System.Windows.Forms is already imported in the project
rg "using System.Windows.Forms" HandheldCompanion/ --type csRepository: Valkirie/HandheldCompanion
Length of output: 1552
🏁 Script executed:
# Look for other window positioning code patterns in the codebase
rg -A 3 -B 3 "Left\s*=|Top\s*=|Width\s*=|Height\s*=" HandheldCompanion/Views/Windows/ --type cs | head -80Repository: Valkirie/HandheldCompanion
Length of output: 7065
🏁 Script executed:
# Check the WPF Window class to understand what Left/Top coordinates mean
# and verify if negative values are even valid in WPF
rg "Left|Top" HandheldCompanion/Views/Windows/MainWindow.xaml.cs --type cs | head -20Repository: Valkirie/HandheldCompanion
Length of output: 937
Clamp against the target monitor's working area, not the primary screen.
Lines 229-242 restrict positioning to SystemParameters.PrimaryScreenWidth/Height and force Left/Top to be non-negative. This breaks valid saved positions on secondary monitors arranged left or above the primary display. Additionally, the hardcoded 40 pixel margin for the taskbar is brittle and doesn't account for different taskbar sizes or positions.
Use Screen.AllScreens to locate the monitor where the window was previously positioned (or default to primary), then clamp against that monitor's WorkingArea. OverlayQuickTools.xaml.cs in the same codebase already demonstrates this pattern—use the same approach here to properly support multi-monitor configurations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@HandheldCompanion/Views/Windows/MainWindow.xaml.cs` around lines 229 - 242,
The code clamps window size/position against
SystemParameters.PrimaryScreenWidth/Height and forces non-negative Left/Top;
change this to locate the target monitor via Screen.AllScreens using the saved
MainWindowLeft/MainWindowTop (from
ManagerFactory.settingsManager.GetDouble("MainWindowLeft") / "MainWindowTop") or
default to Screen.PrimaryScreen, then use that monitor.WorkingArea (not a
hardcoded 40px) to clamp Width, Height, Left, and Top; ensure Width/Height are
min/maxed against WorkingArea.Width/Height and Left/Top are clamped between
WorkingArea.Left and WorkingArea.Right-Width (and similarly for Top) so
positions on monitors left/above primary are preserved (follow the same pattern
used in OverlayQuickTools.xaml.cs).
| Height = (int)Math.Max(MinHeight, ManagerFactory.settingsManager.GetDouble("MainWindowHeight")); | ||
| Width = (int)Math.Max(MinWidth, ManagerFactory.settingsManager.GetDouble("MainWindowWidth")); |
There was a problem hiding this comment.
The new 1080x620 default won’t apply on first launch.
Lines 232-233 always replace the XAML size with settings values, and HandheldCompanion/Properties/Settings.Designer.cs still defaults those settings to 1280x720. On a clean profile, the window will therefore start from 1280x720 and only then be clamped, so the new XAML default size is never actually used. Please update the backing settings defaults as well, or only override the XAML size when a persisted value exists.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@HandheldCompanion/Views/Windows/MainWindow.xaml.cs` around lines 232 - 233,
The window size is always overwritten by persisted settings (Height/Width set
from ManagerFactory.settingsManager.GetDouble) so the new XAML defaults
(1080x620) never apply on first run; fix by either updating the backing defaults
in the generated Settings (update the default MainWindowHeight/MainWindowWidth
values in Properties/Settings.Designer.cs to 1080 and 620) or change the
MainWindow constructor logic to only call
ManagerFactory.settingsManager.GetDouble("MainWindowHeight"/"MainWindowWidth")
and assign to Height/Width when a persisted value exists (e.g., check for
presence/IsSet or a sentinel before overriding the XAML Height/Width).
Updated the workflow to use .NET 10 SDK and modified installer version handling. Removed release pull request and release creation steps.
b4468fc to
51759a7
Compare
Problem
On 8-inch handheld devices (e.g. MSI Claw A8) with 1920×1200 resolution at 200% DPI scaling, the logical screen resolution is only 960×600. The MainWindow had
MinWidth="1280"andMinHeight="720", making it impossible to display the full window — users could not see or interact with parts of the UI, and the window could not be resized smaller.Changes
MainWindow.xaml:
MinWidthfrom 1280 → 800MinHeightfrom 720 → 450Widthto 1080 andHeightto 620MainWindow.xaml.cs:
Testing
Tested on MSI Claw A8 (Ryzen Z2 Extreme) with 1920×1200 display at 200% scaling. The window now displays fully and is resizable. Content remains scrollable via the existing ScrollViewerEx.
Summary by CodeRabbit