fix(layout): correct isLayoutCalculated always returning true#2642
Merged
gorhom merged 2 commits intogorhom:masterfrom Apr 8, 2026
Merged
fix(layout): correct isLayoutCalculated always returning true#2642gorhom merged 2 commits intogorhom:masterfrom
gorhom merged 2 commits intogorhom:masterfrom
Conversation
TwanLuttik
added a commit
to TwanLuttik/react-native-bottom-sheet
that referenced
this pull request
Apr 8, 2026
- Fix gorhom#2642: isLayoutCalculated logic bug (|| should be &&) causing incorrect detent positions - Fix gorhom#2498: prevent closing two modals at once by guarding splice with sheetIndex !== -1 - Fix gorhom#2342: use Math.floor on animatedIndex to fix Android freeze with reduced animations - Fix gorhom#2595: React 19 compatibility and type safety for findNodeHandle web utility
The `isLayoutCalculated` derived value uses an OR condition to check
whether `containerHeight` has been provided:
if (containerHeight !== null || containerHeight !== undefined)
This expression is always true for any value — if containerHeight is
null, the `!== undefined` branch is true, and vice versa. The intended
check should use AND:
if (containerHeight !== null && containerHeight !== undefined)
Without this fix, `isLayoutCalculated` returns true before layout
measurements complete, allowing `evaluatePosition` to run with
`containerHeight = INITIAL_LAYOUT_VALUE (-999)`. This produces
incorrect detent positions and can cause the sheet's content panel to
animate to an off-screen position while the backdrop renders correctly
(since both are derived from the same animatedPosition, but the
incorrect initial animation target can leave the body off-screen after
the "completed" callback fires and the snap-point-change correction
races with the mount animation).
The second condition (`!== INITIAL_LAYOUT_VALUE`) still serves as the
primary guard, but the first condition should be logically correct so
it doesn't silently pass for null/undefined containerHeight values.
gorhom
force-pushed
the
fix/isLayoutCalculated-always-true
branch
from
bc6eb05 to
eccfff3
Compare
Owner
|
thanks @spsaucier, that was a good catch. i had also merged the initial value condition to prevent early setting the state to true |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
isLayoutCalculatedinBottomSheet.tsxline 229 uses||(OR) instead of&&(AND) when checking whethercontainerHeighthas been provided:x !== null || x !== undefinedis a tautology — it evaluates totruefor every possible value. This meansisContainerHeightCalculatedis always set totrueon line 230, regardless of the actual container height state.Impact
While the second guard (
containerHeight !== INITIAL_LAYOUT_VALUE) usually catches this, the logical error meansisLayoutCalculatedcan returntruebefore layout measurements complete ifcontainerHeighthappens to benullorundefinedrather thanINITIAL_LAYOUT_VALUE. This allowsevaluatePositionto run prematurely, potentially producing incorrect detent positions.We observed a production issue where the bottom sheet's backdrop rendered correctly but the content panel was positioned off-screen (invisible), with
onChange(index >= 0)still firing — leaving users stuck behind a non-dismissable backdrop. The premature layout calculation is a plausible contributing factor.Change
Single character:
||→&&on line 229 ofsrc/components/bottomSheet/BottomSheet.tsx.