microsoft / microsoft/microsoft-ui-reactor
[Bug] ContentDialog.IsOpen and Flyout.IsOpen have no falling edge — declaring IsOpen = false never closes them
- 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
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