Skip to content

Fix LogBox inspector content under horizontal safe area insets - #59797

Merged
huntie merged 1 commit into
mainfrom
pr59797
Nov 10, 2026
Merged

huntie merged 1 commit into
mainfrom
pr59797

Conversation

@huntie

@huntie huntie commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

The LogBox inspector applies the safe area to its header and footer buttons only, so the error message, code frames and stacks run under a side inset, such as the iPhone Duo's status column or the notch in landscape. This affects the "Failed to compile" screen, which cannot be dismissed, and the dismissable inspector alike.

Changes

  • Read the inspector root's insets with experimental_onSafeAreaInsetsChange, and apply the left and right insets as padding to the message header and to the content inside the body's ScrollView, so backgrounds stay edge to edge.
  • Map the physical insets to paddingStart and paddingEnd by isRTL, so the RTL left/right swap doesn't move them.

This adds no SafeAreaView use, ahead of its removal.

Changelog:
[General][Fixed] - LogBox: Keep the inspector's message, code frames and stacks clear of horizontal safe area insets

Test Plan

yarn test packages/react-native/Libraries/LogBox

✅ Passes, with no snapshot changes.

RNTester on the iPhone Duo simulator (iOS 27.1 runtime, Xcode 27.2), folded display, where the status column is a right inset. The syntax error is an unresolvable import added while the app runs, delivered by Fast Refresh; the render error is a throw in RNTesterApp.

Syntax error, before Syntax error, after

✅ The message and code frame wrap before the status column; the header and message backgrounds still reach the edge.

Render error, before Render error, after

✅ The dismissable inspector gets the same inset; the header and Dismiss, Minimize and Copy buttons are unchanged.

Render error, "See More" expanded Expanded and scrolled

✅ With the message inside the ScrollView, it and the component stack keep the inset while scrolling.

iPhone 18 Pro (iOS 27.0 runtime), portrait, syntax error, after

✅ With no side insets, the layout is unchanged.

ℹ️ Not run locally: the Fantom LogBox snapshot tests (yarn fantom), left to CI.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 4, 2026
@huntie
huntie marked this pull request as ready for review November 4, 2026 13:50
@facebook-github-tools facebook-github-tools Bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Nov 4, 2026
@huntie

huntie commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

@huntie-bot Once this lands, pick request it to both 88 and 89.

Comment on lines +66 to +73
const {left, right} = event.nativeEvent.insets;
// Insets are physical edges. Logical edges resolve by layout direction
// alone, which keeps clear of the RTL left/right swap.
const {isRTL} = I18nManager.getConstants();
setSafeAreaPadding({
paddingStart: isRTL ? right : left,
paddingEnd: isRTL ? left : right,
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we use paddingLeft and paddingRight here to avoid the manual RTL logic?

@huntie huntie Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested this on the iPhone Duo simulator in RTL, I think the manual RTL logic is needed regardless (for as long as doLeftAndRightSwapInRTL exists).

RTL Current (paddingStart/End + isRTL) paddingLeft/paddingRight
doLeftAndRightSwapInRTL: true (default)
doLeftAndRightSwapInRTL: false

This mapping also holds with doLeftAndRightSwapInRTL: false, since paddingStart/End resolve by layout direction alone (verified on device), so I'll keep it as is.

I guess Left/Right might be a semantic improvement, but I don't think it reduces the logic, unless you had something else in mind?

@robhogan

robhogan commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator

@huntie-bot Once this lands, pick request it to both 88 and 89.

0.88 doesn't have experimental_onSafeAreaInsetsChange (it was added in #58109 and never picked)

@huntie

huntie commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

@robhogan Gah, yeah you're right.

@huntie-bot Amend: Pick this to only 0.89 please.

@huntie
huntie enabled auto-merge (squash) November 10, 2026 12:08
@huntie
huntie disabled auto-merge November 10, 2026 12:12
@huntie
huntie enabled auto-merge (squash) November 10, 2026 12:12
@huntie
huntie merged commit 975584c into main Nov 10, 2026
210 of 212 checks passed
@huntie
huntie deleted the pr59797 branch November 10, 2026 14:46
rubennorte added a commit to rubennorte/react-native that referenced this pull request Nov 10, 2026
## Summary:

react#59797 added `experimental_onSafeAreaInsetsChange` to the root `logbox_inspector` view, and react#59893 refreshed the LogBox snapshots without that change, so both landed but the committed `LogBoxInspector` baseline is missing the prop. The two inspector snapshot tests fail in Fantom CI mode, which no longer rewrites mismatched baselines (react#59895).

Regenerate the two affected snapshots; the only change is the added prop on the root view.

## Changelog:

[Internal]

## Test Plan:

- `CI=true yarn fantom packages/react-native/Libraries/LogBox`: 2 snapshot failures before; 90/90 tests and 54/54 snapshots pass after, on two consecutive runs.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
rubennorte added a commit that referenced this pull request Nov 10, 2026
…59911)

* Force native EventTarget event dispatching on for the React renderer

## Summary:

#59896 removed `enableNativeEventTargetEventDispatching` from `ReactNativeFeatureFlags`, but the vendored Fabric renderer still reads it through `react-native/react-private-interface` to choose between `dispatchNativeEvent` and the legacy plugin-based dispatch. With the getter gone, the renderer saw `undefined`, treated the flag as off, and fell back to the legacy path, so listeners registered with `addEventListener` were never called.

The `ReactNativeFeatureFlags` getter in the private interface now returns the real flags plus `enableNativeEventTargetEventDispatching: () => true`, so only the renderer sees it. This can be removed once the renderer is synced from a React build that no longer reads the flag.

## Changelog:

[Internal]

## Test Plan:

- `yarn fantom packages/react-native/src/private/renderer/core/__tests__/EventTargetDispatching-itest.js`: 16 failures before, 78 passed / 20 skipped after.
- `yarn fantom packages/react-native/src/private/webapis/dom packages/react-native/src/private/renderer`: 513 passed, 26 skipped.
- `yarn eslint --max-warnings 0` and `yarn prettier --check` on both changed files pass.
- `yarn flow-check` reports only 2 unrelated parse errors from local git ref logs under `.git/logs/refs/remotes/` whose names end in `.json`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

* Fix build_js_types: move abortSignal's required param first

Summary:
#59896 made `ReactNativeElement` unconditionally extend `EventTarget`, which
imports the `AbortSignal` type, pulling `AbortSignal.d.ts` into the generated
types checked by `yarn test-generated-typescript`. The internal
`abortSignal(reason = ..., signal)` helper declares a defaulted parameter before
a required one, which flow-api-translator emits as
`abortSignal(reason?: unknown | void, signal: AbortSignal)` and tsc rejects with
TS1016.

Reorder the parameters to `abortSignal(signal, reason = ...)` and update both
internal callers. The public API snapshot is unchanged.

Changelog: [Internal]

Test Plan:
- `yarn build-types --skip-snapshot && yarn test-generated-typescript`: fails
  with TS1016 before, passes after
- `yarn build-types --validate`: API snapshot is up to date
- `yarn eslint --max-warnings 0 packages/react-native/src/private/webapis/dom/abort-api/`

* Update LogBoxInspector snapshots for the safe area insets listener

## Summary:

#59797 added `experimental_onSafeAreaInsetsChange` to the root `logbox_inspector` view, and #59893 refreshed the LogBox snapshots without that change, so both landed but the committed `LogBoxInspector` baseline is missing the prop. The two inspector snapshot tests fail in Fantom CI mode, which no longer rewrites mismatched baselines (#59895).

Regenerate the two affected snapshots; the only change is the added prop on the root view.

## Changelog:

[Internal]

## Test Plan:

- `CI=true yarn fantom packages/react-native/Libraries/LogBox`: 2 snapshot failures before; 90/90 tests and 54/54 snapshots pass after, on two consecutive runs.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Expo Partner: Expo Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants