fix(ui): deduplicate shadow ancestor events by abharms · Pull Request #439 · youversion/platform-sdk-react

abharms · 2026-10-07T22:23:17Z

Summary

deduplicate React light-DOM ancestor delivery when a composed click, keydown, or focus event crosses an automatic SDK shadow boundary
preserve component callback targets and native propagation, retargeting, and composedPath() behavior
retain separate delivery for distinct events, redispatch of the same native event object, and events originating on the host itself
document the validated consumer contract and the actual-Safari follow-up in YPE-5952

Requirement evidence

Requirement	Evidence
`onClick`, `onKeyDown`, and `onFocus` reach a light-DOM React ancestor once per native event	The consumer-compatibility Storybook workflow records each React invocation against its native event and asserts exact per-event delivery.
Distinct events and redispatched native event objects remain distinct deliveries	Two user clicks retain distinct native identities; one `KeyboardEvent` dispatched twice produces two component and ancestor observations.
Component callbacks keep their targets	The workflow asserts `target` and `currentTarget` for click, keydown, and focus callbacks.
Native behavior remains intact	Native listeners still receive the events with host retargeting and the original internal node at the start of `composedPath()`.
Focus and nested roots are covered	The workflow covers React `onFocus`/native `focusin`; the nested-root story asserts one React click delivery at each scope and native retargeting across both boundaries.
Shared-host fix with no public interface change	The change is confined to internal `ShadowRootHost` event handling; no exported prop or type changes.
Cross-browser workflow	The changed story passes in Chromium, Firefox, and Playwright WebKit. Actual Safari remains assigned to YPE-5952.

Review scope

Required outcomes:

one light-DOM React ancestor delivery for the validated bubbling click, keydown, and React focus/native focusin paths
unchanged native event delivery and retargeting
unchanged component callback targets
separate delivery for distinct native dispatches

Permitted support work:

one shared-host implementation change
one consolidated Storybook compatibility workflow
compatibility and rollout documentation

Non-goals:

a generic event framework or arbitrary event types
consumer-created shadow roots
per-component duplicate event suites
unrelated portal, focus, or overlay refactors
assistive-technology claims or actual-Safari validation

Please treat a finding as blocking only when it identifies an unmet in-scope requirement, a documented repository-standard violation in added or modified code, or a concrete regression or defect caused or worsened by this diff. Label other valid improvements as non-blocking follow-ups.

Verification

pnpm --filter @youversion/platform-react-ui exec vitest run --project unit --maxWorkers=1 --fileParallelism=false — 52 files, 632 tests passed
pnpm typecheck
pnpm lint
pnpm turbo build --force
Chromium Storybook workflow — 3/3 passed
Firefox focused event workflow — passed
Playwright WebKit focused event workflow — passed
mutation checks confirmed that event-instance caching would incorrectly suppress redispatch and host-origin events

The default parallel local pnpm test run encountered eight unrelated 5-second timeout-only failures in reader, popover, search, and version-picker tests after PR #433 merged. Every timed-out test passed in the green serial UI run above; core and hooks were green in the original run. CI remains the authoritative parallel run.

Review audits

Standards: no required findings
Spec: every atomic YPE-6040 requirement has direct evidence; no required findings
Compatibility: no unintended behavioral change found; no required findings

Confidence Score: 5/5

The changes since the last review appear safe to merge.

What we checked:

Safari checks remain required: No. Both documents still require actual Safari checks before the coordinated release.

Summary

The PR prevents duplicate React ancestor calls for bubbling click, keydown, and focusin events across SDK shadow roots. It adds Storybook checks for callback targets, native delivery, repeated events, and nested roots.

Changes since the last review only clarify documentation.
Remaining duplicate delivery is explicitly documented.
Actual Safari checks remain required before the coordinated release.
No new actionable issues were found.

Diagram

sequenceDiagram
  participant Control as Internal control
  participant Shadow as Shadow-root React listener
  participant Ancestor as React ancestor
  participant Host as Shadow host
  participant Native as External native listener
  Control->>Shadow: Native event
  Shadow->>Ancestor: First React traversal
  Control->>Host: Native event crosses boundary
  Host->>Host: Stop duplicate React traversal
  Host->>Native: Native delivery continues

_{Reviews (7) · Last reviewed commit: "docs: clarify shadow event rollout dispo..." · Reviewed by Greptile}

changeset-bot · 2026-10-07T22:23:21Z

🦋 Changeset detected

Latest commit: 306c26a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 0 packages

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

chatgpt-codex-connector · 2026-10-07T22:23:34Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-08T15:47:15.041531Z	`85b1321`	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

…adow-events

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9f38af0868

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-10-08T14:06:04Z

+    button.dispatchEvent(keyDownEvent);
+
+    await expect(outsideKeyDowns).toHaveLength(2);
+    await expect(new Set(outsideKeyDowns.map(({ event }) => event))).toHaveLength(1);


Compare Set size instead of length

When this story runs in either Storybook test job from .github/workflows/storybook.yml, this assertion fails regardless of event behavior: a Set exposes .size, not .length, while Vitest's toHaveLength requires a numeric .length property. Compare the set's size with toBe(1) instead, and make the same correction to the click-event assertion below.

Useful? React with 👍 / 👎.

cameronapak

Review

YPE-6040

Summary

Standards: 0 must-fix. Spec: 0 must-fix. Primary concern: none.

Review evidence

Scope: reviewed revision 85b1321, YPE-6040, the shared host implementation, consumer workflows, documentation, and existing review threads.
Method: traced React and native event delivery for click, keydown, focus, repeated event objects, host-origin events, and nested roots.

Behavior or check	Method / command	Result	Evidence source
Cross-browser consumer workflow	Storybook CI	Chromium, Firefox, and WebKit passed	CI run
Build, lint, types, tests, and bundle size	CI	Passed	CI run
`Set.toHaveLength` concern	Inspected and executed Storybook matcher	Valid: Chai reads `Set.size`; both assertions passed	Coordinator

Limits: actual Safari remains intentionally deferred to YPE-5952. Capture-phase and other event types are documented non-goals.
CI and bot review: all required checks are green. The unresolved Set.toHaveLength bot comment is not actionable.
Event: APPROVE.

Written by Code Reviewer bot on behalf of Cam.

cameronapak · 2026-10-08T16:45:37Z

+    const nativeEvent = event.nativeEvent;
+    const host = event.currentTarget;
+    const origin = nativeEvent.composedPath()[0];
+    const isRetargetedHostTraversal = nativeEvent.target === host && origin !== host;


praise: This predicate narrowly identifies the retargeted second React traversal while preserving host-origin events and native propagation.

For Agents: precise traversal boundary

Comparing the retargeted native target with the original composed-path node avoids caching event objects or suppressing redispatch. The cross-browser workflow covers repeated native objects, distinct clicks, host-origin key events, and nested roots.

Written by Code Reviewer bot on behalf of Cam.

abharms added 5 commits October 7, 2026 17:07

fix(ui): deduplicate shadow ancestor events

cc7ef06

test(ui): simplify shadow event evidence

c069f26

fix(ui): preserve repeated native event dispatch

8ec688b

fix(ui): identify duplicate shadow traversal

f328a84

test(ui): prove shadow delivery per event

b1939af

abharms added 2 commits October 8, 2026 08:32

docs: clarify remaining shadow event duplication

e9ddf25

Merge branch 'journey-to-the-shadow-dom' into ype-6040-deduplicate-sh…

9f38af0

…adow-events

chatgpt-codex-connector Bot reviewed Oct 8, 2026

View reviewed changes

abharms added 4 commits October 8, 2026 09:15

test(ui): allow slower browser story readiness

88f7c3f

test(ui): wait for picker data before browser interaction

6d393ef

test(ui): isolate picker browser fixtures

306c26a

docs: clarify shadow event rollout disposition

85b1321

cameronapak approved these changes Oct 8, 2026

View reviewed changes

abharms merged commit caaf04f into journey-to-the-shadow-dom Oct 8, 2026
23 checks passed

abharms deleted the ype-6040-deduplicate-shadow-events branch October 8, 2026 17:11

Navigation Menu

Repository navigation

fix(ui): deduplicate shadow ancestor events - #439

fix(ui): deduplicate shadow ancestor events#439
abharms merged 11 commits into
journey-to-the-shadow-domfrom
ype-6040-deduplicate-shadow-events

abharms commented Oct 7, 2026 •

edited by greptile-apps Bot

Loading

Uh oh!

changeset-bot Bot commented Oct 7, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot commented Oct 7, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Oct 8, 2026

Uh oh!

cameronapak left a comment

Uh oh!

cameronapak Oct 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

abharms commented Oct 7, 2026 • edited by greptile-apps Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Requirement evidence

Review scope

Verification

Review audits

Confidence Score: 5/5

Summary

Diagram

Uh oh!

changeset-bot Bot commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

chatgpt-codex-connector Bot commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

cameronapak left a comment

Choose a reason for hiding this comment

Review

Summary

Uh oh!

cameronapak Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

abharms commented Oct 7, 2026 •

edited by greptile-apps Bot

Loading

changeset-bot Bot commented Oct 7, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Oct 7, 2026 •

edited

Loading