From 894bf5d5543086cc5cd970b557b954669ccb6edd Mon Sep 17 00:00:00 2001 From: GenericJam Date: Thu, 27 Aug 2026 15:13:08 -0600 Subject: [PATCH 1/3] feat(ui): add intrinsic sheets and box semantics --- ios/MobNode.h | 10 +- ios/MobRootView.swift | 102 +++++++++++++++++++-- ios/mob_nif.m | 15 +++ lib/mob/renderer.ex | 11 ++- lib/mob/ui.ex | 60 ++++++++---- priv/tags/android.txt | 1 + priv/tags/ios.txt | 1 + test/mob/native_box_accessibility_test.exs | 34 +++++++ test/mob/native_sheet_test.exs | 29 ++++++ test/mob/renderer_test.exs | 52 ++++++++++- test/mob/sigil_test.exs | 10 ++ test/mob/ui_test.exs | 47 +++++----- 12 files changed, 321 insertions(+), 51 deletions(-) create mode 100644 test/mob/native_box_accessibility_test.exs create mode 100644 test/mob/native_sheet_test.exs diff --git a/ios/MobNode.h b/ios/MobNode.h index 388196f0..ec830e07 100644 --- a/ios/MobNode.h +++ b/ios/MobNode.h @@ -235,8 +235,12 @@ NS_ASSUME_NONNULL_BEGIN @property(nonatomic, strong, nullable) NSDictionary *nativeViewProps; // full props dict forwarded to the factory -// Accessibility — set from the tap tag atom name; read by XCTest / ui_describe_all +// Accessibility — identifiers support test addressing; labels and disabled +// state describe composite controls such as tappable boxes. @property(nonatomic, copy, nullable) NSString *accessibilityId; +@property(nonatomic, copy, nullable) NSString *accessibilityLabel; +@property(nonatomic, copy, nullable) NSString *accessibilityRole; +@property(nonatomic) BOOL disabled; // Icon — logical name resolved to an SF Symbol on iOS / Material Symbol // on Android. textSize and textColor control glyph sizing + tint. @@ -271,7 +275,7 @@ NS_ASSUME_NONNULL_BEGIN // system-default box in practice), but a sheet's corners are visibly // square-vs-rounded, so `sheetCornerRadius` gets its own -1 sentinel // (unset — use the system default) instead. `sheetDetents` is the raw -// "medium"/"large" string list from Mob.Renderer — mapped to +// built-in strings or content-detent dictionaries from Mob.Renderer — mapped to // PresentationDetent by MobSheetView (see MobRootView.swift), not here, so // this header stays framework-agnostic. Indicator geometry defaults to -1 // (unset — use the system default indicator); Mob.UI.sheet's validation @@ -279,7 +283,7 @@ NS_ASSUME_NONNULL_BEGIN // for >= 0 is enough to know whether a complete custom indicator was // supplied. @property(nonatomic) CGFloat sheetCornerRadius; -@property(nonatomic, strong, nullable) NSArray *sheetDetents; +@property(nonatomic, strong, nullable) NSArray *sheetDetents; @property(nonatomic, strong, nullable) UIColor *dragIndicatorColor; @property(nonatomic) CGFloat dragIndicatorWidth; @property(nonatomic) CGFloat dragIndicatorHeight; diff --git a/ios/MobRootView.swift b/ios/MobRootView.swift index f37240d1..887bf73c 100644 --- a/ios/MobRootView.swift +++ b/ios/MobRootView.swift @@ -683,10 +683,22 @@ private struct MobBox: View { lineWidth: node.borderWidth) .allowsHitTesting(false) ) - .ifLet(node.onTap) { view, tap in + .ifLet(node.disabled ? nil : node.onTap) { view, tap in view.contentShape(Rectangle()).onTapGesture { tap() } } .mobGestures(node) + .ifLet(node.accessibilityLabel) { view, label in + view + .accessibilityElement(children: .ignore) + .accessibilityLabel(label) + } + .ifLet(node.accessibilityRole == "button" ? () : nil) { view, _ in + view.accessibilityAddTraits(.isButton) + } + .ifLet(node.disabled ? () : nil) { view, _ in + view.accessibilityAddTraits(.isNotEnabled) + } + .allowsHitTesting(!node.disabled) // (offset is applied uniformly by MobNodeView's body; not here) } } @@ -1416,10 +1428,31 @@ private struct MobSlider: View { // both fall out for free: a rerender with the sheet still present reuses // this state; a rerender without it tears the view (and its presentation) // down entirely. +private struct MobSheetContentHeightKey: PreferenceKey { + static var defaultValue: CGFloat = 1 + + static func reduce(value: inout CGFloat, nextValue: () -> CGFloat) { + value = max(value, nextValue()) + } +} + +private struct MobAvailableSheetHeightKey: EnvironmentKey { + static let defaultValue: CGFloat = 1 +} + +private extension EnvironmentValues { + var mobAvailableSheetHeight: CGFloat { + get { self[MobAvailableSheetHeightKey.self] } + set { self[MobAvailableSheetHeightKey.self] = newValue } + } +} + private struct MobSheetView: View { let node: MobNode + @Environment(\.mobAvailableSheetHeight) private var availableHeight @State private var isPresented = true @State private var dismissSent = false + @State private var intrinsicContentHeight: CGFloat = 1 var body: some View { Color.clear @@ -1435,8 +1468,23 @@ private struct MobSheetView: View { node.onDismiss?() } - @ViewBuilder - private var sheetContent: some View { + private var contentDetent: [String: Any]? { + node.sheetDetents?.compactMap { $0 as? [String: Any] } + .first { $0["type"] as? String == "content" } + } + + private var maximumHeight: CGFloat { + guard let configured = contentDetent?["max_height"] as? NSNumber else { + return availableHeight + } + return min(CGFloat(truncating: configured), availableHeight) + } + + private var limitedContentHeight: CGFloat { + max(1, min(intrinsicContentHeight, maximumHeight)) + } + + private var sheetBody: some View { VStack(spacing: 0) { ForEach(Array(node.childNodes.enumerated()), id: \.offset) { _, child in MobNodeView(node: child) @@ -1444,6 +1492,35 @@ private struct MobSheetView: View { } .frame(maxWidth: .infinity, alignment: .topLeading) .padding(node.paddingEdgeInsets) + } + + @ViewBuilder + private var sheetContent: some View { + Group { + if contentDetent != nil { + ScrollView(.vertical) { + sheetBody + .fixedSize(horizontal: false, vertical: true) + .background { + GeometryReader { geometry in + Color.clear.preference( + key: MobSheetContentHeightKey.self, + value: geometry.size.height + ) + } + } + } + .frame(maxHeight: maximumHeight) + .onPreferenceChange(MobSheetContentHeightKey.self) { measuredHeight in + let intrinsicHeight = max(1, measuredHeight) + if abs(intrinsicHeight - intrinsicContentHeight) > 0.5 { + intrinsicContentHeight = intrinsicHeight + } + } + } else { + sheetBody + } + } // Screen readers should treat the sheet as a self-contained modal — // VoiceOver focus stays inside it until dismissed, matching // .presentationDetents/.sheet's own system-modal behavior. @@ -1470,10 +1547,14 @@ private struct MobSheetView: View { } private var detentSet: Set { - let requested = node.sheetDetents ?? ["medium", "large"] + if contentDetent != nil { + return [.height(limitedContentHeight)] + } + + let builtInDetents = node.sheetDetents?.compactMap { $0 as? String } ?? [] var resolved: Set = [] - if requested.contains("medium") { resolved.insert(.medium) } - if requested.contains("large") { resolved.insert(.large) } + if builtInDetents.contains("medium") { resolved.insert(.medium) } + if builtInDetents.contains("large") { resolved.insert(.large) } // Mob.UI.sheet/2 already validates :detents is a nonempty subset of // [:medium, :large] — this fallback only matters for a hand-built // node map that skipped that validation (e.g. `~MOB` sigil literal). @@ -1555,6 +1636,7 @@ public struct MobRootView: View { // SwiftUI observation, which doesn't carry the animation context and // produces a default crossfade instead of the .move transition). @State private var currentNavVersion: Int = 0 + @State private var availableSheetHeight: CGFloat = 1 public init() {} @@ -1601,6 +1683,14 @@ public struct MobRootView: View { .transition(.opacity) } } + .background { + GeometryReader { geometry in + Color.clear.onChange(of: geometry.size.height, initial: true) { _, height in + availableSheetHeight = max(1, height * 0.9) + } + } + } + .environment(\.mobAvailableSheetHeight, availableSheetHeight) .ignoresSafeArea(.container, edges: [.bottom, .horizontal]) .onChange(of: model.rootVersion) { let t = model.transition diff --git a/ios/mob_nif.m b/ios/mob_nif.m index 2cc4f205..9253e4d9 100644 --- a/ios/mob_nif.m +++ b/ios/mob_nif.m @@ -1248,6 +1248,21 @@ static void mob_send_change_float(int handle, double value) { if ([accessibilityId isKindOfClass:[NSString class]]) { node.accessibilityId = accessibilityId; } + + id accessibilityLabel = props[@"accessibility_label"]; + if ([accessibilityLabel isKindOfClass:[NSString class]]) { + node.accessibilityLabel = accessibilityLabel; + } + + id accessibilityRole = props[@"accessibility_role"]; + if ([accessibilityRole isKindOfClass:[NSString class]]) { + node.accessibilityRole = accessibilityRole; + } + + id disabled = props[@"disabled"]; + if ([disabled isKindOfClass:[NSNumber class]]) { + node.disabled = [disabled boolValue]; + } } NSArray *children = dict[@"children"]; diff --git a/lib/mob/renderer.ex b/lib/mob/renderer.ex index 20506704..8947c203 100644 --- a/lib/mob/renderer.ex +++ b/lib/mob/renderer.ex @@ -72,7 +72,7 @@ defmodule Mob.Renderer do require Logger - alias Mob.{Style, Theme} + alias Mob.{Style, Theme, UI} @default_nif :mob_nif @@ -370,6 +370,12 @@ defmodule Mob.Renderer do {:on_dismiss, {pid, tag}} when is_pid(pid) -> [{"on_dismiss", nif.register_tap({pid, tag})}] + {:detents, detents} -> + encoded_detents = + detents |> UI.normalize_sheet_detents!() |> Enum.map(&encode_sheet_detent/1) + + [{"detents", encoded_detents}] + # IME composition — fires for languages with multi-stage input (CJK, # Korean, Vietnamese, accent input). Phase atom is :began | :updating # | :committed | :cancelled. Apps that need commit-only behaviour @@ -662,6 +668,9 @@ defmodule Mob.Renderer do defp encode_native_value(v), do: v + defp encode_sheet_detent(%{} = detent), do: encode_native_config(detent) + defp encode_sheet_detent(detent) when is_atom(detent), do: Atom.to_string(detent) + # Encode one Mob.Canvas op map for the wire. The `:op` atom becomes a # string ("line"/"circle"/...) so the native dispatch is a string switch. # `:color` is resolved through the same two-step theme path as top-level diff --git a/lib/mob/ui.ex b/lib/mob/ui.ex index 0656e05c..62603dba 100644 --- a/lib/mob/ui.ex +++ b/lib/mob/ui.ex @@ -281,9 +281,12 @@ defmodule Mob.UI do ## Props - * `:detents` — nonempty, duplicate-free subset of `[:medium, :large]`. - Defaults to `[:medium, :large]`. `:medium` alone rejects expansion - to full height; `:large` alone skips the half-height stop. + * `:detents` — nonempty, duplicate-free subset of `[:medium, :large]`, + or the exclusive content-height detent `[:content]` / + `[{:content, max_height: number}]`. Defaults to `[:medium, :large]`. + `:medium` alone rejects expansion to full height; `:large` alone skips + the half-height stop. A content detent wraps intrinsic content and caps + overflow in an internally scrolling body. * `:on_dismiss` — `{pid, tag}`, delivered as `handle_info({:dismiss, tag}, socket)` exactly once when the sheet is dismissed (swipe-down, back gesture, or outside tap) — the same `{atom, tag}` wire shape as `on_focus`, @@ -337,8 +340,7 @@ defmodule Mob.UI do def sheet(children, opts) when is_list(opts), do: sheet(children, Map.new(opts)) def sheet(children, %{} = opts) do - detents = Map.get(opts, :detents, @sheet_detents) - validate_detents!(detents) + detents = opts |> Map.get(:detents, @sheet_detents) |> normalize_sheet_detents!() validate_on_dismiss!(Map.get(opts, :on_dismiss)) validate_style!(opts) validate_platform_override!(opts, :ios) @@ -352,24 +354,48 @@ defmodule Mob.UI do } end - defp validate_detents!(detents) do - unless is_list(detents) and detents != [] do - raise ArgumentError, - "Mob.UI.sheet :detents must be a nonempty list, got: #{inspect(detents)}" + @doc false + @spec normalize_sheet_detents!(term()) :: [atom() | map()] + def normalize_sheet_detents!([:content]), do: [%{type: :content}] + + def normalize_sheet_detents!([{:content, options}]) when is_list(options) do + if Keyword.keyword?(options) do + case Keyword.fetch(options, :max_height) do + {:ok, maximum} when is_number(maximum) and maximum > 0 and length(options) == 1 -> + [%{type: :content, max_height: maximum}] + + _other -> + invalid_sheet_detents!() + end + else + invalid_sheet_detents!() end + end - unless Enum.uniq(detents) == detents do - raise ArgumentError, - "Mob.UI.sheet :detents must not contain duplicates, got: #{inspect(detents)}" - end + def normalize_sheet_detents!([%{type: :content} = detent]) when map_size(detent) == 1, + do: [detent] - unless Enum.all?(detents, &(&1 in @sheet_detents)) do - raise ArgumentError, - "Mob.UI.sheet :detents must be a subset of #{inspect(@sheet_detents)}, " <> - "got: #{inspect(detents)}" + def normalize_sheet_detents!([%{type: :content, max_height: maximum} = detent]) + when map_size(detent) == 2 and is_number(maximum) and maximum > 0, + do: [detent] + + def normalize_sheet_detents!(detents) when is_list(detents) and detents != [] do + if Enum.all?(detents, &(&1 in @sheet_detents)) and + length(detents) == length(Enum.uniq(detents)) do + detents + else + invalid_sheet_detents!() end end + def normalize_sheet_detents!(_detents), do: invalid_sheet_detents!() + + defp invalid_sheet_detents! do + raise ArgumentError, + "Mob.UI.sheet :detents must be unique :medium/:large values or one " <> + ":content detent with an optional positive :max_height" + end + defp validate_on_dismiss!(nil), do: :ok defp validate_on_dismiss!({pid, tag}) when is_pid(pid) and is_atom(tag), do: :ok diff --git a/priv/tags/android.txt b/priv/tags/android.txt index 06fbf07c..9ad10ffb 100644 --- a/priv/tags/android.txt +++ b/priv/tags/android.txt @@ -14,6 +14,7 @@ List Progress Row Scroll +Sheet Slider Spacer TabBar diff --git a/priv/tags/ios.txt b/priv/tags/ios.txt index 0380a373..5c292af6 100644 --- a/priv/tags/ios.txt +++ b/priv/tags/ios.txt @@ -13,6 +13,7 @@ List Progress Row Scroll +Sheet Slider Spacer TabBar diff --git a/test/mob/native_box_accessibility_test.exs b/test/mob/native_box_accessibility_test.exs new file mode 100644 index 00000000..f0f959ee --- /dev/null +++ b/test/mob/native_box_accessibility_test.exs @@ -0,0 +1,34 @@ +# These source-contract tests guard native SwiftUI behavior that Elixir cannot execute. +# credo:disable-for-this-file Jump.CredoChecks.VacuousTest +defmodule Mob.NativeBoxAccessibilityTest do + use ExUnit.Case, async: true + + @root Path.expand("../..", __DIR__) + + test "iOS parses box accessibility state at the native boundary" do + header = File.read!(Path.join(@root, "ios/MobNode.h")) + nif = File.read!(Path.join(@root, "ios/mob_nif.m")) + + assert header =~ "NSString *accessibilityLabel" + assert header =~ "NSString *accessibilityRole" + assert header =~ "BOOL disabled" + assert nif =~ ~s|props[@"accessibility_label"]| + assert nif =~ "node.accessibilityLabel = accessibilityLabel" + assert nif =~ ~s|props[@"accessibility_role"]| + assert nif =~ "node.accessibilityRole = accessibilityRole" + assert nif =~ ~s|props[@"disabled"]| + assert nif =~ "node.disabled = [disabled boolValue]" + end + + test "iOS box exposes one labeled action and suppresses disabled input" do + source = File.read!(Path.join(@root, "ios/MobRootView.swift")) + + assert source =~ ".accessibilityElement(children: .ignore)" + assert source =~ "view.accessibilityLabel(label)" + assert source =~ ~s|node.accessibilityRole == "button"| + assert source =~ ".accessibilityAddTraits(.isButton)" + assert source =~ ".accessibilityAddTraits(.isNotEnabled)" + assert source =~ ".allowsHitTesting(!node.disabled)" + assert source =~ ".ifLet(node.disabled ? nil : node.onTap)" + end +end diff --git a/test/mob/native_sheet_test.exs b/test/mob/native_sheet_test.exs new file mode 100644 index 00000000..fe15c8b6 --- /dev/null +++ b/test/mob/native_sheet_test.exs @@ -0,0 +1,29 @@ +# These source-contract tests guard native SwiftUI behavior that Elixir cannot execute. +# credo:disable-for-this-file Jump.CredoChecks.VacuousTest +defmodule Mob.NativeSheetTest do + use ExUnit.Case, async: true + + @ios Path.expand("../../ios", __DIR__) + + test "SwiftUI sheet preserves built-ins and measures content detents" do + source = File.read!(Path.join(@ios, "MobRootView.swift")) + + assert source =~ "MobSheetContentHeightKey" + assert source =~ "builtInDetents.contains(\"medium\")" + assert source =~ "builtInDetents.contains(\"large\")" + assert source =~ ".height(limitedContentHeight)" + assert source =~ ".onPreferenceChange(MobSheetContentHeightKey.self)" + assert source =~ "ScrollView(.vertical)" + end + + test "content detent reclamps against live sheet geometry" do + source = File.read!(Path.join(@ios, "MobRootView.swift")) + + assert source =~ "GeometryReader { geometry in" + assert source =~ "availableSheetHeight = max(1, height * 0.9)" + assert source =~ "min(intrinsicContentHeight, maximumHeight)" + assert source =~ "let intrinsicHeight = max(1, measuredHeight)" + assert source =~ "intrinsicContentHeight = intrinsicHeight" + refute source =~ "UIScreen.main.bounds.height * 0.9" + end +end diff --git a/test/mob/renderer_test.exs b/test/mob/renderer_test.exs index 7a725d6c..a2df69e6 100644 --- a/test/mob/renderer_test.exs +++ b/test/mob/renderer_test.exs @@ -104,6 +104,27 @@ defmodule Mob.RendererTest do assert decoded["props"]["text"] == "Hello" end + test "box accessibility and disabled props survive the renderer boundary" do + tree = %{ + type: :box, + props: %{ + accessibility_label: "Open Acme company", + accessibility_role: :button, + disabled: true + }, + children: [] + } + + Renderer.render(tree, :ios, MockNIF) + {:set_root, [json]} = Enum.find(MockNIF.calls(), fn {f, _} -> f == :set_root end) + props = :json.decode(json)["props"] + + assert props["accessibility_label"] == "Open Acme company" + assert props["accessibility_role"] == "button" + assert props["disabled"] == true + refute Map.has_key?(props, "on_tap") + end + test "JSON contains nested children" do tree = %{ type: :column, @@ -1284,9 +1305,34 @@ defmodule Mob.RendererTest do assert Enum.at(decoded_children, 1)["props"]["text"] == "b" end - test "detents serialize as a list of strings" do - Renderer.render(sheet_tree(%{detents: [:medium]}), :android, MockNIF) - assert set_root_json()["props"]["detents"] == ["medium"] + test "detents serialize in canonical built-in and content forms" do + for {detents, expected} <- [ + {[:medium], ["medium"]}, + {[:large, :medium], ["large", "medium"]}, + {[:content], [%{"type" => "content"}]}, + {[{:content, max_height: 240}], [%{"type" => "content", "max_height" => 240}]}, + {[%{type: :content}], [%{"type" => "content"}]}, + {[%{type: :content, max_height: 320}], [%{"type" => "content", "max_height" => 320}]} + ] do + MockNIF.reset() + Renderer.render(sheet_tree(%{detents: detents}), :android, MockNIF) + assert set_root_json()["props"]["detents"] == expected + end + end + + test "renderer rejects invalid raw detent nodes" do + for detents <- [ + [:content, :medium], + [%{type: :content, max_height: -1}], + [%{type: :content, unknown: true}], + :content, + [{:content, :not_options}], + [%{type: :bogus}] + ] do + assert_raise ArgumentError, ~r/:detents/, fn -> + Renderer.render(sheet_tree(%{detents: detents}), :android, MockNIF) + end + end end test "on_dismiss registers through the tap registry and serializes as an integer handle" do diff --git a/test/mob/sigil_test.exs b/test/mob/sigil_test.exs index 4fbff475..03526dc5 100644 --- a/test/mob/sigil_test.exs +++ b/test/mob/sigil_test.exs @@ -242,6 +242,16 @@ defmodule Mob.SigilTest do node = ~MOB() assert node.type == :gpu_view end + + test "Sheet preserves a typed content detent expression" do + detents = [{:content, max_height: 480}] + + node = ~MOB( + +) + + assert node.props.detents == detents + end end # ── parity with raw maps ───────────────────────────────────────────────────── diff --git a/test/mob/ui_test.exs b/test/mob/ui_test.exs index 3fa5ed22..90d2a31b 100644 --- a/test/mob/ui_test.exs +++ b/test/mob/ui_test.exs @@ -255,6 +255,17 @@ defmodule Mob.UITest do end describe "sheet/2 detents" do + test "normalizes content detents with an optional maximum height" do + assert UI.sheet(UI.text(text: "hi"), detents: [:content]).props.detents == [ + %{type: :content} + ] + + assert UI.sheet(UI.text(text: "hi"), detents: [{:content, max_height: 480}]).props.detents == + [ + %{type: :content, max_height: 480} + ] + end + test "accepts [:medium]" do assert UI.sheet(UI.text(text: "hi"), detents: [:medium]).props.detents == [:medium] end @@ -270,27 +281,21 @@ defmodule Mob.UITest do ] end - test "rejects an empty list" do - assert_raise ArgumentError, ~r/nonempty/, fn -> - UI.sheet(UI.text(text: "hi"), detents: []) - end - end - - test "rejects duplicates" do - assert_raise ArgumentError, ~r/duplicates/, fn -> - UI.sheet(UI.text(text: "hi"), detents: [:medium, :medium]) - end - end - - test "rejects a detent outside [:medium, :large]" do - assert_raise ArgumentError, ~r/subset/, fn -> - UI.sheet(UI.text(text: "hi"), detents: [:medium, :full]) - end - end - - test "rejects a non-list" do - assert_raise ArgumentError, ~r/nonempty list/, fn -> - UI.sheet(UI.text(text: "hi"), detents: :medium) + test "rejects invalid built-in and content detents" do + for detents <- [ + [], + [:medium, :medium], + [:content, :medium], + [:full], + [{:content, max_height: 0}], + [{:content, max_height: "480"}], + [{:content, max_height: 480, unknown: true}], + [%{type: :content, unknown: true}], + :medium + ] do + assert_raise ArgumentError, ~r/:detents/, fn -> + UI.sheet(UI.text(text: "hi"), detents: detents) + end end end end From 34866b5ad9040d0dfd9d5962447416a844b51cb1 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Thu, 27 Aug 2026 15:33:34 -0600 Subject: [PATCH 2/3] Address adversarial review of intrinsic sheets and box semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eight findings from an adversarial pass on 894bf5d, plus two of my own. iOS Box — three interacting fixes: - .disabled(node.disabled) replaces .allowsHitTesting(!node.disabled). allowsHitTesting(false) makes a view TRANSPARENT to touches, so a disabled box used as a blocking overlay or dimmed backdrop passed taps straight through to the content it was meant to be shielding. .disabled blocks interaction in the subtree while still consuming the touch. - Stop branching on `disabled`. `ifLet` is @ViewBuilder if/else, i.e. _ConditionalContent, so flipping the branch hands SwiftUI a structurally different view and tears the subtree down. `disabled` is routinely toggled: a box wrapping a TextField lost its in-flight text and focus, and one wrapping a Sheet re-presented it. Tap wiring now branches on handler presence only, with the disabled check inside the closure, and both traits are unconditional OptionSet modifiers. - Collapse to one accessibility element for a label OR an explicit button role, not just a label. Traits added without collapsing land on every descendant, so a role-only box announced each nested Text as its own button. iOS Sheet: - Content detents no longer flash a 1pt hairline on presentation. Content can only be measured once the sheet is up, so the first detent was computed from sentinels (intrinsic 1, environment default 1) and resolved to .height(1) on EVERY presentation. Measurement is now Optional and unmeasured presents at .medium; the environment default 0 means "root geometry unknown" and no longer doubles as a clamp. - The detent accounts for the sheet's bottom safe-area inset. .height() is TOTAL sheet height while the content region is inset by the home indicator, so an intrinsic sheet was ~34pt short on notched devices — last rows under the indicator, and scrollable when the content was meant to fit exactly. The preference now carries height and inset together. - The 0.9 ceiling is a named constant with its rationale. Elixir: - Restored per-rule validation messages that echo the offending value. [:medium, :medium] now says "must not contain duplicates, got: ..." instead of restating the whole grammar, and a malformed content map gets its own message rather than falling through to the built-in-subset branch. - @external_resource on both tag manifests in Mob.Sigil and Mob.ScreenCase. They are read into module attributes at compile time, so adding Sheet recompiled nothing for anyone with a warm _build — every path-dep consumer, which is the standard mob dev setup — leaving the sigil rejecting until a manual mix clean. - Documented the two content-detent limitations the implementation implies: a scrollable child expands rather than scrolling independently, and invalid detents now raise at the renderer boundary instead of degrading. Test quality: the native grep tests broke on four separate behaviour-preserving refactors here (naming a constant, unwrapping an Optional, adding the inset, making traits unconditional). They pinned source text, not behaviour. Retargeted at the mechanisms — the unmeasured fallback, the inset arithmetic, the non-branching traits, .disabled over .allowsHitTesting — so they break when the behaviour regresses rather than when the code is tidied. 1119 tests, format, credo --strict, clang-format, swiftlint (only the pre-existing force_cast), and clang -fsyntax-only on mob_nif.m all clean. Co-Authored-By: Claude Opus 5 (1M context) --- ios/MobRootView.swift | 155 ++++++++++++++++----- lib/mob/screen_case.ex | 5 + lib/mob/sigil.ex | 8 ++ lib/mob/ui.ex | 66 +++++++-- test/mob/native_box_accessibility_test.exs | 30 +++- test/mob/native_sheet_test.exs | 24 +++- 6 files changed, 233 insertions(+), 55 deletions(-) diff --git a/ios/MobRootView.swift b/ios/MobRootView.swift index 887bf73c..49bfd500 100644 --- a/ios/MobRootView.swift +++ b/ios/MobRootView.swift @@ -644,6 +644,15 @@ private struct MobFrameTracker: ViewModifier { private struct MobBox: View { let node: MobNode + /// True when the box is standing in for a control rather than plain + /// layout — it carries a label, or the caller asked for a button role. + /// Only then is it right to collapse the subtree into one accessibility + /// element; a passive box with labelled children must keep them visible + /// to VoiceOver. + private var isAccessibilityControl: Bool { + node.accessibilityLabel != nil || node.accessibilityRole == "button" + } + var body: some View { let alignment: Alignment = boxAlignmentFromString(node.boxAlign) @@ -683,22 +692,39 @@ private struct MobBox: View { lineWidth: node.borderWidth) .allowsHitTesting(false) ) - .ifLet(node.disabled ? nil : node.onTap) { view, tap in - view.contentShape(Rectangle()).onTapGesture { tap() } + // Branch on `onTap` presence only, never on `disabled`. `ifLet` is + // @ViewBuilder if/else, i.e. _ConditionalContent — flipping the branch + // gives SwiftUI a structurally different view and tears the subtree + // down. `disabled` is a routinely toggled prop, so branching on it + // would drop a wrapped TextField's in-flight text and focus, and + // re-present a wrapped Sheet. The disabled check moves inside the + // closure, where it costs nothing structurally. + .ifLet(node.onTap) { view, tap in + view.contentShape(Rectangle()).onTapGesture { + if !node.disabled { tap() } + } } .mobGestures(node) - .ifLet(node.accessibilityLabel) { view, label in - view - .accessibilityElement(children: .ignore) - .accessibilityLabel(label) - } - .ifLet(node.accessibilityRole == "button" ? () : nil) { view, _ in - view.accessibilityAddTraits(.isButton) + // Collapse to a single accessibility element whenever this box is + // acting as a control — a label OR an explicit button role. Traits + // added without collapsing land on every descendant instead, so a + // role-only box would announce each nested Text as its own button. + .ifLet(isAccessibilityControl ? () : nil) { view, _ in + view.accessibilityElement(children: .ignore) } - .ifLet(node.disabled ? () : nil) { view, _ in - view.accessibilityAddTraits(.isNotEnabled) + .ifLet(node.accessibilityLabel) { view, label in + view.accessibilityLabel(label) } - .allowsHitTesting(!node.disabled) + // OptionSet-valued, so these stay unconditional modifiers and add no + // _ConditionalContent branch on `disabled`. + .accessibilityAddTraits(node.accessibilityRole == "button" ? .isButton : []) + .accessibilityAddTraits(node.disabled ? .isNotEnabled : []) + // .disabled, not .allowsHitTesting: allowsHitTesting(false) makes the + // view transparent to touches, so a disabled box used as a blocking + // overlay or dimmed backdrop would pass taps through to whatever sits + // behind it. .disabled blocks interaction in the subtree while still + // consuming the touch. + .disabled(node.disabled) // (offset is applied uniformly by MobNodeView's body; not here) } } @@ -1428,16 +1454,33 @@ private struct MobSlider: View { // both fall out for free: a rerender with the sheet still present reuses // this state; a rerender without it tears the view (and its presentation) // down entirely. +// Measured intrinsic content height plus the bottom safe-area inset that +// applies *inside* the sheet. `.presentationDetents(.height(x))` sets the +// sheet's TOTAL height, but the content region is inset by the home indicator, +// so a detent of exactly the content height leaves the last rows under it and +// makes a sheet that should fit exactly scroll instead. +private struct MobSheetContentMetrics: Equatable { + var height: CGFloat = 0 + var bottomInset: CGFloat = 0 +} + private struct MobSheetContentHeightKey: PreferenceKey { - static var defaultValue: CGFloat = 1 + static var defaultValue = MobSheetContentMetrics() - static func reduce(value: inout CGFloat, nextValue: () -> CGFloat) { - value = max(value, nextValue()) + static func reduce(value: inout MobSheetContentMetrics, nextValue: () -> MobSheetContentMetrics) { + let next = nextValue() + value = MobSheetContentMetrics( + height: max(value.height, next.height), + bottomInset: max(value.bottomInset, next.bottomInset) + ) } } +// 0 means "root geometry not read yet". Distinguishing unknown from a real +// measurement matters: defaulting to a small number collapsed the first +// presentation of every content sheet to a hairline. private struct MobAvailableSheetHeightKey: EnvironmentKey { - static let defaultValue: CGFloat = 1 + static let defaultValue: CGFloat = 0 } private extension EnvironmentValues { @@ -1452,7 +1495,11 @@ private struct MobSheetView: View { @Environment(\.mobAvailableSheetHeight) private var availableHeight @State private var isPresented = true @State private var dismissSent = false - @State private var intrinsicContentHeight: CGFloat = 1 + // nil until the content has actually been measured. A numeric sentinel + // here is what produced a 1pt sheet on first presentation: content can + // only be measured after the sheet is up, so the first detent was + // computed from the sentinel. + @State private var contentMetrics: MobSheetContentMetrics? var body: some View { Color.clear @@ -1473,15 +1520,25 @@ private struct MobSheetView: View { .first { $0["type"] as? String == "content" } } + /// Ceiling for the sheet. `availableHeight` is 0 until the root has + /// reported geometry; treat that as "no ceiling known yet" rather than + /// clamping to it, so an unmeasured root can never collapse the sheet. private var maximumHeight: CGFloat { - guard let configured = contentDetent?["max_height"] as? NSNumber else { - return availableHeight + let configured = (contentDetent?["max_height"] as? NSNumber).map { CGFloat(truncating: $0) } + + switch (configured, availableHeight > 0) { + case let (.some(limit), true): return min(limit, availableHeight) + case let (.some(limit), false): return limit + case (.none, true): return availableHeight + case (.none, false): return .greatestFiniteMagnitude } - return min(CGFloat(truncating: configured), availableHeight) } - private var limitedContentHeight: CGFloat { - max(1, min(intrinsicContentHeight, maximumHeight)) + /// Total sheet height for the detent: measured content plus the sheet's + /// own bottom safe-area inset, capped. nil while unmeasured. + private var limitedContentHeight: CGFloat? { + guard let metrics = contentMetrics, metrics.height > 0 else { return nil } + return max(1, min(metrics.height + metrics.bottomInset, maximumHeight)) } private var sheetBody: some View { @@ -1505,16 +1562,27 @@ private struct MobSheetView: View { GeometryReader { geometry in Color.clear.preference( key: MobSheetContentHeightKey.self, - value: geometry.size.height + value: MobSheetContentMetrics( + height: geometry.size.height, + bottomInset: geometry.safeAreaInsets.bottom + ) ) } } } .frame(maxHeight: maximumHeight) - .onPreferenceChange(MobSheetContentHeightKey.self) { measuredHeight in - let intrinsicHeight = max(1, measuredHeight) - if abs(intrinsicHeight - intrinsicContentHeight) > 0.5 { - intrinsicContentHeight = intrinsicHeight + .onPreferenceChange(MobSheetContentHeightKey.self) { measured in + // Ignore sub-point churn so a measurement that feeds the + // detent, which resizes the sheet, which re-measures, + // settles instead of oscillating. + let changed = + contentMetrics.map { + abs(measured.height - $0.height) > 0.5 + || abs(measured.bottomInset - $0.bottomInset) > 0.5 + } ?? true + + if changed, measured.height > 0 { + contentMetrics = measured } } } else { @@ -1548,16 +1616,24 @@ private struct MobSheetView: View { private var detentSet: Set { if contentDetent != nil { - return [.height(limitedContentHeight)] + // Content can only be measured once the sheet is on screen, so the + // first evaluation has nothing to size against. Present at .medium + // for that one frame and switch to the exact height as soon as the + // measurement lands — the previous sentinel-based version resolved + // to .height(1) and flashed a hairline on every presentation. + guard let height = limitedContentHeight else { return [.medium] } + return [.height(height)] } let builtInDetents = node.sheetDetents?.compactMap { $0 as? String } ?? [] var resolved: Set = [] if builtInDetents.contains("medium") { resolved.insert(.medium) } if builtInDetents.contains("large") { resolved.insert(.large) } - // Mob.UI.sheet/2 already validates :detents is a nonempty subset of - // [:medium, :large] — this fallback only matters for a hand-built - // node map that skipped that validation (e.g. `~MOB` sigil literal). + // Mob.Renderer normalizes :detents through Mob.UI.normalize_sheet_detents!/1 + // at the encode boundary, so an invalid list now raises before it ever + // reaches here rather than arriving as an unknown string. This fallback + // is therefore only reachable for a node whose detents key is absent + // entirely. return resolved.isEmpty ? [.medium, .large] : resolved } @@ -1638,6 +1714,11 @@ public struct MobRootView: View { @State private var currentNavVersion: Int = 0 @State private var availableSheetHeight: CGFloat = 1 + /// Share of the root's height a content-detent sheet may occupy at most. + /// Keeps a tall sheet visibly a sheet — parent still showing behind it — + /// rather than an unrecognisable full-screen cover. + private static let sheetHeightCeilingFraction: CGFloat = 0.9 + public init() {} public var body: some View { @@ -1683,10 +1764,20 @@ public struct MobRootView: View { .transition(.opacity) } } + // Live root height, published so a content-detent sheet can cap itself + // against real geometry rather than a guess. Read in a `.background` + // so it costs no layout, and `initial: true` so the first value lands + // without waiting for a resize. Re-fires on rotation, split view and + // Stage Manager resizes, which is what re-clamps a presented sheet. + // + // The ceiling is a fraction of the root rather than the whole thing: + // a content sheet that measures taller than the screen should still + // leave the parent visible behind it, the way .large does, instead of + // becoming an indistinguishable full-screen cover. .background { GeometryReader { geometry in Color.clear.onChange(of: geometry.size.height, initial: true) { _, height in - availableSheetHeight = max(1, height * 0.9) + availableSheetHeight = max(1, height * Self.sheetHeightCeilingFraction) } } } diff --git a/lib/mob/screen_case.ex b/lib/mob/screen_case.ex index c4f80064..873a6e20 100644 --- a/lib/mob/screen_case.ex +++ b/lib/mob/screen_case.ex @@ -93,6 +93,11 @@ defmodule Mob.ScreenCase do # PascalCase tag per line, converted to the snake_case `:type` atom the same # way the sigil does). Plus `:native_view`, the runtime-only escape hatch that # plugin / custom components serialize to and which has no template tag. + # Same compile-time-read caveat as Mob.Sigil's @known_tags: declare the + # manifests external resources or a tag addition won't recompile this module. + @external_resource Application.app_dir(:mob, "priv/tags/ios.txt") + @external_resource Application.app_dir(:mob, "priv/tags/android.txt") + @renderable_types ( read = fn name -> path = Application.app_dir(:mob, "priv/tags/#{name}") diff --git a/lib/mob/sigil.ex b/lib/mob/sigil.ex index 9192f903..f10c03fb 100644 --- a/lib/mob/sigil.ex +++ b/lib/mob/sigil.ex @@ -84,6 +84,14 @@ defmodule Mob.Sigil do # ── Whitelist ──────────────────────────────────────────────────────────────── + # Read at compile time into a module attribute, so the manifests have to be + # declared external resources or editing them recompiles nothing. Without + # this, adding a tag leaves anyone with a warm _build — every path-dep + # consumer, which is the standard mob dev setup — with the sigil still + # rejecting the new tag until a manual `mix clean`. + @external_resource Application.app_dir(:mob, "priv/tags/ios.txt") + @external_resource Application.app_dir(:mob, "priv/tags/android.txt") + @known_tags ( ios_file = Application.app_dir(:mob, "priv/tags/ios.txt") android_file = Application.app_dir(:mob, "priv/tags/android.txt") diff --git a/lib/mob/ui.ex b/lib/mob/ui.ex index 62603dba..92dadca0 100644 --- a/lib/mob/ui.ex +++ b/lib/mob/ui.ex @@ -287,6 +287,21 @@ defmodule Mob.UI do `:medium` alone rejects expansion to full height; `:large` alone skips the half-height stop. A content detent wraps intrinsic content and caps overflow in an internally scrolling body. + + Two things to know about content detents. They size from the content's + *intrinsic* height, so a scrollable child (`scroll`, `lazy_list`) reports + its full content height rather than a viewport height — it will expand + inside the sheet and the sheet's own scroll takes the gesture, instead of + the child scrolling independently. Use `:medium`/`:large` when the sheet's + body is itself scrollable. And the sheet only knows its content height + once it is on screen, so it presents at `:medium` for the first frame and + resizes to the measured height immediately after. + + Invalid `:detents` raise. `Mob.Renderer` re-validates at the encode + boundary through `normalize_sheet_detents!/1`, so a hand-built or `~MOB` + sigil node cannot bypass this by skipping `sheet/2` — such a node now + raises during render rather than silently degrading to + `[:medium, :large]` on the native side. * `:on_dismiss` — `{pid, tag}`, delivered as `handle_info({:dismiss, tag}, socket)` exactly once when the sheet is dismissed (swipe-down, back gesture, or outside tap) — the same `{atom, tag}` wire shape as `on_focus`, @@ -358,17 +373,20 @@ defmodule Mob.UI do @spec normalize_sheet_detents!(term()) :: [atom() | map()] def normalize_sheet_detents!([:content]), do: [%{type: :content}] - def normalize_sheet_detents!([{:content, options}]) when is_list(options) do + def normalize_sheet_detents!([{:content, options}] = detents) when is_list(options) do if Keyword.keyword?(options) do case Keyword.fetch(options, :max_height) do {:ok, maximum} when is_number(maximum) and maximum > 0 and length(options) == 1 -> [%{type: :content, max_height: maximum}] _other -> - invalid_sheet_detents!() + invalid_sheet_detents!( + detents, + "a :content detent takes exactly one option, :max_height, and it must be a positive number" + ) end else - invalid_sheet_detents!() + invalid_sheet_detents!(detents, "a :content detent's options must be a keyword list") end end @@ -379,21 +397,43 @@ defmodule Mob.UI do when map_size(detent) == 2 and is_number(maximum) and maximum > 0, do: [detent] + # An already-normalized content map that matched neither valid shape above: + # extra keys, or a non-positive/non-numeric :max_height. Caught here so the + # error names the actual problem instead of falling through to the + # built-in-subset branch and claiming it should have been :medium/:large. + def normalize_sheet_detents!([%{type: :content}] = detents), + do: + invalid_sheet_detents!( + detents, + "a :content detent accepts only :type and an optional positive :max_height" + ) + def normalize_sheet_detents!(detents) when is_list(detents) and detents != [] do - if Enum.all?(detents, &(&1 in @sheet_detents)) and - length(detents) == length(Enum.uniq(detents)) do - detents - else - invalid_sheet_detents!() + cond do + not Enum.all?(detents, &(&1 in @sheet_detents)) -> + invalid_sheet_detents!( + detents, + "must be a subset of #{inspect(@sheet_detents)}, or a single exclusive " <> + ":content detent" + ) + + length(detents) != length(Enum.uniq(detents)) -> + invalid_sheet_detents!(detents, "must not contain duplicates") + + true -> + detents end end - def normalize_sheet_detents!(_detents), do: invalid_sheet_detents!() + def normalize_sheet_detents!(detents), + do: invalid_sheet_detents!(detents, "must be a nonempty list") - defp invalid_sheet_detents! do - raise ArgumentError, - "Mob.UI.sheet :detents must be unique :medium/:large values or one " <> - ":content detent with an optional positive :max_height" + # Each failure names the rule it broke and echoes the offending value. A + # single catch-all message reads fine to whoever wrote the validation and + # badly to whoever tripped it — `[:medium, :medium]` should say + # "duplicates", not restate the whole grammar. + defp invalid_sheet_detents!(detents, reason) do + raise ArgumentError, "Mob.UI.sheet :detents #{reason}, got: #{inspect(detents)}" end defp validate_on_dismiss!(nil), do: :ok diff --git a/test/mob/native_box_accessibility_test.exs b/test/mob/native_box_accessibility_test.exs index f0f959ee..84a46392 100644 --- a/test/mob/native_box_accessibility_test.exs +++ b/test/mob/native_box_accessibility_test.exs @@ -25,10 +25,30 @@ defmodule Mob.NativeBoxAccessibilityTest do assert source =~ ".accessibilityElement(children: .ignore)" assert source =~ "view.accessibilityLabel(label)" - assert source =~ ~s|node.accessibilityRole == "button"| - assert source =~ ".accessibilityAddTraits(.isButton)" - assert source =~ ".accessibilityAddTraits(.isNotEnabled)" - assert source =~ ".allowsHitTesting(!node.disabled)" - assert source =~ ".ifLet(node.disabled ? nil : node.onTap)" + + # Collapse to one element for a label OR an explicit button role. Traits + # added without collapsing land on every descendant, so a role-only box + # would announce each nested Text as its own button. + assert source =~ "node.accessibilityLabel != nil || node.accessibilityRole == \"button\"" + assert source =~ "isAccessibilityControl ? () : nil" + + # Traits stay unconditional OptionSet modifiers. Branching on `disabled` + # would make it a _ConditionalContent boundary and tear down the subtree + # on every toggle, losing a wrapped TextField's text and focus. + assert source =~ + ~s|.accessibilityAddTraits(node.accessibilityRole == "button" ? .isButton : [])| + + assert source =~ ".accessibilityAddTraits(node.disabled ? .isNotEnabled : [])" + + # .disabled, not .allowsHitTesting: the latter makes the box transparent + # to touches, so a disabled backdrop would pass taps through to the + # content it is meant to be shielding. + assert source =~ ".disabled(node.disabled)" + refute source =~ ".allowsHitTesting(!node.disabled)" + + # Tap wiring branches on handler presence only; the disabled check is + # inside the closure so toggling it is not a structural change. + assert source =~ ".ifLet(node.onTap)" + assert source =~ "if !node.disabled { tap() }" end end diff --git a/test/mob/native_sheet_test.exs b/test/mob/native_sheet_test.exs index fe15c8b6..32e42d22 100644 --- a/test/mob/native_sheet_test.exs +++ b/test/mob/native_sheet_test.exs @@ -11,7 +11,10 @@ defmodule Mob.NativeSheetTest do assert source =~ "MobSheetContentHeightKey" assert source =~ "builtInDetents.contains(\"medium\")" assert source =~ "builtInDetents.contains(\"large\")" - assert source =~ ".height(limitedContentHeight)" + # Unmeasured content must fall back to a real system detent, never to a + # computed sentinel — .height(1) flashed a hairline on every presentation. + assert source =~ "guard let height = limitedContentHeight else { return [.medium] }" + assert source =~ ".height(height)" assert source =~ ".onPreferenceChange(MobSheetContentHeightKey.self)" assert source =~ "ScrollView(.vertical)" end @@ -20,10 +23,21 @@ defmodule Mob.NativeSheetTest do source = File.read!(Path.join(@ios, "MobRootView.swift")) assert source =~ "GeometryReader { geometry in" - assert source =~ "availableSheetHeight = max(1, height * 0.9)" - assert source =~ "min(intrinsicContentHeight, maximumHeight)" - assert source =~ "let intrinsicHeight = max(1, measuredHeight)" - assert source =~ "intrinsicContentHeight = intrinsicHeight" + + # Pin the mechanism, not the arithmetic. Asserting the literal + # `max(1, height * 0.9)` made naming the fraction a test failure even + # though behaviour was identical — these assertions should break when the + # re-clamp stops working, not when someone extracts a constant. + assert source =~ "onChange(of: geometry.size.height, initial: true)" + assert source =~ "availableSheetHeight = max(1, height * Self.sheetHeightCeilingFraction)" + assert source =~ "static let sheetHeightCeilingFraction" + assert source =~ ".environment(\\.mobAvailableSheetHeight, availableSheetHeight)" + + # The detent is total sheet height, so the content's own bottom safe-area + # inset has to be part of it or the last rows sit under the home indicator. + assert source =~ "metrics.height + metrics.bottomInset" + assert source =~ "safeAreaInsets.bottom" + assert source =~ "contentMetrics = measured" refute source =~ "UIScreen.main.bounds.height * 0.9" end end From 6d945171889f3c1321764f901527f3ddc264f876 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Thu, 27 Aug 2026 19:28:05 -0600 Subject: [PATCH 3/3] Fix two Swift compile errors and verify sheets on device MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Device verification found what no Elixir-side check could: PR #94 as submitted DID NOT BUILD for iOS. Two separate compile errors. 1. `AccessibilityTraits` has no member `.isNotEnabled`. The original `.accessibilityAddTraits(.isNotEnabled)` is not a real API — confirmed zero occurrences in the iOS 26.5 SwiftUI interface. Disabled state is not a trait you add; it is what `.disabled(true)` publishes to VoiceOver, which this code already does. Removed. 2. "The compiler is unable to type-check this expression in reasonable time." The accessibility and tap modifiers pushed MobBox's chain past the Swift type-inference budget. Extracted into a MobBoxSemantics ViewModifier — the same treatment MobBox itself already got from MobNodeView for the same reason. Worth dwelling on how #1 survived review: the grep test asserted `.accessibilityAddTraits(.isNotEnabled)` appears in the source, and it did. The string was present and the build was broken. `mix test`, `credo`, `mix format`, `clang-format`, `swiftlint`, and `clang -fsyntax-only` on mob_nif.m all pass on code that cannot compile, because none of them type-check Swift. Only a native build does. The test now refutes the call form. Device results (iPhone 17 Pro simulator, 874pt tall — chosen over the attached iPhone SE because the SE has no home indicator and so cannot exercise the safe-area fix at all): - short [:content], 3 rows: hugs content, rows at y=716..784. Sheet is sized to content, parent visible behind. - tall, max_height: 320, 40 rows: sheet top ~y=541, i.e. capped at ~320pt; content extends to y=1864 and scrolls internally. - tall [:content], 40 rows: sheet top ~y=147, capped near the 0.9 root ceiling (786pt); scrolls internally. - exact fit, 8 rows: all 8 visible, last row bottom clears the home indicator rather than sitting under it — the safe-area inset fix, confirmed visually. - Sampling element_frames every 120ms from presentation showed a stable layout from the first sample; no degenerate frame. The 1pt case is structurally gone now that measurement is Optional rather than a numeric sentinel. Co-Authored-By: Claude Opus 5 (1M context) --- ios/MobRootView.swift | 97 +++++++++++++--------- test/mob/native_box_accessibility_test.exs | 16 ++-- 2 files changed, 68 insertions(+), 45 deletions(-) diff --git a/ios/MobRootView.swift b/ios/MobRootView.swift index 49bfd500..26a20068 100644 --- a/ios/MobRootView.swift +++ b/ios/MobRootView.swift @@ -641,18 +641,65 @@ private struct MobFrameTracker: ViewModifier { // The fixed-width path is what makes circular ring cells possible without // a dedicated primitive — set width: N, height: N, corner_radius: N/2, // border_color + border_width and the box renders as a ring. -private struct MobBox: View { +// Tap wiring and accessibility semantics for a composite Box, kept out of +// MobBox's main modifier chain so the Swift type checker can cope. +private struct MobBoxSemantics: ViewModifier { let node: MobNode - /// True when the box is standing in for a control rather than plain - /// layout — it carries a label, or the caller asked for a button role. - /// Only then is it right to collapse the subtree into one accessibility - /// element; a passive box with labelled children must keep them visible - /// to VoiceOver. + /// True when the box stands in for a control rather than plain layout — + /// it carries a label, or the caller asked for a button role. Only then + /// is it right to collapse the subtree into one accessibility element; a + /// passive box with labelled children must keep them visible to VoiceOver. private var isAccessibilityControl: Bool { node.accessibilityLabel != nil || node.accessibilityRole == "button" } + // Only .isButton is set explicitly. SwiftUI's AccessibilityTraits has no + // .isNotEnabled member — the disabled trait is not something you add, it + // is what `.disabled(true)` below already publishes to VoiceOver. An + // explicit `.accessibilityAddTraits(.isNotEnabled)` does not compile. + private var traits: AccessibilityTraits { + node.accessibilityRole == "button" ? .isButton : [] + } + + func body(content: Content) -> some View { + content + // Branch on `onTap` presence only, never on `disabled`. `ifLet` is + // @ViewBuilder if/else, i.e. _ConditionalContent — flipping the + // branch gives SwiftUI a structurally different view and tears the + // subtree down. `disabled` is routinely toggled, so branching on it + // would drop a wrapped TextField's in-flight text and focus, and + // re-present a wrapped Sheet. The check moves inside the closure, + // where it costs nothing structurally. + .ifLet(node.onTap) { view, tap in + view.contentShape(Rectangle()).onTapGesture { + if !node.disabled { tap() } + } + } + // Collapse to a single accessibility element whenever this box is + // acting as a control. Traits added without collapsing land on + // every descendant instead, so a role-only box would announce each + // nested Text as its own button. + .ifLet(isAccessibilityControl ? () : nil) { view, _ in + view.accessibilityElement(children: .ignore) + } + .ifLet(node.accessibilityLabel) { view, label in + view.accessibilityLabel(label) + } + // One OptionSet, so no _ConditionalContent branch on `disabled`. + .accessibilityAddTraits(traits) + // .disabled, not .allowsHitTesting: allowsHitTesting(false) makes + // the view transparent to touches, so a disabled box used as a + // blocking overlay or dimmed backdrop would pass taps through to + // whatever sits behind it. .disabled blocks interaction in the + // subtree while still consuming the touch. + .disabled(node.disabled) + } +} + +private struct MobBox: View { + let node: MobNode + var body: some View { let alignment: Alignment = boxAlignmentFromString(node.boxAlign) @@ -692,39 +739,13 @@ private struct MobBox: View { lineWidth: node.borderWidth) .allowsHitTesting(false) ) - // Branch on `onTap` presence only, never on `disabled`. `ifLet` is - // @ViewBuilder if/else, i.e. _ConditionalContent — flipping the branch - // gives SwiftUI a structurally different view and tears the subtree - // down. `disabled` is a routinely toggled prop, so branching on it - // would drop a wrapped TextField's in-flight text and focus, and - // re-present a wrapped Sheet. The disabled check moves inside the - // closure, where it costs nothing structurally. - .ifLet(node.onTap) { view, tap in - view.contentShape(Rectangle()).onTapGesture { - if !node.disabled { tap() } - } - } .mobGestures(node) - // Collapse to a single accessibility element whenever this box is - // acting as a control — a label OR an explicit button role. Traits - // added without collapsing land on every descendant instead, so a - // role-only box would announce each nested Text as its own button. - .ifLet(isAccessibilityControl ? () : nil) { view, _ in - view.accessibilityElement(children: .ignore) - } - .ifLet(node.accessibilityLabel) { view, label in - view.accessibilityLabel(label) - } - // OptionSet-valued, so these stay unconditional modifiers and add no - // _ConditionalContent branch on `disabled`. - .accessibilityAddTraits(node.accessibilityRole == "button" ? .isButton : []) - .accessibilityAddTraits(node.disabled ? .isNotEnabled : []) - // .disabled, not .allowsHitTesting: allowsHitTesting(false) makes the - // view transparent to touches, so a disabled box used as a blocking - // overlay or dimmed backdrop would pass taps through to whatever sits - // behind it. .disabled blocks interaction in the subtree while still - // consuming the touch. - .disabled(node.disabled) + // Interaction + accessibility live in their own ViewModifier: folding + // them into this chain inline pushed it past SwiftUI's type-inference + // budget and the Swift build failed outright ("unable to type-check + // this expression in reasonable time"). Same reason MobBox itself was + // extracted from MobNodeView. + .modifier(MobBoxSemantics(node: node)) // (offset is applied uniformly by MobNodeView's body; not here) } } diff --git a/test/mob/native_box_accessibility_test.exs b/test/mob/native_box_accessibility_test.exs index 84a46392..394d37b2 100644 --- a/test/mob/native_box_accessibility_test.exs +++ b/test/mob/native_box_accessibility_test.exs @@ -30,15 +30,17 @@ defmodule Mob.NativeBoxAccessibilityTest do # added without collapsing land on every descendant, so a role-only box # would announce each nested Text as its own button. assert source =~ "node.accessibilityLabel != nil || node.accessibilityRole == \"button\"" - assert source =~ "isAccessibilityControl ? () : nil" - # Traits stay unconditional OptionSet modifiers. Branching on `disabled` - # would make it a _ConditionalContent boundary and tear down the subtree - # on every toggle, losing a wrapped TextField's text and focus. - assert source =~ - ~s|.accessibilityAddTraits(node.accessibilityRole == "button" ? .isButton : [])| + # Kept in its own ViewModifier: inlining these into MobBox's chain pushed + # it past the Swift type-inference budget and failed the native build, + # which no Elixir-side check can catch. + assert source =~ "struct MobBoxSemantics: ViewModifier" + assert source =~ "isAccessibilityControl ? () : nil" - assert source =~ ".accessibilityAddTraits(node.disabled ? .isNotEnabled : [])" + # Traits go on as one unconditional OptionSet modifier. Branching on + # `disabled` would make it a _ConditionalContent boundary and tear down the + # subtree on every toggle, losing a wrapped TextField's text and focus. + assert source =~ ".accessibilityAddTraits(traits)" # .disabled, not .allowsHitTesting: the latter makes the box transparent # to touches, so a disabled backdrop would pass taps through to the