Fix message box close behavior

[DavidGBrett]

Should fix #4503

Tries to make the message box follow the official behavior described in https://learn.microsoft.com/en-us/dotnet/api/system.windows.messageboxbutton?view=windowsdesktop-10.0#remarks

Particularly regarding the Yes/No type
For that type the close button is to be disabled ( I also hide it), and the close by esc functionality is also disabled
Its not really supposed to have a way to close it without selecting yes or no.
Despite that in the edge case of a forced close (e.g. alt-f4) we still have to decide on a result
It's not 100% clear what the result should be in that case, e.g. Cancel or None
But given how many users of this message box make the mistake of treating !No as Yes, it is probably safer to return No here

---

Summary by cubic

Aligns MessageBoxEx close behavior with Windows. Hides the close button for Yes/No, fixes titlebar close handling, and sets a safe default result when closed indirectly.

Summary of changes

• Changed
  • Hide the close button for Yes/No via named TitleBar (TitleBar.ShowCloseButton = false).
  • Disable system caption buttons when using CustomWindowTitleBar (WindowChrome.UseAeroCaptionButtons = false) so the OS close isn’t clickable behind the custom bar.
  • Titlebar close now works correctly for supported types and maps to a sensible result; Esc is ignored for Yes/No, and for others falls back to the default result.
• Added
  • DefaultCloseResult to mirror System.Windows.MessageBox when closed indirectly: OK→OK, OKCancel/YesNoCancel→Cancel, YesNo→No, else→None.
  • Initialize _result to DefaultCloseResult before showing the dialog to ensure consistent outcomes.
• Removed
  • Manual _result assignments in Esc/close handlers; rely on DefaultCloseResult.
• Memory usage
  • No material impact; only a small saved flag and default result state.
• Security risks
  • None expected; reduces unintended close paths. Logs a warning if a Yes/No dialog somehow receives a close request.
• Unit tests
  • No new tests added.

Release Note
Message boxes now follow Windows behavior: the close button is hidden for Yes/No, and closing the window picks a reasonable default answer.

^{Written for commit 834ba11. Summary will update on new commits.}

[cubic-dev-ai]

1 issue found across 3 files

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[coderabbitai]

📝 Walkthrough

Walkthrough

MessageBoxEx now hides the title bar close button for YesNo dialogs and pre-computes a default close result used when the dialog closes without a button click. KeyEsc_OnPress and Button_Cancel were updated accordingly, and CustomWindowTitleBar disables OS Aero caption buttons on host windows.

Changes

MessageBoxEx close behavior

Layer / File(s)	Summary
Title bar naming and close button hiding Flow.Launcher/MessageBoxEx.xaml, Flow.Launcher/MessageBoxEx.xaml.cs	The title bar control is named TitleBar, and the constructor hides the close button for MessageBoxButton.YesNo dialogs.
Default close result computation and usage Flow.Launcher/MessageBoxEx.xaml.cs	A new DefaultCloseResult property computes results for closes without button clicks; Show(...) pre-sets _result from it, KeyEsc_OnPress closes via DialogResult = false relying on the preset result, and Button_Cancel logs a warning for the hidden close button on YesNo dialogs.
Aero caption button suppression Flow.Launcher/Resources/Controls/CustomWindowTitleBar.xaml.cs	AttachToHostWindow disables UseAeroCaptionButtons on the host window's WindowChrome to prevent OS caption buttons from appearing alongside custom controls.

Estimated code review effort: 2 (Simple) | ~15 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant MessageBoxEx
  participant DefaultCloseResult

  User->>MessageBoxEx: Show(...)
  MessageBoxEx->>DefaultCloseResult: compute based on MessageBoxButton
  DefaultCloseResult-->>MessageBoxEx: _result preset
  User->>MessageBoxEx: Press Escape or click close button
  alt YesNo dialog
    MessageBoxEx-->>User: return without closing
  else other button types
    MessageBoxEx->>MessageBoxEx: DialogResult = false, close
    MessageBoxEx-->>User: returns preset _result
  end

Loading

Possibly related PRs

• Flow-Launcher/Flow.Launcher#4501: Both PRs modify the dialog close path in MessageBoxEx.xaml.cs, including Button_Cancel/close-button handling, and share the CustomWindowTitleBar integration.

Suggested reviewers: onesounds, taooceros

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Title check	✅ Passed	The title clearly summarizes the main change: fixing message box close behavior.
Description check	✅ Passed	The description is directly related to the changes and explains the intended message box behavior fix.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix-message-box

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

Flow.Launcher/MessageBoxEx.xaml.cs (1)

59-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Redundant _result writes in SetButtonVisibilityFocusAndResult are now dead code.

SetButtonVisibilityFocusAndResult(button, defaultResult) at Line 59 sets _result per focus/defaultResult logic, but Line 63 unconditionally overwrites it with msgBox.DefaultCloseResult before ShowDialog() is even called. The intermediate _result = ... assignments inside SetButtonVisibilityFocusAndResult (e.g. lines 85, 93, 98, 107, 112, 120, 125, 130) never have any observable effect anymore. Functionally this is fine today since DefaultCloseResult correctly encapsulates the intended close-without-button-click semantics, but the dead writes are misleading for anyone maintaining this method later (they'll assume changing them affects the close result).

Consider removing the now-unused _result assignments from SetButtonVisibilityFocusAndResult, or renaming/refactoring so it's clear that method only controls visibility/focus and DefaultCloseResult is the sole source of truth for the pre-set result.

♻️ Example cleanup (illustrative)

                 case MessageBoxButton.OK:
                     msgBox.btnCancel.Visibility = Visibility.Collapsed;
                     msgBox.btnNo.Visibility = Visibility.Collapsed;
                     msgBox.btnYes.Visibility = Visibility.Collapsed;
                     msgBox.btnOk.Focus();
-                    _result = MessageBoxResult.OK;
                     break;

(repeat for other branches once confirmed the pre-ShowDialog value is always superseded by DefaultCloseResult)

Also applies to: 76-136

🤖 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 `@Flow.Launcher/MessageBoxEx.xaml.cs` around lines 59 - 64, The `_result`
writes inside `SetButtonVisibilityFocusAndResult` are dead code because
`MessageBoxEx` always overwrites `_result` with `msgBox.DefaultCloseResult`
before `ShowDialog()`; remove the redundant `_result` assignments in that method
and keep it focused on button visibility/focus only, or refactor its
name/signature to make that responsibility clear. Use
`SetButtonVisibilityFocusAndResult`, `_result`, and `DefaultCloseResult` as the
key symbols when updating the logic.

🤖 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 `@Flow.Launcher/MessageBoxEx.xaml.cs`:
- Around line 59-64: The `_result` writes inside
`SetButtonVisibilityFocusAndResult` are dead code because `MessageBoxEx` always
overwrites `_result` with `msgBox.DefaultCloseResult` before `ShowDialog()`;
remove the redundant `_result` assignments in that method and keep it focused on
button visibility/focus only, or refactor its name/signature to make that
responsibility clear. Use `SetButtonVisibilityFocusAndResult`, `_result`, and
`DefaultCloseResult` as the key symbols when updating the logic.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cf839378-d166-43be-be51-66a74fa1a582

📥 Commits

Reviewing files that changed from the base of the PR and between 6a98243 and 069d536.

📒 Files selected for processing (3)

• Flow.Launcher/MessageBoxEx.xaml
• Flow.Launcher/MessageBoxEx.xaml.cs
• Flow.Launcher/Resources/Controls/CustomWindowTitleBar.xaml.cs