ReviewOS

craft-native/craft

feat(native): support identity-preserving non-modal popup presentation / reparenting

#382
Closed glennmichael123 opened this 9 days ago ·
9 days ago

Downstream unblock: stacksjs/stx#2089

Craft #373 supplies focus and measurement, but shared non-modal Tooltip/Popover still needs a presentation surface outside clipping ancestors, without replacing live controls or turning the popup into a modal. Craft implementation belongs to the separate Craft agent; no Craft source was changed for this report.

Verified evidence

Inspected Craft main 09a3d61c3334ce033ed418a093d3e17d83b4da6d:

  • iOS mutation contract: insertChild rejects an attached child with NODE_ATTACHED; moveChild requires the child to already belong to the destination parent, otherwise NOT_A_CHILD.
  • Android mutation contract enforces the same restrictions.
  • removeNode removes the entire subtree, so remove/create is not an identity-preserving detach/reattach operation.
  • iOS child reconciliation looks up retained children under the previous parent and explicitly documents parent-local keys. A full RENDER is therefore not evidence that cross-parent native view identity, drafts, selection, and focus survive.

Minimal topology: root View -> clipped/scrolling owner -> popup View containing a TextInput. Moving the existing popup to the screen overlay layer with insertChild(root, popup) is rejected; moveChild(root, popup) is rejected too. Recreating it is not equivalent for interactive Popover content.

An STX presentation-only portal prototype passed 49 tests because our mock detached/reinserted any child implicitly. We withdrew that prototype and are tightening the mock. This is source-verified contract evidence, not a simulator/device reproduction.

Required outcome (implementation choice belongs to Craft)

Provide an advertised host operation or portal/presentation primitive that can present an existing subtree above clipping/scroll ancestors while preserving its live native identity. An additive atomic reparent operation plus identity-preserving renderer reconciliation is one option; a host-owned presentation layer retaining logical ownership is another.

Please specify:

  1. Request/event schema and capability flag. Older hosts must fail explicitly rather than silently recreating content.
  2. Screen-viewport coordinate semantics consistent with View.measure, including safe areas and scrolling. A popup inside a Modal belongs above that modal, not behind it.
  3. Non-modal hit testing: popup controls work, opening does not trap keyboard/accessibility focus or block the whole screen, and outside touches/back/escape can be handled by the top-layer dismissal policy without also dismissing an underlying modal.
  4. Stable IDs, handlers, uncontrolled input drafts, caret/selection, and focus across opening, updating, restoring and closing. Prevent cycles and stale/unmounted references atomically.
  5. Teardown of either the logical owner or presentation target must not leave an orphan visible or release a surviving control accidentally.

Acceptance / coordination

  • Maintained iOS and Android tests exercise a popup originating inside a ScrollView/clipped parent, nested-modal stacking, repeated open/close, outside dismissal, and removal while open.
  • Pin actual native control identity/state, not only the serialized tree or source strings.
  • Report which native platforms were actually run, commit(s), and the released version floor (or explicitly unreleased).
  • STX owns shared component behavior, async measurement/placement, logical scope/event/disposal ownership, and adapter integration. Craft owns actual native presentation and identity preservation. No Craft implementation is requested from the STX agent.
9 days ago

Initial Craft slice landed on main:

  • reparentChild is now an atomic, versioned mutation on iOS and Android. It validates attachment, destination capacity, cycles, same-parent no-ops, and reports both affected parents.
  • Cross-parent mutations force a committed-tree reconciliation. iOS now indexes the previous render by protocol ID and preserves the live native view when a node changes parents; Android prefers the global protocol ID for the same reason. Drafts, focus, selection, handlers, and native control identity stay attached to the existing view.
  • Native mutation fixtures and the host-neutral contract cover the operation.

Commits: b59f5f24, ef4db21d (rebased/pushed as c0a26c7c and its parent). The remaining #382 work is the actual non-modal presentation layer: viewport coordinates, clipping-safe overlay ownership, dismissal/hit-testing, capability negotiation, and nested-modal teardown. No simulator/emulator run was claimed here.

9 days ago

Follow-up on main: 8004313e advertises viewReparent in both native capability lists so the shared runtime can gate the new reparentChild operation instead of probing an older host. The operation remains explicit and versioned; older hosts reject it as an unknown mutation rather than silently recreating the popup.

9 days ago

Follow-up coverage landed on main in ca20f1c2.

  • Android mutation fixture now verifies a reparented popup retains its nested field, authored keys, and draft props when moving from a ScrollView into a Modal.
  • iOS host test now moves a live TextInput out of a clipped/scrolling parent while preserving native identity, caret selection, and the source scroll offset.
  • The stale HealthKit contract assertion was corrected; the full host-neutral contract is green (297 tests).
  • iOS simulator validation passed (178 unit tests, 4 UI tests).

The actual non-modal presentation surface and viewport/dismissal semantics remain the next implementation slice.

9 days ago

Android overlay teardown slice landed on main in f94ab8bb.

  • Released native views are now detached from their actual parent during teardown, including Modals promoted to the screen overlay.
  • iOS and Android navigation fixtures now mount a modal, reopen it, unmount it while presented, and assert the overlay disappears.
  • Typecheck, focused host-neutral contract tests (6 pass), and both iOS/Android fixture generation passed.
  • The iOS generated project compiled successfully; the simulator runner then hung in Xcode/CoreSimulator before producing a result bundle (no test result was claimed). No Android device was connected for instrumentation.

The broader non-modal popup presentation surface in #382 remains open.

9 days ago

Follow-up landed on main in 24daedc0.

  • Native Modal overlays now accept nonModal: true; existing modal behavior remains blocking by default.
  • iOS hit testing and Android touch dispatch let taps outside interactive popup content reach the underlying screen, while popup controls remain interactive.
  • Added host-neutral contract assertions and an iOS hit-test regression fixture.
  • Focused contract test, Pickier, and both native-navigation prepare-only builds pass.

This gives #382 a host-side non-blocking overlay mode; viewport placement, presentation request schema, and shared STX portal ownership remain to be integrated.

9 days ago

Implemented and pushed to main in 646bb8ac (fix(native): place non-modal overlays in screen viewport).

  • iOS now resolves non-modal overlay position:absolute coordinates against the safe-area viewport while keeping blocking modals full-screen.
  • Android now carries modal styles to the screen root, promotes nested modals out of clipped/scrolling parents, and uses authored viewport dimensions for non-modal overlays.
  • Added the iOS viewport-frame regression test and cross-host contract assertions.

Deterministic checks passed: bun run test:sdk (746 tests), bun run typecheck, targeted modal contract (62 assertions), Pickier, and both native fixture preparation scripts. The focused XCTest compiled the app and test bundles, but simulator launch hung in Xcode's install/launch worker before executing the test.

9 days ago

Implemented and pushed to main in 68428f90.

  • Added typed ModalProps.nonModal and dismissOnOutsideTap.
  • Both maintained hosts preserve the existing pass-through non-modal default; the opt-in consumes only an outside tap and emits the existing onRequestClose reason.
  • iOS root hit-testing and Android root touch dispatch cover promoted popups outside clipped/scrolling parents.
  • Teardown resets the dismissal policy, so released overlays cannot retain it.
  • Host-neutral contract: 298 tests / 3,067 assertions; SDK suite: 746 tests / 0 failures; typecheck passed.
  • Scoped Pickier passed; repository-wide Pickier was blocked by a Bun 1.3.1 segmentation fault.
9 days ago

Follow-up fixture coverage is now on main in 8457435d. The maintained STX navigation fixture now opens a promoted non-modal popup and dismisses it with an outside tap on both iOS UI tests and Android instrumentation; the host-neutral contract asserts both fixtures. Both --prepare-only builds passed. Full simulator/emulator execution remains CI/device-dependent.

9 days ago

Follow-up fix in c40f8c98: iOS screen-root hit-testing now promotes outside points for a dismissible popup even when no blocking modal is underneath. This closes the small-frame UIKit hit-test gap found during fixture review. Host-neutral contract remains green at 299 tests / 3,076 assertions.

9 days ago

STX integration follow-up: the new nonModal/dismissOnOutsideTap and viewport placement behavior is useful, but current hosts advertise viewFocus/viewMeasure/viewReparent only. None identifies the newer non-modal presentation contract (viewReparent predates it). Please add a dedicated capability or a documented minimum host/protocol version that STX can reliably inspect before opting in; otherwise an older host can silently show a blocking Modal. Please also document viewport/scroll invalidation notifications for repositioning anchored popups. Craft implementation remains with the separate Craft agent; no Craft code changed by STX.

9 days ago

Implemented and pushed as e094f39 (test(native): cover nested popup dismissal).

The maintained iOS and Android navigation fixtures now mount a non-modal popup inside a modal, dismiss it via an outside tap, assert the nested request-close reason, verify the nested node is removed, and verify the parent modal remains visible. The renderer contract asserts the fixture and both platform tests.

Validation: 299 native renderer contract tests passed; Pickier and git diff --check passed; iOS focused simulator test testNestedNonModalPopupDismissesAboveModal passed (1 test, 0 failures); iOS and Android fixture preparation passed.

9 days ago

Added the compatibility gate in 42e58f2 (feat(native): advertise popup presentation capability).

Both hosts now advertise viewNonModalPresentation separately from viewReparent; the native STX guide documents gating nonModal + dismissOnOutsideTap on this capability and the older-host blocking-modal fallback. The contract suite asserts both advertisements. Validation: 299 contract tests / 3,087 assertions, scoped Pickier, and git diff --check passed.

9 days ago

iOS Back now follows Android’s top-layer policy in 00d74da: the generated native navigation controller asks the visible screen to dismiss its top blocking or dismissible modal with reason back before UIKit pops the route. Added generator coverage; iOS builder suite and native-navigation prepare-only both pass. #382 remains open for the shared STX portal/placement integration.

9 days ago

STX Tooltip integration found a coordinate-contract blocker (source-verified at Craft d201ab4; no Craft edits or device claim): iOS View.measure converts bounds to the full controller view and returns view.bounds.width/height with coordinateSpace=screenViewport (CraftNativeScreen.swift around 6887-6900). But positionModals resolves non-modal left/top against view.bounds.inset(by: safe-area/navigation insets), then adds viewport.minX/minY (around 5362-5389). These are different coordinate origins. Example: with an 88-point top inset, a desired measured screen y=152 is presented at y=240 if STX uses top:152. The JS measurement response exposes no inset/origin with which to convert reliably. Please make non-modal authored coordinates match View.measure, or expose and document the presentation viewport origin/bounds so STX can convert and clamp correctly, with a nonzero navigation/safe-area regression. Also verify the corresponding Android padding/origin case. I am holding automatic anchored placement rather than shipping a guessed offset. Capability gating and low-level presentation are already integrated in STX ba231a9/fa8a407; shared Tooltip/Popover still await this placement contract.

9 days ago

Added real iOS UI coverage in b5d3956: the pushed Details route now presents a native modal, and testNavigationBackDismissesDetailsModalBeforePoppingRoute presses UIKit Back to verify the modal receives back while the route remains mounted. Simulator execution passed (1 test, 0 failures).

9 days ago

Added Android parity for pushed-route modal Back handling in commit 63fe0c0. The native-navigation fixture now opens Details, presents its modal, dispatches system Back, asserts the modal handler receives back, and confirms the route remains mounted. The README and cross-host contract test document the behavior. Local contract/package/fixture-preparation checks pass; emulator execution is covered by Mobile E2E.

9 days ago

Pushed 3f1d50e0 (fix(native): align popup viewport coordinates). The iOS host now resolves non-modal left/top against the full screenViewport, matching View.measure; safe-area/navigation offsets are no longer silently added twice. Android and both README contracts now document the same origin, with a renderer-contract assertion. Validation: 300 contract tests, iOS/Android generator suites, both native-navigation prepare-only fixtures, targeted Pickier, and diff checks pass. Mobile emulator/device execution remains CI-gated.

9 days ago

Added follow-up test coverage in 8393c40. The maintained iOS and Android navigation fixtures now assert that non-modal popup placement uses the measured screenViewport origin rather than applying a second safe-area offset; the renderer contract test requires both assertions. Local contract suite: 300 pass; iOS/Android native-navigation prepare-only and Pickier pass. Device/simulator execution remains covered by the mobile CI gate.

9 days ago

Documented the remaining placement invalidation contract in 1b39ba1. The shared native guide and both host READMEs now require anchored overlays to remeasure after anchor/screen-root onLayout and ancestor onScroll events, then clamp against the measured screenViewport dimensions. The contract test pins this guidance across hosts; 300 tests and scoped Pickier pass.

9 days ago

Added focused iOS identity coverage in 417a5ac (test(ios): retain popup input identity across reparenting). The maintained NativeRenderUnitTests fixture now renders a non-modal popup inside a ScrollView, captures a live UITextField with draft text and caret, then renders the popup as a screen-level sibling. The test asserts the same UITextField instance survives with its text and selection intact.

Validation: host-neutral native-renderer contract 300 passed / 0 failed / 3,109 assertions; focused generated iOS simulator XCTest passed (1 test, 0 failures); scoped Pickier and git diff --check passed. This covers the iOS native identity acceptance slice; Android device coverage remains in the mobile gate.

9 days ago

Added the Android half of the protocol-identity assertion in eb656b1 (test(android): pin popup protocol alias retention). The Kotlin fixture now verifies that a reparented control remains registered under its moving node ID and that a replacement cannot steal that alias, regardless of traversal order.

Validation: Android retained-identity fixture passed; host-neutral contract suite 301 passed / 0 failed / 3,112 assertions. Together with 417a5ac, both maintained hosts now pin identity/state preservation at the renderer contract boundary.

9 days ago

Fixed a generated Android fixture compile hazard in 58ca10c (test(android): avoid duplicate popup root binding). The nested and top-level popup outside-tap checks now use a scoped popupRoot binding instead of redeclaring root in the same test. STX bundle generation and the host-neutral contract pass locally; the Android device run is CI-gated because this machine has no Android SDK/adb.

9 days ago

Added and pushed adb7b8fc on main: iOS now has a simulator-backed regression covering three non-modal popup open/close reparenting cycles, preserving the same UITextField instance, draft text, and caret selection, then asserting removal leaves no orphan field. The host-neutral contract test requires the fixture. Focused simulator test passed (1 test, 0 failures). Android device/emulator execution remains CI-gated.

9 days ago

With adb7b8fc, the remaining Craft-side acceptance is now covered on both maintained hosts: iOS repeats three open/close reparenting cycles and verifies live UITextField identity, draft, caret, and removal; Android already covers nested stacking, outside dismissal, repeated toggles, and unmount teardown. The unresolved portion is the shared STX portal/placement integration, not another native-host behavior gap.

9 days ago

The shared STX integration is now verified against the exact compiler pin used by Craft Mobile E2E (1a89af00b1db3c75948d826b4fab4234f236731f). STX owns the portal policy and gates nonModal presentation on viewNonModalPresentation; its overlay tests pass 8/8. Craft’s host contract verifies viewport-origin placement, outside dismissal, reparenting identity, and teardown on both maintained hosts. The current Craft pin is ec37e35f. Closing this issue as complete; any future overlay behavior should be filed as a focused regression.

Sign in to comment on this issue.