feat(v3): add FullscreenButtonState to WebviewWindowOptions for API consistency#5224
Merged
Merged
Conversation
…onsistency Add FullscreenButtonState ButtonState to WebviewWindowOptions, matching the existing MinimiseButtonState, MaximiseButtonState, and CloseButtonState pattern. The old setFullscreenButtonEnabled(bool) has been replaced with setFullscreenButtonState(ButtonState) across all platform implementations. Changes: - Added FullscreenButtonState field to WebviewWindowOptions - Added SetFullscreenButtonState to Window interface - macOS: C helper now uses setButtonState for enable/disable/hidden - Windows: implemented using WS_MAXIMIZEBOX style flag - Linux/Android/iOS/Server: no-op stubs - Init-time button state initialization updated for darwin and windows Closes: #4967
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughReplaces the boolean fullscreen-button API with a tri-state ChangesFullscreen Button State and Server-mode stubs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
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 docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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.11.4)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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. Review rate limit: 3/8 reviews remaining, refill in 34 minutes and 10 seconds.Comment |
Member
Author
|
🤖 PR Triage Review ✅ Accepted Adds FullscreenButtonState to WebviewWindowOptions for API consistency (v3). Improves API ergonomics. Platform: All (v3) Next Steps: Dispatching for testing on all platforms. Reviewed by Wails PR Reviewer Bot |
…ilation error The setFullscreenButtonState function called setButtonState before its static definition, causing ISO C99 implicit function declaration errors on macOS builds. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
Member
Author
|
Beta Release Manager review PR is clean — no outstanding review comments. Found and fixed one C compilation error before marking ready:
The fix has been pushed to this branch. PR is now marked ready for review. @leaanthony — please approve and merge when CI passes. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a FullscreenButtonState option and corresponding window APIs to make fullscreen button control consistent with the existing minimise/maximise/close button state APIs across v3 window implementations.
Changes:
- Adds
FullscreenButtonState ButtonStatetoWebviewWindowOptionsand a newWindow.SetFullscreenButtonState(ButtonState)API. - Replaces the internal fullscreen enable/disable hook with
setFullscreenButtonState(ButtonState)across platform implementations (real implementations on macOS + Windows; stubs elsewhere). - Adds unit tests asserting the new options field exists and validating
ButtonStateconstant values.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| v3/pkg/application/window.go | Extends the public Window interface with SetFullscreenButtonState. |
| v3/pkg/application/webview_window.go | Adds the public WebviewWindow.SetFullscreenButtonState and internal impl hook. |
| v3/pkg/application/webview_window_options.go | Adds FullscreenButtonState to WebviewWindowOptions. |
| v3/pkg/application/webview_window_darwin.go | Implements fullscreen button state via shared setButtonState and applies it during window init. |
| v3/pkg/application/webview_window_windows.go | Implements fullscreen button state via Win32 style flags and applies it during window init. |
| v3/pkg/application/webview_window_linux.go | Updates Linux stub to satisfy the new internal interface. |
| v3/pkg/application/webview_window_android.go | Updates Android stub to satisfy the new internal interface. |
| v3/pkg/application/webview_window_ios.go | Updates iOS stub to satisfy the new internal interface. |
| v3/pkg/application/application_server.go | Adds server-mode no-op stub for the new internal method (plus formatting changes). |
| v3/pkg/application/browser_window.go | Adds a no-op SetFullscreenButtonState to keep BrowserWindow implementing Window. |
| v3/pkg/application/webview_window_options_test.go | Adds tests for the new option field and ButtonState values. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
v3/pkg/application/webview_window_linux.go (1)
96-98: 💤 Low value
setFullscreenButtonStateis misplaced among legacy dead-code stubs.The stub is sandwiched between
setCloseButtonEnabled(bool)(line 92) andsetMinimiseButtonEnabled(bool)(line 100), which are old commented-out methods that are not part of any interface. The corresponding no-opsetMinimiseButtonState/setMaximiseButtonState/setCloseButtonStatestubs live at lines 443–450, where this method belongs.♻️ Suggested placement
Move the stub to be adjacent to the other
ButtonState-based stubs at lines 443–450:// SetMinimiseButtonState is unsupported on Linux func (w *linuxWebviewWindow) setMinimiseButtonState(state ButtonState) {} // SetMaximiseButtonState is unsupported on Linux func (w *linuxWebviewWindow) setMaximiseButtonState(state ButtonState) {} // SetCloseButtonState is unsupported on Linux func (w *linuxWebviewWindow) setCloseButtonState(state ButtonState) {} +// SetFullscreenButtonState is unsupported on Linux +func (w *linuxWebviewWindow) setFullscreenButtonState(state ButtonState) {}And remove the stub from line 96–98.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@v3/pkg/application/webview_window_linux.go` around lines 96 - 98, The stub function setFullscreenButtonState is misplaced among legacy commented-out methods; relocate its no-op implementation to be adjacent to the other ButtonState-based stubs (setMinimiseButtonState, setMaximiseButtonState, setCloseButtonState) so all ButtonState handlers are grouped together, and remove the duplicate/misplaced stub currently sitting between setCloseButtonEnabled and setMinimiseButtonEnabled.v3/pkg/application/webview_window_darwin.go (1)
467-467: 💤 Low valueStale comment: "disable window fullscreen button".
The function now sets the button to any
ButtonState(enabled / disabled / hidden), not just disables it.📝 Proposed fix
-// disable window fullscreen button -void setFullscreenButtonState(void* nsWindow, int state) { +// setFullscreenButtonState sets the fullscreen (zoom) button state +// 0 = enabled, 1 = disabled, 2 = hidden +static void setFullscreenButtonState(void* nsWindow, int state) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@v3/pkg/application/webview_window_darwin.go` at line 467, The inline comment "disable window fullscreen button" is stale—update the comment near where ButtonState is applied (the comment containing that exact string) to reflect that the code sets the window fullscreen button's state (enabled/disabled/hidden) rather than only disabling it; mention the ButtonState enum being used so readers know it can set enabled, disabled, or hidden.v3/pkg/application/webview_window_ios.go (1)
158-158: 💤 Low valueMissing explanatory comment on the
setFullscreenButtonStatestub.Every sibling stub (
setMinimiseButtonState,setMaximiseButtonState,setCloseButtonState) includes a comment explaining why it's a no-op on iOS, but this new stub does not.📝 Proposed fix
-func (w *iosWebviewWindow) setFullscreenButtonState(_ ButtonState) {} +func (w *iosWebviewWindow) setFullscreenButtonState(_ ButtonState) { + // iOS doesn't have a fullscreen button like desktop platforms +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@v3/pkg/application/webview_window_ios.go` at line 158, Add a short explanatory comment above the setFullscreenButtonState stub on iosWebviewWindow explaining it is intentionally a no-op on iOS (matching the style of setMinimiseButtonState, setMaximiseButtonState, setCloseButtonState stubs), e.g., note that iOS does not expose a window fullscreen control so the method is left empty to satisfy the interface.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@v3/pkg/application/webview_window_darwin.go`:
- Around line 464-471: The function setFullscreenButtonState has external
linkage while its sibling helpers (setButtonState, setMinimiseButtonState,
setMaximiseButtonState, setCloseButtonState) are static; change
setFullscreenButtonState to be static so it has internal linkage like the others
to avoid symbol collisions — locate the setFullscreenButtonState definition and
add the static qualifier to its declaration/definition to match setButtonState
and the other helper functions.
- Around line 1393-1396: The new unconditional call to
w.setFullscreenButtonState(options.FullscreenButtonState) causes the zero value
(ButtonEnabled) to override a previously set MaximiseButtonState; only call
w.setFullscreenButtonState when options.FullscreenButtonState is explicitly
non-default (i.e., != ButtonEnabled/zero) so MaximiseButtonState remains
authoritative when FullscreenButtonState is unset, and add static to the C
helper function setFullscreenButtonState (matching other internal helpers) and
update its comment to reflect it can enable/disable/hide based on the state
parameter.
In `@v3/pkg/application/webview_window_windows.go`:
- Around line 2453-2461: The two methods setFullscreenButtonState and
setMaximiseButtonState both toggle the same WS_MAXIMIZEBOX bit, causing
setFullscreenButtonState (called after setMaximiseButtonState in run) to
re-enable the maximize box when FullscreenButtonState is the default
ButtonEnabled; fix by preventing setFullscreenButtonState from altering
WS_MAXIMIZEBOX when state == ButtonEnabled (i.e., only modify WS_MAXIMIZEBOX in
setFullscreenButtonState when state != ButtonEnabled), or alternately change run
to call setFullscreenButtonState only when FullscreenButtonState is explicitly
non-default; reference functions: setFullscreenButtonState,
setMaximiseButtonState, run, and symbol WS_MAXIMIZEBOX.
---
Nitpick comments:
In `@v3/pkg/application/webview_window_darwin.go`:
- Line 467: The inline comment "disable window fullscreen button" is
stale—update the comment near where ButtonState is applied (the comment
containing that exact string) to reflect that the code sets the window
fullscreen button's state (enabled/disabled/hidden) rather than only disabling
it; mention the ButtonState enum being used so readers know it can set enabled,
disabled, or hidden.
In `@v3/pkg/application/webview_window_ios.go`:
- Line 158: Add a short explanatory comment above the setFullscreenButtonState
stub on iosWebviewWindow explaining it is intentionally a no-op on iOS (matching
the style of setMinimiseButtonState, setMaximiseButtonState, setCloseButtonState
stubs), e.g., note that iOS does not expose a window fullscreen control so the
method is left empty to satisfy the interface.
In `@v3/pkg/application/webview_window_linux.go`:
- Around line 96-98: The stub function setFullscreenButtonState is misplaced
among legacy commented-out methods; relocate its no-op implementation to be
adjacent to the other ButtonState-based stubs (setMinimiseButtonState,
setMaximiseButtonState, setCloseButtonState) so all ButtonState handlers are
grouped together, and remove the duplicate/misplaced stub currently sitting
between setCloseButtonEnabled and setMinimiseButtonEnabled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7b109c2b-4feb-4960-a411-f96755173a6c
📒 Files selected for processing (11)
v3/pkg/application/application_server.gov3/pkg/application/browser_window.gov3/pkg/application/webview_window.gov3/pkg/application/webview_window_android.gov3/pkg/application/webview_window_darwin.gov3/pkg/application/webview_window_ios.gov3/pkg/application/webview_window_linux.gov3/pkg/application/webview_window_options.gov3/pkg/application/webview_window_options_test.gov3/pkg/application/webview_window_windows.gov3/pkg/application/window.go
- Tests: convert TestFullscreenButtonState_OptionExists to table-driven - Windows: remove WS_SYSMENU side-effect from setFullscreenButtonState; WS_SYSMENU is owned by setCloseButtonState to avoid cross-method interference when enabling fullscreen re-shows the close/system menu - Windows/macOS: skip setFullscreenButtonState at init when value is the default ButtonEnabled to prevent the zero-value from overriding an explicit MaximiseButtonState setting (both use the same underlying control on each platform) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@v3/pkg/application/webview_window_windows.go`:
- Around line 2455-2461: In setFullscreenButtonState, when handling the
ButtonEnabled case restore the WS_SYSMENU flag along with WS_MAXIMIZEBOX (i.e.,
call w.setStyle(true, w32.WS_MAXIMIZEBOX|w32.WS_SYSMENU)) so the maximize
control is visible if the system menu was previously removed (e.g., by
setCloseButtonState(ButtonHidden)); follow the same pattern used in
setMaximiseButtonState to ensure consistent state management with w.setStyle.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7861089e-555d-4a9c-94eb-f580a564157b
📒 Files selected for processing (3)
v3/pkg/application/webview_window_darwin.gov3/pkg/application/webview_window_options_test.gov3/pkg/application/webview_window_windows.go
🚧 Files skipped from review as they are similar to previous changes (1)
- v3/pkg/application/webview_window_options_test.go
- darwin: add static qualifier to setFullscreenButtonState C helper, consistent with all other internal button helpers (setButtonState, setMinimiseButtonState, setMaximiseButtonState, setCloseButtonState) - darwin: update stale comment "disable window fullscreen button" to reflect the function enables, disables, or hides based on state - windows: restore WS_SYSMENU alongside WS_MAXIMIZEBOX in ButtonEnabled case; WS_MAXIMIZEBOX requires WS_SYSMENU per Win32 API spec and the init-order regression is already prevented by the skip-when-default guard Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
- linux: relocate setFullscreenButtonState stub to be adjacent to the other ButtonState-based stubs (setMinimiseButtonState, setMaximiseButtonState, setCloseButtonState) with a matching comment, removing it from among the legacy dead-code setXxxButtonEnabled methods - ios: move setFullscreenButtonState stub to be adjacent to the other ButtonState stubs and add an explanatory comment consistent with the sibling setMinimiseButtonState/setMaximiseButtonState/setCloseButtonState stubs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
Member
Author
|
Addressed the remaining CodeRabbit nitpick comments (commit e00b157): Linux ( iOS ( All actionable CodeRabbit and Copilot comments are now resolved. CI is green across all platforms. PR is ready for approval and merge. |
leaanthony
pushed a commit
to jes0131/wails
that referenced
this pull request
May 3, 2026
… FullscreenButtonState to WebviewWindowOptions for API consistency
This was referenced May 3, 2026
This was referenced May 15, 2026
Closed
leaanthony
added a commit
that referenced
this pull request
May 16, 2026
Resolves conflicts caused by #5224 (FullscreenButtonState landed on master) while preserving the macOS NSWindowZoomButton override fix from this PR: - run() applies max(MaximiseButtonState, FullscreenButtonState) once, instead of the "skip if default" workaround from master. - macOS keeps a single setFullscreenButtonState (deduped against master's copy); behavior is unchanged since both C helpers target NSWindowZoomButton. - Windows: setFullscreenButtonState is a no-op (no dedicated fullscreen button in the standard title bar). The dead "if != ButtonEnabled" guard in Windows run() has been removed. - iOS / Android: deduped against master's existing stubs. Docs: customising-windows.mdx now documents FullscreenButtonState, the runtime setter, the macOS shared-button semantics, and the Windows no-op.
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
FullscreenButtonState ButtonStatetoWebviewWindowOptionsfor API consistency withMinimiseButtonState,MaximiseButtonState, andCloseButtonState. The old internalsetFullscreenButtonEnabled(bool)method has been replaced withsetFullscreenButtonState(ButtonState)across all platform implementations.Changes
WebviewWindowOptions— addedFullscreenButtonState ButtonStatefieldWindowinterface — addedSetFullscreenButtonState(state ButtonState) WindowsetButtonState()for enable/disable/hidden supportWS_MAXIMIZEBOXstyle flag (reuse the maximize button since that's what fullscreen zoom uses on Windows)setFullscreenButtonStatealongside the other button state initializationsTest
TestFullscreenButtonState_OptionExistsandTestFullscreenButtonState_ButtonStateValuesGOOS=linux go build ./pkg/application/compiles successfullyGOOS=linux go test ./pkg/application/ -run "TestFullscreenButton"passesCloses #4967
Notes for reviewer
Summary by CodeRabbit
New Features
Refactor
Tests