feat: add refresh displays tray context menu option - #34
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughMonitor refreshes are no longer triggered by window visibility changes. A new tray command explicitly shows the main window and invokes ChangesDisplay refresh control
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant TrayService
participant MainWindow
participant MainPage
User->>TrayService: Select Refresh displays
TrayService->>MainWindow: Show and foreground window
TrayService->>MainWindow: ForceRefreshMonitors()
MainWindow->>MainPage: RefreshMonitors()
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
MainWindow.xaml.cs (1)
74-81: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the now-empty visibility handler.
After removing the refresh call, this handler only evaluates conditions and enters an empty block. Remove it and its
AppWindow.Changedsubscription unless another visibility behavior is intended. As per coding guidelines, prefer readable code and keep methods small and focused.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@MainWindow.xaml.cs` around lines 74 - 81, Remove the now-empty AppWindow_Changed handler and its AppWindow.Changed subscription; also remove any related unused wiring or references, preserving other window behavior unchanged.Source: Coding guidelines
Services/TrayService.cs (1)
94-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the existing show/foreground flow.
The refresh command duplicates the same
AppWindow.Show()andSetForegroundWindow()logic used by “Show Window.” Extract a small helper and call it from both commands, then perform the monitor refresh only in the1004branch. As per coding guidelines, avoid duplicate logic and preserve the existing architecture.Proposed refactor
+private void ShowWindow() +{ + _window.AppWindow.Show(); + Win32.SetForegroundWindow(_hWnd); +} if (selectedId == 1001) { - _window.AppWindow.Show(); - Win32.SetForegroundWindow(_hWnd); + ShowWindow(); } else if (selectedId == 1004) { if (_window is MainWindow mainWindow) { - _window.AppWindow.Show(); - Win32.SetForegroundWindow(_hWnd); + ShowWindow(); mainWindow.ForceRefreshMonitors(); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Services/TrayService.cs` around lines 94 - 102, Extract the shared window display and foreground logic from the “Show Window” and selectedId 1004 branches into a small helper, then call that helper from both commands. Keep mainWindow.ForceRefreshMonitors() exclusively in the 1004 refresh branch.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@MainWindow.xaml.cs`:
- Around line 74-81: Remove the now-empty AppWindow_Changed handler and its
AppWindow.Changed subscription; also remove any related unused wiring or
references, preserving other window behavior unchanged.
In `@Services/TrayService.cs`:
- Around line 94-102: Extract the shared window display and foreground logic
from the “Show Window” and selectedId 1004 branches into a small helper, then
call that helper from both commands. Keep mainWindow.ForceRefreshMonitors()
exclusively in the 1004 refresh branch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ac841715-6f6a-4912-b808-b35239d66682
📒 Files selected for processing (2)
MainWindow.xaml.csServices/TrayService.cs
1 participant
Removes automatic monitor refreshing on tray open to ensure instant UI rendering. Adds a manual 'Refresh displays' context menu option to resync with hardware when needed.
Summary by CodeRabbit
New Features
Bug Fixes