fix: eliminate HeaderBase layout flicker with useLayoutEffect#25929
fix: eliminate HeaderBase layout flicker with useLayoutEffect#25929georgewrmarshall wants to merge 2 commits into
Conversation
Replace onLayout callbacks with useLayoutEffect + View.measure() to measure accessory widths synchronously before paint, eliminating visible layout flicker during initial render. Changes: - Add useLayoutEffect with View.measure() for synchronous dimension measurement - Replace onLayout callbacks with refs - Remove handleStartAccessoryLayout and handleEndAccessoryLayout - Measurements now occur before browser paint, preventing two-stage layout
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
metamaskbot
added
the
team-design-system
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Address Cursor bug report issues: - Keep both useLayoutEffect + measure() AND onLayout callbacks (High Severity) - Update dependencies to track actual content, not just boolean existence (Medium Severity) Changes: - Add back onLayout callbacks as fallback when measure() returns 0 - Update useLayoutEffect dependencies to include actual accessory props - Add width comparison checks to prevent unnecessary state updates - Improve comments to clarify dual measurement strategy The useLayoutEffect attempts synchronous measurement to reduce flicker, while onLayout provides fallback for initial mount and content changes.
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection: HeaderBase is used in:
The change is relatively contained (measurement timing logic) but affects a core UI component used across many flows. Selected tags cover the main areas where HeaderBase is used to ensure no visual regressions occur. Performance Test Selection: |
| startAccessoryRef.current.measure((_x, _y, width, _height) => { | ||
| if (width > 0 && width !== startAccessoryWidth) { | ||
| setStartAccessoryWidth(width); | ||
| } |
There was a problem hiding this comment.
Guard width > 0 prevents resetting stale accessory widths
High Severity
The width > 0 guard in both the measure() callback and the onLayout fallback 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, so startAccessoryWidth or endAccessoryWidth retains its stale non-zero value. This causes accessoryWrapperWidth via Math.max to use an incorrect measurement, misaligning the title. The previous code had no such guard and unconditionally called setStartAccessoryWidth(e.nativeEvent.layout.width), correctly handling the zero-width case.
Additional Locations (1)
| endButtonIconProps, | ||
| startAccessoryWidth, | ||
| endAccessoryWidth, | ||
| ]); |
There was a problem hiding this comment.
State values in useLayoutEffect deps cause extra cycles
Medium Severity
Including startAccessoryWidth and endAccessoryWidth (state values) in the useLayoutEffect dependency array causes the effect to re-run every time a measurement updates the state. Each width change triggers another measure() call that finds the value unchanged and does nothing. The reference pattern in useFeedScrollManager.ts deliberately excludes state from its useLayoutEffect deps to avoid this redundant cycle. Removing these two state entries from the array would eliminate the unnecessary re-execution.
|
2 participants
Description
This PR fixes visible layout flicker in the HeaderBase component by replacing
onLayoutcallbacks withuseLayoutEffect+View.measure()pattern for measuring accessory widths.Problem: HeaderBase currently uses
onLayoutcallbacks to measure accessory widths for centering the title. This causes a two-stage layout process:Solution: Switch to
useLayoutEffectwithView.measure()pattern (similar touseFeedScrollManager.ts). This measures dimensions synchronously before the browser paint, eliminating the visible two-stage layout.Changelog
CHANGELOG entry: Fixed layout flicker in HeaderBase component during initial render
Related issues
Fixes: (add issue number if available)
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Technical Details
Changes Made:
useRefanduseLayoutEffect, removeduseCallbackandLayoutChangeEventstartAccessoryRefandendAccessoryReffor measuring accessory widthsuseLayoutEffect: Measures accessory widths synchronously before painthandleStartAccessoryLayoutandhandleEndAccessoryLayoutonLayoutcallbacks with refsWhy This Eliminates Flicker:
useLayoutEffectruns synchronously after DOM mutations but before browser paintNote
Medium Risk
Touches a shared UI primitive’s layout measurement logic;
measure()/useLayoutEffecttiming can vary by platform and could impact header centering or cause extra renders if widths churn.Overview
Reduces visible title-centering flicker in
HeaderBaseby switching width calculation for the start/end accessory wrappers from layout-driven state updates to ref-basedmeasure()calls executed inuseLayoutEffect.Adds refs for both accessory containers, triggers synchronous measurements when accessories change, and keeps
onLayouthandlers as a fallback to update widths whenmeasure()reports0or content updates.Written by Cursor Bugbot for commit 09b9bc1. This will update automatically on new commits. Configure here.