Skip to content

fix(v3/darwin): runtime effectiveZoomButtonState reconciliation + changelog for #5319#5467

Merged
leaanthony merged 2 commits into
wailsapp:masterfrom
taliesin-ai:agent/engineer-mac/3fd103fd-v2
May 16, 2026
Merged

fix(v3/darwin): runtime effectiveZoomButtonState reconciliation + changelog for #5319#5467
leaanthony merged 2 commits into
wailsapp:masterfrom
taliesin-ai:agent/engineer-mac/3fd103fd-v2

Conversation

@taliesin-ai

@taliesin-ai taliesin-ai commented May 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

PR #5319 (merged 2026-05-16) fixed the startup path for the MaximiseButtonState / FullscreenButtonState NSWindowZoomButton conflict on macOS. This PR addresses the remaining Copilot review comments:

  • Runtime path fix: setMaximiseButtonState and setFullscreenButtonState now each call effectiveZoomButtonState(state, w.parent.options.<other>) before writing to NSWindowZoomButton. A runtime SetMaximiseButtonState or SetFullscreenButtonState call can no longer silently override the other state.
  • Helper function: effectiveZoomButtonState(a, b ButtonState) ButtonState — returns the more restrictive state (Hidden > Disabled > Enabled). Defined in webview_window_options.go for testability without a CGo dependency.
  • Tests: 13 table-driven subtests covering all ButtonState combinations.
  • Changelog: Adds the v3/UNRELEASED_CHANGELOG.md entry that the auto-changelog workflow failed to add after fix(v3/darwin): add FullscreenButtonState and resolve NSWindowZoomButton conflict #5319 merged.

Supersedes PR #5457 (conflicted with master after #5319 merged).

Verification

  • go vet ./v3/pkg/application/... — clean
  • TestEffectiveZoomButtonState — 13/13 PASS

CC @leaanthony

Summary by CodeRabbit

  • Bug Fixes
    • Fixed an issue on macOS where the Maximise and Fullscreen button states could override each other at startup and runtime. The more restrictive state is now properly applied to prevent conflicts.

Review Change Stack

…onState+FullscreenButtonState reconciliation

PR wailsapp#5319 fixed the startup path. This follow-up fixes the runtime path:
both setMaximiseButtonState and setFullscreenButtonState now call
effectiveZoomButtonState() before writing to NSWindowZoomButton, so a
runtime SetMaximiseButtonState/SetFullscreenButtonState call can no
longer silently override the other state.

Also adds v3/UNRELEASED_CHANGELOG.md entry for wailsapp#5319 (auto-changelog
workflow failed to add it automatically after the PR was merged) and
13 unit tests covering all ButtonState combinations.

go vet: clean
TestEffectiveZoomButtonState: 13/13 PASS
@coderabbitai

coderabbitai Bot commented May 16, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 39697a43-5e94-444c-8df7-9aab8f6573c0

📥 Commits

Reviewing files that changed from the base of the PR and between 1c2ec6a and 0030246.

📒 Files selected for processing (4)
  • v3/UNRELEASED_CHANGELOG.md
  • v3/pkg/application/webview_window_darwin.go
  • v3/pkg/application/webview_window_options.go
  • v3/pkg/application/webview_window_options_test.go

Walkthrough

This PR resolves a macOS-specific conflict where maximise and fullscreen button states both target the same native NSWindowZoomButton. It introduces a helper function to apply the more restrictive state, updates both button setters to use it, includes comprehensive tests, and documents the fix.

Changes

macOS Zoom Button State Conflict Resolution

Layer / File(s) Summary
Zoom button state resolution helper and tests
v3/pkg/application/webview_window_options.go, v3/pkg/application/webview_window_options_test.go
Introduces effectiveZoomButtonState(a, b ButtonState) helper that applies the more restrictive of two button states (Hidden > Disabled > Enabled ordering). Comprehensive table-driven tests verify all state combinations return the expected effective state.
Apply restrictive state to button setters
v3/pkg/application/webview_window_darwin.go
setFullscreenButtonState and setMaximiseButtonState now compute the applied button state via effectiveZoomButtonState using the other control's current state, with clarifying comments explaining both states target the shared NSWindowZoomButton and preventing silent override.
Changelog documentation
v3/UNRELEASED_CHANGELOG.md
Documents the macOS button state conflict fix in the unreleased changelog with reference to the related issue.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Possibly related PRs

  • wailsapp/wails#5319: Both PRs implement the same macOS NSWindowZoomButton conflict fix by introducing the effective state logic and updating button setters to apply the more restrictive state.
  • wailsapp/wails#5224: Related changes to macOS webview window options around fullscreen button state handling on the shared NSWindowZoomButton.

Suggested labels

MacOS, Documentation, size:M, Bug, reviewed ✅

Suggested reviewers

  • leaanthony

Poem

🐰 Two buttons vied for one control's embrace,
Until the helper brought them into place—
The stricter state now wins the day,
No silent override can have its way,
On macOS, harmony finds its space! 🪟✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The PR description covers the core changes, verification steps, and references to related PRs. However, the template's required sections (Type of change checklist, testing verification checklist, and test configuration) are not filled in. Complete the Type of change checkbox (Bug fix), check the testing platform boxes (macOS), and provide wails doctor output or environment details in the Test Configuration section.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main changes: runtime effectiveZoomButtonState reconciliation for the macOS zoom button conflict and the changelog entry for issue #5319.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.2)

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants