-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
fix: eliminate HeaderBase layout flicker with useLayoutEffect #25929
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| // Third party dependencies. | ||
| import React, { useCallback, useState } from 'react'; | ||
| import React, { useRef, useState, useLayoutEffect, useCallback } from 'react'; | ||
| import { View, LayoutChangeEvent } from 'react-native'; | ||
|
|
||
| // External dependencies. | ||
|
|
@@ -42,17 +42,11 @@ const HeaderBase: React.FC<HeaderBaseProps> = ({ | |
| const tw = useTailwind(); | ||
| const insets = useSafeAreaInsets(); | ||
|
|
||
| const startAccessoryRef = useRef<View>(null); | ||
| const endAccessoryRef = useRef<View>(null); | ||
| const [startAccessoryWidth, setStartAccessoryWidth] = useState(0); | ||
| const [endAccessoryWidth, setEndAccessoryWidth] = useState(0); | ||
|
|
||
| const handleStartAccessoryLayout = useCallback((e: LayoutChangeEvent) => { | ||
| setStartAccessoryWidth(e.nativeEvent.layout.width); | ||
| }, []); | ||
|
|
||
| const handleEndAccessoryLayout = useCallback((e: LayoutChangeEvent) => { | ||
| setEndAccessoryWidth(e.nativeEvent.layout.width); | ||
| }, []); | ||
|
|
||
| // Determine alignment and text variant based on variant prop | ||
| const isLeftAligned = variant === HeaderBaseVariant.Display; | ||
| const textVariant = HEADERBASE_VARIANT_TEXT_VARIANTS[variant]; | ||
|
|
@@ -63,6 +57,53 @@ const HeaderBase: React.FC<HeaderBaseProps> = ({ | |
| endAccessory || (endButtonIconProps && endButtonIconProps.length > 0); | ||
| const hasAnyAccessory = hasStartContent || hasEndContent; | ||
|
|
||
| // Attempt synchronous measurement on mount/updates to reduce flicker | ||
| useLayoutEffect(() => { | ||
| if (startAccessoryRef.current) { | ||
| startAccessoryRef.current.measure((_x, _y, width, _height) => { | ||
| if (width > 0 && width !== startAccessoryWidth) { | ||
| setStartAccessoryWidth(width); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| if (endAccessoryRef.current) { | ||
| endAccessoryRef.current.measure((_x, _y, width, _height) => { | ||
| if (width > 0 && width !== endAccessoryWidth) { | ||
| setEndAccessoryWidth(width); | ||
| } | ||
| }); | ||
| } | ||
| }, [ | ||
| startAccessory, | ||
| startButtonIconProps, | ||
| endAccessory, | ||
| endButtonIconProps, | ||
| startAccessoryWidth, | ||
| endAccessoryWidth, | ||
| ]); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. State values in useLayoutEffect deps cause extra cyclesMedium Severity Including |
||
|
|
||
| // Fallback onLayout callbacks for when measure() returns 0 or content changes | ||
| const handleStartAccessoryLayout = useCallback( | ||
| (event: LayoutChangeEvent) => { | ||
| const { width } = event.nativeEvent.layout; | ||
| if (width > 0 && width !== startAccessoryWidth) { | ||
| setStartAccessoryWidth(width); | ||
| } | ||
| }, | ||
| [startAccessoryWidth], | ||
| ); | ||
|
|
||
| const handleEndAccessoryLayout = useCallback( | ||
| (event: LayoutChangeEvent) => { | ||
| const { width } = event.nativeEvent.layout; | ||
| if (width > 0 && width !== endAccessoryWidth) { | ||
| setEndAccessoryWidth(width); | ||
| } | ||
| }, | ||
| [endAccessoryWidth], | ||
| ); | ||
|
|
||
| // For Compact: render both wrappers if any accessory exists (for centering) | ||
| // For Display: only render wrappers if their respective accessory exists | ||
| const shouldRenderStartWrapper = isLeftAligned | ||
|
|
@@ -150,7 +191,7 @@ const HeaderBase: React.FC<HeaderBaseProps> = ({ | |
| } | ||
| {...startAccessoryWrapperProps} | ||
| > | ||
| <View onLayout={handleStartAccessoryLayout}> | ||
| <View ref={startAccessoryRef} onLayout={handleStartAccessoryLayout}> | ||
| {renderStartContent()} | ||
| </View> | ||
| </View> | ||
|
|
@@ -182,6 +223,7 @@ const HeaderBase: React.FC<HeaderBaseProps> = ({ | |
| {...endAccessoryWrapperProps} | ||
| > | ||
| <View | ||
| ref={endAccessoryRef} | ||
| onLayout={handleEndAccessoryLayout} | ||
| style={ | ||
| hasMultipleEndButtons ? tw.style('flex-row gap-2') : undefined | ||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard
width > 0prevents resetting stale accessory widthsHigh Severity
The
width > 0guard in both themeasure()callback and theonLayoutfallback prevents accessory widths from ever resetting to zero. In Compact variant, when one accessory is removed while the other remains, the wrapper still renders (for centering) but contains no children, resulting in a width-0 layout event. The guard blocks this update, sostartAccessoryWidthorendAccessoryWidthretains its stale non-zero value. This causesaccessoryWrapperWidthviaMath.maxto use an incorrect measurement, misaligning the title. The previous code had no such guard and unconditionally calledsetStartAccessoryWidth(e.nativeEvent.layout.width), correctly handling the zero-width case.Additional Locations (1)
app/component-library/components/HeaderBase/HeaderBase.tsx#L89-L92