edit_room_modal closes via an undeferred GlobalSignal write in onclick (dioxus-signal-safety violation)

[sanity]

Problem

ui/src/components/room_list/edit_room_modal.rs closes the room-details panel
by writing a GlobalSignal directly inside an onclick, with no
crate::util::defer():

• edit_room_modal.rs:66 — the backdrop overlay
• edit_room_modal.rs:375 — the ✕ close button

onclick: move |_| {
    EDIT_ROOM_MODAL.write().room = None;
},

.claude/rules/dioxus-signal-safety.md names this shape explicitly as wrong:

ALSO WRONG — onclick handlers trigger the same RefCell panic

onclick: move |_| {
    ROOMS.write().map.remove(&key);
};

The rationale is that a signal write's Drop fires subscriber notifications
synchronously, and Firefox mobile runs those during Drop, so a memo doing
try_read() on the same signal can hit a re-entrant RefCell already borrowed
panic. defer() also supplies the Dioxus runtime + root scope.

The leave-room path in the same file (line 333) already does this correctly,
inside a crate::util::defer() block — so this is an inconsistency within one
component, not a deliberate exemption.

Evidence / severity

No symptom reproduced. An instrumented 25-cycle open/copy/close/reopen loop
against both WebKit and Firefox (alternating close-button and backdrop routes)
produced no panic and no stale render. So this is filed as a latent
rule violation, not a known-live bug — severity low, correctness risk real
but unobserved.

Deliberately not fixed in #536 (the Firefox selection fix): making the close
deferred changes modal-close timing app-wide, which is a bigger blast radius
than that PR warrants without evidence of harm. Noting it here so it is not
silently lost.

Suggested fix

Wrap both writes:

onclick: move |_| {
    crate::util::defer(move || {
        EDIT_ROOM_MODAL.write().room = None;
    });
},

Then confirm copy-clipboard-feedback.spec.ts (which closes this class of
modal via the backdrop) and the room-details specs still pass — the close
becomes asynchronous, so any assertion that races it would surface there.

Worth grepping for the same shape in the other modals while in there.

[AI-assisted - Claude]

[sanity]

Issue Priority Assessment

Generated: 2026-07-29T16:06:39.750924295+00:00
Issue hash: 4777644c

Value Assessment

Confirmed: both undeferred EDIT_ROOM_MODAL.write() writes still exist exactly as described (lines ~66 and ~375). Issue is fully current, not obsolete.

Relevance: Fully confirmed against current code — both violations exist verbatim (backdrop onclick and ✕ button onclick), and the leave-room path in the same file still correctly uses defer() for comparison. Nothing has changed since filing.

User impact: Low and speculative. The issue itself states no reproduction after a 25-cycle stress test on WebKit and Firefox. This is a latent correctness risk (re-entrant RefCell panic on Firefox mobile per the documented pattern), not an observed bug affecting real users today.

Strategic alignment: Minor hygiene/consistency fix enforcing an existing documented safety rule (dioxus-signal-safety.md). It doesn't advance features or roadmap goals, but closing rule violations proactively prevents future intermittent, hard-to-debug WASM panics — consistent with the project's "flaky/undefined behavior is a real bug" philosophy.

Blocking factor: None — doesn't block other work; it's an isolated, self-contained two-line fix in a leaf UI component.

Urgency: Low. No active incident, no deadline. Worth batching into a small cleanup PR (ideally also grepping other modals for the same anti-pattern, as the issue suggests) rather than treating as time-sensitive.

Overall: a valid, low-effort, low-risk correctness fix — good candidate for quick pickup but not high priority.

Cost Assessment

Confirmed: both onclick handlers still exist at the described locations, unwrapped. Code matches the issue description exactly.

This is a low-cost, low-risk, mechanical fix. The two-line change is exactly as described: wrap each EDIT_ROOM_MODAL.write().room = None; in crate::util::defer(move || {...}), matching the pattern already used at line 333 (leave-room path) in the same file. crate::util::defer already exists and is used elsewhere in the codebase (per the dioxus-signal-safety rule doc), so no new infrastructure is needed — this is a pure copy-paste of an established idiom, not a novel implementation.

Scope is confined to one file. The issue also suggests grepping other modals for the same undeferred-write-in-onclick shape, which would modestly widen scope if similar violations are found elsewhere (e.g. other modal close handlers) — worth a quick grep -rn "onclick: move" ui/src/components | grep -B2 '\.write()' sweep, but that's minutes of work, not new design.

Complexity is minimal — no tricky edge cases in the fix itself. The only subtlety is timing: deferring the close makes it asynchronous (via setTimeout(0)), so any Playwright test asserting the modal disappears synchronously right after click could need a waitFor-style adjustment. The issue explicitly calls out copy-clipboard-feedback.spec.ts and room-details specs as needing re-verification post-fix, which is really the only real testing cost here — running the existing Playwright suite (already documented in AGENTS.md) rather than writing new tests, since this is a defensive/latent-bug fix with no reproduced symptom.

No external dependencies (no signups, approvals, or third-party coordination). Risk is low: this only changes closing timing for one modal, isolated blast radius, and the pattern is proven safe elsewhere in the same file. Given no reproduced crash, the risk of the current code is also low (per the issue's own severity note), so this is a cheap, safe, well-specified fix — overall a small/quick task, roughly 30–60 minutes including the grep sweep and Playwright re-run.

---

Generated by issue-prioritizer