diff --git a/CHANGELOG.md b/CHANGELOG.md index e7595868..1bc20805 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,43 @@ Full module documentation: [hexdocs.pm/mob](https://hexdocs.pm/mob). --- +## [0.7.29] - 2026-08-27 + +### Added +- **`Mob.UI.sheet/2`** — a native modal bottom sheet (iOS `.sheet`, + Android Material 3 `ModalBottomSheet`) that composes ordinary Mob + nodes as content. `:detents` (`[:medium, :large]` subset), `:on_dismiss` + (delivered as `{:dismiss, tag}`, exactly once), `:background`, `:scrim`, + `:corner_radius`, and a custom drag indicator (`:drag_indicator_color`/ + `_width`/`_height`/`_rail_height`, all four required together or omit + all four). Per-platform `:ios`/`:android` style overrides via the + existing platform-block mechanism. See + `decisions/2026-08-26-native-sheet-primitive.md` for the presentation- + state-via-identity design, the background/corner_radius + double-application avoidance on both platforms, and the documented iOS + scrim-opacity limitation (native `.sheet` doesn't expose dimming-layer + opacity — Android applies `:scrim` exactly, iOS stays system-black). + +### Fixed +- Drag-indicator completeness validation (all four geometry props + together or none) is now checked against `:ios`/`:android` overrides + merged with the base props, not just the base props alone — a partial + override no longer silently passes validation and renders the system + default indicator instead of the requested one. +- Color props that resolve to neither the active theme nor the base + palette now log a warning instead of silently passing an unresolved + atom through to native (previously a likely typo'd theme token would + render as an invisible, fully-transparent color with no signal at all). +- iOS: sheet content now receives its `:padding` (was dropped). +- iOS: `corner_radius: 0` on a sheet is no longer indistinguishable from + "not set" — square corners are now representable and distinct from the + system default. +- iOS: a sheet's `:id` no longer reports a 0x0 frame via + `Mob.Test`/`element_frames` — its switch-case view is an invisible + presentation anchor, not the sheet's real on-screen content, so frame + tracking is skipped there rather than publishing a value known to be + wrong. + ## [0.7.28] - 2026-08-26 ### Fixed diff --git a/decisions/2026-08-27-frame-registry-purge-by-id.md b/decisions/2026-08-27-frame-registry-purge-by-id.md new file mode 100644 index 00000000..ffc8b74e --- /dev/null +++ b/decisions/2026-08-27-frame-registry-purge-by-id.md @@ -0,0 +1,88 @@ +# iOS element_frames registry: purge-by-id, not wipe-and-repopulate + +- Date: 2026-08-27 +- Status: accepted + +## Context + +`Mob.Test.element_frames/1` (backed by `mob_register_frame`/`element_frames` +in `ios/mob_nif.m`) lets an agent read a tagged element's on-screen frame +without a screenshot. The registry was cleared unconditionally +(`mob_clear_frames()`) at the top of every `nif_set_root` call, on the +assumption that `MobFrameTracker`'s `.background(GeometryReader{...})` in +`MobRootView.swift` would reliably repopulate every surviving element on its +next layout pass. + +That assumption was wrong in a specific, confirmed way: `MobFrameTracker` +registered via `.onChange(of: geo.frame(in: .global), initial: true)`, which +only re-fires when the frame's *value* changes. An element whose on-screen +position/size is unchanged between two renders — the common case for most of +a screen on most renders — never got a new `onChange` firing, so its entry +stayed wiped until something eventually moved it. `Mob.Test.element_frames` +would report a still-visible, unmoved element as missing. + +## Decision + +Replace the registry's clear step with a **purge by id**: `nif_set_root` +parses the incoming tree first, walks it to collect every `:id` present +(`mob_collect_frame_ids`), and removes only the registry entries whose id is +*not* in that set (`mob_purge_frames_except`). A surviving element's +existing entry is never touched — nothing to race, nothing that needs to +"repopulate." `MobFrameTracker`'s original `onChange(of: frame, initial: +true)` is untouched and still the only thing that writes an entry, exactly +as before this change; it just no longer needs to fire on every render, only +on a genuine first-appearance or a real frame change. + +### Approaches tried and rejected + +Three variations of "make every surviving element re-register on every +render" were tried and rejected, all for the same underlying reason, +confirmed by device testing with an instrumented native build +(`NSLog` in `mob_register_frame`/the clear step, watched live via `xcrun +simctl spawn log stream`) plus a two-element repro screen (one +element that never moves, one removed by a later tap): + +1. **`onChange(of: MobViewModel.shared.rootVersion)`** (via + `@ObservedObject`) — forces re-registration on every render. Fixed the + "static element survives a no-op-frame rerender" case, but a removed + element's entry *also* survived its own removal: an `@Published` change + reaches every still-mounted subscriber, including a view mid-removal in + the same transaction, before SwiftUI finishes pruning views absent from + the new declared tree. +2. **Same mechanism via a custom `@Environment` key** instead of + `@ObservedObject` — same failure. Environment propagation turned out to + have the identical property: a view being removed still gets evaluated + with the updated environment value one more time. +3. **Direct, unconditional registration inside the `GeometryReader` closure + body** (no `onChange` at all) — correctly stopped the removed element + from reappearing, but was unreliable for the *surviving* element: it + worked for one kind of sibling change and not another, indicating the + closure isn't dependably re-invoked on every render regardless of + whether *this* element's own geometry needs recomputing. +4. **Combining #2 with `.id(generation)`** on the tracking subview, to try + to force a fresh reconstruction every render — no different result than + #2 alone. + +All four shared the same root issue: they tried to make **registration** +race-proof against SwiftUI's removal timing. Purging by id sidesteps the +race entirely by making the **registry's list of what should exist** +authoritative, computed directly from the tree BEAM just sent — not +inferred from which views happen to fire a reactive callback. + +## Consequences + +- `MobRootView.swift` is unchanged. The fix is entirely in `ios/mob_nif.m`: + `nif_set_root` now parses the tree before touching the registry (it used + to clear first, parse second), and calls `mob_purge_frames_except` with + the freshly-collected id set instead of `mob_clear_frames`. +- Verified device-side (iOS 17 Pro simulator) via a throwaway two-element + screen and a scripted `Mob.Test`-equivalent RPC sequence: a static + element survives an unrelated sibling's re-render, and a removed + element's frame disappears on the same render that removes it. No + automated regression test exists for this (no XCTest target in this + repo — see `CLAUDE.md`'s native-change verification note); this is a + documented, reproducible manual verification, not CI-enforced. +- If SwiftUI's removal-pass timing ever needs the *reverse* signal (e.g. + "notify me when an element is about to be removed, before it happens"), + this same purge-by-id computation (`mob_collect_frame_ids`) is the + natural place to diff old-vs-new id sets and produce that. diff --git a/ios/mob_nif.m b/ios/mob_nif.m index 460b4d07..12cec61f 100644 --- a/ios/mob_nif.m +++ b/ios/mob_nif.m @@ -2000,8 +2000,9 @@ static ERL_NIF_TERM nif_set_theme(ErlNifEnv *env, int argc, const ERL_NIF_TERM a return g_font_fallback ?: @[]; } -static NSMutableDictionary *mob_frame_registry(void); // both defined with the -static void mob_clear_frames(void); // element frame registry below +static NSMutableDictionary *mob_frame_registry(void); // both defined with the +static void mob_purge_frames_except(NSSet *); // element frame registry below +static NSSet *mob_collect_frame_ids(MobNode *); static ERL_NIF_TERM nif_set_root(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) { ErlNifBinary bin; @@ -2009,10 +2010,6 @@ static ERL_NIF_TERM nif_set_root(ErlNifEnv *env, int argc, const ERL_NIF_TERM ar !enif_inspect_iolist_as_binary(env, argv[0], &bin)) return enif_make_badarg(env); - // New render tree — drop stale element frames; MobFrameTracker repopulates - // on the next layout pass. - mob_clear_frames(); - NSData *data = [NSData dataWithBytes:bin.data length:bin.size]; NSError *err = nil; id json = [NSJSONSerialization JSONObjectWithData:data options:0 error:&err]; @@ -2025,6 +2022,17 @@ static ERL_NIF_TERM nif_set_root(ErlNifEnv *env, int argc, const ERL_NIF_TERM ar if (!node) return enif_make_atom(env, "error"); + // Purge only the ids absent from this tree, rather than wiping the whole + // registry and relying on MobFrameTracker to repopulate every surviving + // element. A wipe-then-repopulate design raced SwiftUI's own teardown: + // an element being removed can still get one more GeometryReader layout + // pass as part of that removal, so a repopulation trigger tied to "did I + // just get (re)rendered" fires for outgoing views too, right as they're + // disappearing, and their stale frame survives. Purging by id instead + // never touches a surviving element's existing entry (nothing to race), + // and correctly drops one that's genuinely gone from the new tree. + mob_purge_frames_except(mob_collect_frame_ids(node)); + // Snapshot and reset the transition enif_mutex_lock(tap_mutex); char transition[16]; @@ -6404,11 +6412,36 @@ void mob_register_frame(const char *id, double x, double y, double w, double h) } } -// Drop stale frames when the render tree changes (called from nif_set_root). -static void mob_clear_frames(void) { +// Recursively collect every :id present in a freshly-parsed tree, so +// nif_set_root can purge just the registry entries that fell out of the new +// tree instead of wiping everything (see mob_purge_frames_except below for +// why: a wipe-everything + MobFrameTracker-repopulates design races +// SwiftUI's own removal pass for an outgoing element). +static void mob_collect_frame_ids_into(MobNode *node, NSMutableSet *ids) { + if (node.nativeViewId) + [ids addObject:node.nativeViewId]; + for (MobNode *child in node.children) + mob_collect_frame_ids_into(child, ids); +} + +static NSSet *mob_collect_frame_ids(MobNode *root) { + NSMutableSet *ids = [NSMutableSet set]; + mob_collect_frame_ids_into(root, ids); + return ids; +} + +// Remove any registered frame whose id isn't in the incoming tree (called +// from nif_set_root with that tree's live id set). A surviving element's +// entry is never touched here — no race, no dependency on it re-registering +// itself — only genuinely-removed ids are dropped. +static void mob_purge_frames_except(NSSet *liveIds) { NSMutableDictionary *reg = mob_frame_registry(); @synchronized(reg) { - [reg removeAllObjects]; + NSMutableArray *stale = [NSMutableArray array]; + for (NSString *key in reg) + if (![liveIds containsObject:key]) + [stale addObject:key]; + [reg removeObjectsForKeys:stale]; } } diff --git a/mix.exs b/mix.exs index 34f4d177..0191664c 100644 --- a/mix.exs +++ b/mix.exs @@ -4,7 +4,7 @@ defmodule Mob.MixProject do def project do [ app: :mob, - version: "0.7.28", + version: "0.7.29", elixir: "~> 1.19", start_permanent: Mix.env() == :prod, elixirc_paths: elixirc_paths(Mix.env()),