fix: Fix onLayout callback not being triggered due to premature lastLayoutRef initialization (#423)#425
Conversation
…mature lastlayoutref initialization (#423) The issue occurs because the `lastLayoutRef` is initialized with the correct dimensions during the `useLayoutEffect`, but then the `onLayout` callback is immediately called with the same dimensions, causing it to be skipped due to the dimension comparison check. This breaks the expected behavior where `onLayout` should be called when the layout actually changes. Signed-off-by: Zendy <50132805+zendy199x@users.noreply.github.com>
…mature lastlayoutref initialization (#423) The issue occurs because the `lastLayoutRef` is initialized with the correct dimensions during the `useLayoutEffect`, but then the `onLayout` callback is immediately called with the same dimensions, causing it to be skipped due to the dimension comparison check. This breaks the expected behavior where `onLayout` should be called when the layout actually changes. Signed-off-by: Zendy <50132805+zendy199x@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR intends to fix useOnLayoutSync.native so consumer onLayout callbacks are not skipped on the first real layout event (reported in #423), by adjusting how initial layout state is tracked and deduped.
Changes:
- Updates
src/hooks/useOnLayoutSync.native.tsxto add layout deduping and an “initial layout” setup step. - Adds a new test suite
__tests__/useOnLayoutSync.native.test.tsxto validate initial layout handling and dedupe behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
src/hooks/useOnLayoutSync.native.tsx |
Changes how layout events are compared against prior layout state before calling onLayoutChange. |
__tests__/useOnLayoutSync.native.test.tsx |
Adds tests intended to cover initial layout behavior, deduping, and null/missing ref edge cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa71b40b64
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| onLayoutChange: (rectangle: { width: number; height: number }, fromLayoutEffect: boolean) => void; | ||
| ref: React.RefObject<any>; | ||
| } |
There was a problem hiding this comment.
This native-specific hook removes onLayoutProp from its input contract, so it can no longer forward consumer onLayout callbacks. LegendList still passes onLayoutProp into useOnLayoutSync (src/components/LegendList.tsx:441-444), which means user-provided onLayout handlers stop firing on native while continuing to work on non-native builds.
Useful? React with 👍 / 👎.
| lastLayoutRef.current = { width: layout.width, height: layout.height }; | ||
| didLayoutRef.current = true; |
There was a problem hiding this comment.
The effect caches the initial width/height in lastLayoutRef without notifying onLayoutChange. If the first onLayout event reports the same dimensions (the common path described in this fix), the dedupe check at line 22 suppresses the callback and initial layout never reaches downstream logic (for example list sizing in handleLayout).
Useful? React with 👍 / 👎.
| didLayoutRef.current = true; | ||
| } | ||
| } | ||
| }, [ref]); |
There was a problem hiding this comment.
This native implementation only tracks [ref], and it dropped the deps argument that existing callers rely on. Container passes [itemKey, layoutRenderCount] (src/components/Container.tsx:142-148) to force re-measurement for recycled cells and state-driven relayouts; with this code, those changes no longer retrigger sync, so item size updates can be missed when onLayout is not re-fired.
Useful? React with 👍 / 👎.
Signed-off-by: Zendy <50132805+zendy199x@users.noreply.github.com>
Signed-off-by: Zendy <50132805+zendy199x@users.noreply.github.com>
Signed-off-by: Zendy <50132805+zendy199x@users.noreply.github.com>
- update src/hooks/useOnLayoutSync.native.tsx - address 2 review comment(s) Signed-off-by: Zendy <50132805+zendy199x@users.noreply.github.com>
Fixes #423
Summary
The issue occurs because the
lastLayoutRefis initialized with the correct dimensions during theuseLayoutEffect, but then theonLayoutcallback is immediately called with the same dimensions, causing it to be skipped due to the dimension comparison check. This breaks the expected behavior whereonLayoutshould be called when the layout actually changes.Changes
src/hooks/useOnLayoutSync.native.tsxModify the logic to ensure that
onLayoutis called when the layout actually changes, rather than being skipped due to the initialuseLayoutEffectsetting the ref. The fix should separate the initial layout setting from the actual callback triggering, ensuring that the callback is only skipped when there's a genuine duplicate layout event.Testing