fix(v3/windows): address review issues in dark-mode menubar repaint (#5362)#5382
Conversation
…cking Address reviewer feedback on wailsapp#5362: - Replace inSizeMove bool + WM_ENTERSIZEMOVE/WM_EXITSIZEMOVE with a lastSizeWParam field that tracks WM_SIZE wParam transitions. This correctly handles keyboard snap (Win+Left/Right/Up) which bypasses WM_ENTERSIZEMOVE entirely. - Repaint fires only when the size STATE changes (SIZE_MAXIMIZED -> SIZE_RESTORED), not on every SIZE_RESTORED during live drag-resize. - Remove RDW_UPDATENOW from SIZE_RESTORED path; async invalidation (RDW_FRAME|RDW_INVALIDATE) is sufficient and avoids a synchronous non-client repaint on every resize step. - Apply the same lastSizeWParam approach in MenuBarTheme/MenuBarWndProc. Co-authored-by: multica-agent <github@multica.ai>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughTracks the previous WM_SIZE wParam in both MenuBarTheme and windowsWebviewWindow and changes WM_SIZE handling so menubar redraws occur only when wParam transitions between SIZE_MAXIMIZED and SIZE_RESTORED; lastSizeWParam is updated after each WM_SIZE handling. ChangesWindows Menubar Resize-Transition Gating
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.1)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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.
🧹 Nitpick comments (1)
v3/pkg/w32/menubar.go (1)
188-207: 💤 Low valuePer-window state inside a theme struct.
lastSizeWParamis per-window run-time state, butMenuBarThemeis otherwise pure styling/brush data. In the current code each window builds its ownMenuBarTheme, so this works, but the type’s shape no longer prevents two windows from sharing a single*MenuBarTheme. If they did, snap/restore transitions on one window would corrupt the transition gate on the other (both write to the samelastSizeWParam).Since
windowsWebviewWindowalready tracks the same value (w.lastSizeWParaminwebview_window_windows.go), one option is to drop the theme-side copy and pass the previous wParam in (or read it from the window) — keepingMenuBarThemepurely about appearance.🤖 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 `@v3/pkg/w32/menubar.go` around lines 188 - 207, MenuBarTheme currently contains runtime per-window state (lastSizeWParam) which can cause shared-theme corruption; remove the lastSizeWParam field from the MenuBarTheme struct and update all callers that used MenuBarTheme.lastSizeWParam to instead read the previous wParam from the window instance (windowsWebviewWindow.w.lastSizeWParam) or accept it as an explicit parameter; search for uses of MenuBarTheme.lastSizeWParam and refactor the relevant functions/methods to take a previousWPARAM argument or access the windowsWebviewWindow state so MenuBarTheme remains purely styling/brush data.
🤖 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 `@v3/pkg/w32/menubar.go`:
- Around line 188-207: MenuBarTheme currently contains runtime per-window state
(lastSizeWParam) which can cause shared-theme corruption; remove the
lastSizeWParam field from the MenuBarTheme struct and update all callers that
used MenuBarTheme.lastSizeWParam to instead read the previous wParam from the
window instance (windowsWebviewWindow.w.lastSizeWParam) or accept it as an
explicit parameter; search for uses of MenuBarTheme.lastSizeWParam and refactor
the relevant functions/methods to take a previousWPARAM argument or access the
windowsWebviewWindow state so MenuBarTheme remains purely styling/brush data.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0d41ce74-b277-4d87-88fb-67a929e9a3bb
📒 Files selected for processing (2)
v3/pkg/application/webview_window_windows.gov3/pkg/w32/menubar.go
…s): address review issues in dark-mode menubar repaint (wailsapp#5362)
Addresses reviewer feedback on #5362.
Problem
The original guard used
WM_ENTERSIZEMOVE/WM_EXITSIZEMOVEto suppressInvalidateRect/DrawMenuBarduring live drag-resize, but keyboard snap (Win+Left/Right/Up) bypasses those messages entirely -- the menubar was not repainting after a keyboard snap-restore sequence.Fix
Replaced the
inSizeMove boolflag with alastSizeWParamstate-change tracker in bothwindowsWebviewWindowandMenuBarTheme:RDW_UPDATENOWfrom the SIZE_RESTORED path -- async invalidation (RDW_FRAME|RDW_INVALIDATE) is sufficient and avoids a synchronous non-client repaint on every resize stepWM_ENTERSIZEMOVE/WM_EXITSIZEMOVEhandlers are removed; the state-change approach handles both mouse drag and keyboard snap uniformlyFiles changed
v3/pkg/application/webview_window_windows.go-- replaceinSizeMovewithlastSizeWParam; gate SIZE_RESTORED repaint on previous state being SIZE_MAXIMIZED; dropRDW_UPDATENOWv3/pkg/w32/menubar.go-- addlastSizeWParamtoMenuBarTheme; gateInvalidateRect/DrawMenuBaron wParam state changeCloses #5362
CC @leaanthony
Summary by CodeRabbit