microsoft / microsoft/microsoft-ui-reactor

[Bug] ContentDialog.IsOpen and Flyout.IsOpen have no falling edge — declaring IsOpen = false never closes them

Open
#948 0 comments 0 reactions 0 assignees View on GitHub
bug needs-triage
Dominant language
C#
Stars
646
Forks
54
Avg merge
1d 3h
Merged PRs (30d)
84

Description

### What happened?

`ContentDialogElement.IsOpen` and `FlyoutElement.IsOpen` are **rising-edge only**. Declaring `IsOpen = false` on an open dialog or flyout does nothing — Reactor never closes it.

Both update paths handle only the `false → true` transition and have no falling-edge arm:

```csharp
// src/Reactor/Core/V1Protocol/OverlayLifecycle.cs:48 (ContentDialog)
if (n.IsOpen && !o.IsOpen) ShowContentDialog(reconciler, n, fe, requestRerender);
// …no `else if (!n.IsOpen && o.IsOpen)` — nothing ever hides it.

// src/Reactor/Core/V1Protocol/OverlayLifecycle.cs:215 (Flyout)
if (n.IsOpen && !o.IsOpen) WinPrim.FlyoutBase.ShowAttachedFlyout(targetFe);
```

**Expected:** a declared `true → false` transition closes the overlay, mirroring the `false → true` transition that opens it.

**Actual:** the transition is silently ignored. The only way to close is the user dismissing it, or the overlay's own buttons.

### Why this is a defect and not the intended design

PR #940 documents the contract for a declared `bool` that the native control can also mutate: it is **edge-triggered** — the element declares a *transition*, not a mirror, and Reactor writes the control when the declared value *changes*. Four of the eight such properties implement that symmetrically in both directions:

| Property | Rising edge | Falling edge |
|---|---|---|
| `InfoBar.IsOpen` | ✅ | ✅ |
| `TeachingTip.IsOpen` | ✅ | ✅ |
| `SplitView.IsPaneOpen` | ✅ | ✅ |
| `NavigationView.IsPaneOpen` | ✅ | ✅ |
| **`ContentDialog.IsOpen`** | ✅ | ❌ **missing** |
| **`Flyout.IsOpen`** | ✅ | ❌ **missing** |

So `ContentDialog` / `Flyout` are a **degenerate form of the majority rule**, not a third design. That framing matters: this is a gap to close, not a semantic to preserve.

### Steps to reproduce

```csharp
var (showDialog, setShowDialog) = UseState(false);

return VStack(
Button("Open", () => setShowDialog(true)), // works — rising edge
Button("Close", () => setShowDialog(false)), // no-op — falling edge is not handled
ContentDialog("Title", TextBlock("Body"), "OK") with { IsOpen = showDialog });
```

1. Click **Open** — the dialog appears.
2. Click **Close** — nothing happens; the dialog stays open.

Same shape reproduces for `Flyout` with `IsOpen` bound to state.

### Notes for whoever picks this up

- The fix is a falling-edge arm on each update path — `ContentDialog` via `Hide()`, `Flyout` via `FlyoutBase.GetAttachedFlyout(target)?.Hide()`.
- **Watch for an echo loop.** Both overlays raise `Closed` on hide, and both elements expose `OnClosed`. A user-dismissal already fires `OnClosed`; a programmatic hide must not double-fire or re-enter. See spec-047 §8.3 and the `ChangeEchoSuppressor` / value-diff machinery.
- Guard the write on the control actually being open, so a redundant falling edge on an already-closed overlay is a silent no-op rather than a spurious event.
- Worth a selftest fixture in `tests/Reactor.AppTests.Host/SelfTest/Fixtures/` in the same shape as `IsOpenEdgeTriggeredFixtures` (added in #940), which pins both edges for `InfoBar`.

Found while auditing native-mutable declared bools for #940. Reported, not fixed there, because it is a behaviour change to two elements and deserves its own tests and revert-proof.

### Reactor version / commit

184bf5cdd79b807f21d69297e4b8a90ea7356d64

### Platform

x64

### .NET SDK version

10.0.302

### Windows version

Windows 11 Enterprise Insider Preview build 26310

### Windows App SDK version

2.1.3 (WinUI 2.1.0)

### Logs / stack trace

N/A — no exception. The write is simply never issued.

### Confirmation

- [x] I have searched existing issues and this isn't a duplicate.
- [x] This bug reproduces against the current `main` branch.

Contributor guide

Open the contributing guide

Research direction

Start in src/Reactor/Core/V1Protocol/OverlayLifecycle.cs at the ContentDialog path around line 48 and the Flyout path around line 215, then read the ChangeEchoSuppressor and value-diff machinery referenced in the issue. Compare the behavior with IsOpenEdgeTriggeredFixtures in tests/Reactor.AppTests.Host/SelfTest/Fixtures/. Done means both overlays respond to declared true-to-false transitions without duplicate OnClosed events, while redundant closes remain no-ops.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
frontend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.