fix(onboarding): Clip the SCM reveal during its exit tween - #121495
Draft
jaydgoss wants to merge 1 commit into
Draft
fix(onboarding): Clip the SCM reveal during its exit tween#121495jaydgoss wants to merge 1 commit into
jaydgoss wants to merge 1 commit into
Conversation
AnimatePresence renders an exiting child from a frozen snapshot of its last present props, so the state-driven overflow style never reached the DOM on collapse and the content spilled over the sections below. Drive overflow through the animation targets instead, which bypass reconciliation. framer-motion applies non-animatable values immediately, so the clip lands at the start of the exit rather than being tweened. The state, ref, and both animation callbacks are no longer needed.
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
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.
TLDR
ScmCollapsibleRevealdid not clip its content while collapsing, so the exit tween spilled over whatever sat below it. Overflow now rides the animation targets instead of React state, which is the only way to reach an exiting child.Details
AnimatePresencerenders an exiting child fromrenderedChildren, React state holding the element captured when it was last present. This component drove overflow throughuseStateintostyle={{overflow}}, andonAnimationStartfiredsetOverflow('hidden')on collapse. That re-render produces a new element which is then discarded, because AnimatePresence keeps rendering the frozen one carryingoverflow: visible. The whole collapse ran unclipped.The existing
openRefwas a workaround for the same trap one level over, in the completion callback's closure. It does not help here: nothing routed through React props reaches an exiting child.Overflow now comes from the animation targets, which bypass reconciliation and write through the value pipeline:
initialandexitsetoverflow: hiddenanimatereleases it withtransitionEnd: {overflow: 'visible'}Two framer-motion behaviors this leans on, both verified in 12.38.0 rather than assumed:
isAnimatable('hidden')is false, socanAnimatefails andAsyncMotionValueAnimationapplies the value immediately, beforemakeAnimationInstant. The clip lands at the start of the collapse instead of being tweened across it.initial={false},makeLatestValuesuses theanimatetarget and foldstransitionEndkeys into the starting values, so a settled-open mount begins atoverflow: visible. That matches the previoususeState(open ? 'visible' : 'hidden').Open behavior is unchanged. The state, the ref, and both animation callbacks are no longer needed.
This was latent rather than new: every consumer (
ScmCollapsibleSection,ScmAlertOptionCard,ScmAlertFrequencySection,ScmIssueAlertNotificationOptions) had the same unclipped collapse. Their content is short and none of them sit inside a bordered card, which is probably why it went unnoticed.Refs VDY-150
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.