Skip to content

Commit 34ca8b5

Browse files
israelkoclaude
israelko
and
claude
authored
android: size custom top-bar component buttons to content (Fabric collapse fix) (#8328)
* android: re-measure custom top bar button when async React content reports its size On the New Architecture (Fabric), a custom React-component top bar button without explicit width/height could collapse to ~1px. The hosted React surface lays out asynchronously, off the native measure pass, so the first onMeasure often observes a 0-sized child and freezes the button at the ~1px floor with no subsequent re-measure. Attach an OnLayoutChangeListener to the hosted child that re-requests layout when the content's size changes (so onMeasure re-runs and sizes the button to the content). The size-changed guard makes it converge once the button matches the content. The explicit-dimensions check is done inside the listener because onViewAdded fires from the superclass constructor (before `component` is assigned) for the React surface view we need to observe. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: re-trigger CI Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * remove `require-label.yml` workflow (as Yogi suggested 😀) * test: make collapsed-width assertion density-independent The new test hardcoded the collapsed button width as 1px, which only holds at mdpi (density 1.0). CI runs at a higher density, where resolveFinalWidth(0) = ceil(dpToPx(1dp)) > 1. Derive the expected value from the same dpToPx formula the production code uses instead of hardcoding it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: invalidate child measure cache before re-measure assertion The re-measure assertion failed (expected 25 but was 1) because View.measure() caches results: re-measuring the button with the same outer spec reused the child's stale cached 0-width measurement, so resolveFinalWidth still saw 0. In production RN re-measures the surface when it lays out new content; model that with child.forceLayout() (after child.layout(), which clears FORCE_LAYOUT) so the parent's super.onMeasure observes the real content width. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * android: size custom top-bar component buttons to content via a single bounded measure Custom React-component top bar buttons without explicit width/height collapsed to ~1px (invisible) under the New Architecture (Fabric). The two-pass measurement from #8320/#8326 re-measured the hosted ReactSurfaceView with an EXACTLY box derived from the first (often still-empty) discovery pass; that forced box is pushed to Fabric via updateLayoutSpecs, so Fabric keeps laying the content out into the collapsed box and the button never recovers. The same forcing also stretched flex-content buttons to the full bar height. Replace the two-pass with a single bounded (AT_MOST) measure and let ReactSurfaceView size itself to its content (its documented AT_MOST max-of-children behavior). When Fabric mounts/sizes the content it re-requests layout and this measure runs again against the real content size. Explicit width/height still measure EXACTLY; vertical centering is preserved by the existing CENTER_VERTICAL gravity. Verified on-device (Pixel 7, newArch+Hermes) on RN 0.78.3 and 0.85.2: content-hugging and flex component buttons render correctly; flex buttons are pixel-identical to before. Unit tests updated for the single-pass contract. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * android: measure custom top-bar button height EXACTLY to keep content vertically centered The previous commit fixed the collapse by measuring the React surface with a single bounded (AT_MOST) pass, but using AT_MOST for the *height* let content-hugging surfaces shrink, which shifted self-centering content (e.g. a flex container with justifyContent:'center') downward — the playground buttons_navbar snapshot caught this as an off-center round button on Fabric. Measure the height EXACTLY to the available bar height instead. A filled height box lets the React content center itself in the bar (matching the legacy two-pass layout), while the width stays AT_MOST so the surface still sizes to its content and the collapse fix is preserved (forcing the width is what caused the collapse; forcing the height does not). Verified on a Pixel 3a / API emulator (the CI device profile) in release: the buttons_navbar snapshot test passes (centering matches), and the full Buttons (14/14) and Stack (20/20) Detox suites pass. Unit tests updated for the filled-height contract. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 49def36 commit 34ca8b5

3 files changed

Lines changed: 32 additions & 79 deletions

File tree

‎.github/workflows/require-label.yml‎

Lines changed: 0 additions & 13 deletions
This file was deleted.

‎android/src/main/java/com/reactnativenavigation/views/stack/topbar/titlebar/TitleBarReactButtonView.java‎

Lines changed: 14 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
@SuppressLint("ViewConstructor")
2020
public class TitleBarReactButtonView extends ReactView {
21-
private static final float FINAL_WIDTH_PADDING_DP = 1f;
2221
private final ComponentOptions component;
2322

2423
public TitleBarReactButtonView(Context context, ComponentOptions component) {
@@ -45,46 +44,36 @@ protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
4544
this.setId(View.NO_ID);
4645
}
4746

48-
int initialWidthSpec = component.width.hasValue()
47+
// Width: bounded (AT_MOST) so the hosted React surface sizes itself to its content. Under
48+
// Fabric, ReactSurfaceView reports max-of-children under AT_MOST (the laid-out content width)
49+
// and pushes that to the async layout. Crucially we must NOT push a forced EXACTLY *width*:
50+
// before the content has laid out that width is collapsed (~1px), Fabric lays the content into
51+
// it and the button never recovers (#8320/#8326 did this and regressed under the New Arch).
52+
//
53+
// Height: EXACTLY the available bar height. Giving the surface a filled height (rather than a
54+
// content-hugging AT_MOST height) lets content that centers itself (e.g. a flex container with
55+
// justifyContent: 'center') sit vertically centered in the bar, matching the legacy layout.
56+
// It does not cause the width collapse — only a forced width does.
57+
int widthSpec = component.width.hasValue()
4958
? createExactSpec(component.width)
5059
: makeMeasureSpec(resolveAvailableWidth(widthMeasureSpec), AT_MOST);
51-
int initialHeightSpec = createHeightSpec(heightMeasureSpec, component.height);
52-
53-
// First discover the content size without forcing every custom button to actionBarSize.
54-
super.onMeasure(initialWidthSpec, initialHeightSpec);
55-
56-
if (component.width.hasValue() && component.height.hasValue()) {
57-
return;
58-
}
59-
60-
// Then give RN/Yoga a stable exact final box for compatibility with centered button layouts.
61-
// A small allowance avoids clipping implicit RN padding/subpixel layout while staying content-based.
62-
int finalWidth = component.width.hasValue()
63-
? MeasureSpec.getSize(initialWidthSpec)
64-
: resolveFinalWidth(getMeasuredWidth());
65-
int finalHeight = component.height.hasValue()
66-
? MeasureSpec.getSize(initialHeightSpec)
67-
: Math.max(getMeasuredHeight(), 1);
68-
super.onMeasure(makeMeasureSpec(finalWidth, EXACTLY), makeMeasureSpec(finalHeight, EXACTLY));
60+
int heightSpec = createHeightSpec(heightMeasureSpec, component.height);
61+
super.onMeasure(widthSpec, heightSpec);
6962
}
7063

7164
private int createHeightSpec(int measureSpec, Number dimension) {
7265
if (dimension.hasValue()) {
7366
return createExactSpec(dimension);
7467
}
7568
int availableSize = MeasureSpec.getSize(measureSpec);
76-
return makeMeasureSpec(availableSize > 0 ? availableSize : Math.max(resolveActionBarSize(), 1), AT_MOST);
69+
return makeMeasureSpec(availableSize > 0 ? availableSize : Math.max(resolveActionBarSize(), 1), EXACTLY);
7770
}
7871

7972
private int resolveAvailableWidth(int measureSpec) {
8073
int availableSize = MeasureSpec.getSize(measureSpec);
8174
return availableSize > 0 ? availableSize : Math.max(getResources().getDisplayMetrics().widthPixels, 1);
8275
}
8376

84-
private int resolveFinalWidth(int measuredContentWidth) {
85-
return Math.max(measuredContentWidth + (int) Math.ceil(dpToPx(getContext(), FINAL_WIDTH_PADDING_DP)), 1);
86-
}
87-
8877
private int createExactSpec(Number dimension) {
8978
return makeMeasureSpec(MeasureSpec.getSize(dpToPx(getContext(), dimension.get())), EXACTLY);
9079
}

‎android/src/test/java/com/reactnativenavigation/views/TitleBarReactButtonViewTest.java‎

Lines changed: 18 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -32,26 +32,28 @@ public class TitleBarReactButtonViewTest extends BaseTest {
3232
private static final int CHILD_WIDTH = 24;
3333
private static final int CHILD_HEIGHT = 16;
3434

35+
// Without explicit dimensions the button measures the hosted React surface ONCE:
36+
// - width AT_MOST → the surface sizes itself to its content (max-of-children under Fabric).
37+
// - height EXACTLY the available bar height → the surface gets a filled box so content that
38+
// centers itself (flex justifyContent: 'center') stays vertically centered in the bar.
39+
// It deliberately does not push a forced EXACTLY *width*; under the New Architecture that re-pushes
40+
// a (initially collapsed) width to the async Fabric layout and the button never recovers.
3541
@Test
36-
public void missingDimensionsMeasureToContentThenRemeasureExactForStableAlignment() {
42+
public void missingDimensionsSizeWidthToContentAndFillHeight() {
3743
Activity activity = newActivity();
3844
TitleBarReactButtonView uut = createView(activity, new ComponentOptions());
3945
RecordingContentView child = new RecordingContentView(activity);
4046
setContentView(uut, child);
4147

4248
uut.measure(makeMeasureSpec(PARENT_WIDTH, AT_MOST), makeMeasureSpec(PARENT_HEIGHT, AT_MOST));
4349

44-
assertThat(uut.getMeasuredWidth()).isEqualTo(finalWidth(activity));
45-
assertThat(uut.getMeasuredHeight()).isEqualTo(CHILD_HEIGHT);
46-
assertThat(child.widthMeasureSpecs.size()).isEqualTo(2);
50+
assertThat(uut.getMeasuredWidth()).isEqualTo(CHILD_WIDTH);
51+
assertThat(uut.getMeasuredHeight()).isEqualTo(PARENT_HEIGHT);
52+
assertThat(child.widthMeasureSpecs.size()).isEqualTo(1);
4753
assertThat(getMode(child.widthMeasureSpecs.get(0))).isEqualTo(AT_MOST);
4854
assertThat(getSize(child.widthMeasureSpecs.get(0))).isEqualTo(PARENT_WIDTH);
49-
assertThat(getMode(child.heightMeasureSpecs.get(0))).isEqualTo(AT_MOST);
55+
assertThat(getMode(child.heightMeasureSpecs.get(0))).isEqualTo(EXACTLY);
5056
assertThat(getSize(child.heightMeasureSpecs.get(0))).isEqualTo(PARENT_HEIGHT);
51-
assertThat(getMode(child.widthMeasureSpecs.get(1))).isEqualTo(EXACTLY);
52-
assertThat(getSize(child.widthMeasureSpecs.get(1))).isEqualTo(finalWidth(activity));
53-
assertThat(getMode(child.heightMeasureSpecs.get(1))).isEqualTo(EXACTLY);
54-
assertThat(getSize(child.heightMeasureSpecs.get(1))).isEqualTo(CHILD_HEIGHT);
5557
}
5658

5759
@Test
@@ -76,28 +78,24 @@ public void explicitDimensionsMeasureExactly() {
7678
}
7779

7880
@Test
79-
public void zeroParentSpecsFallbackToBoundedAtMostSpecs() {
81+
public void zeroParentSpecsFallBackToScreenWidthAndActionBarHeight() {
8082
Activity activity = newActivity();
8183
TitleBarReactButtonView uut = createView(activity, new ComponentOptions());
8284
RecordingContentView child = new RecordingContentView(activity);
8385
setContentView(uut, child);
8486

8587
uut.measure(makeMeasureSpec(0, AT_MOST), makeMeasureSpec(0, AT_MOST));
8688

87-
assertThat(child.widthMeasureSpecs.size()).isEqualTo(2);
89+
assertThat(child.widthMeasureSpecs.size()).isEqualTo(1);
8890
assertThat(getMode(child.widthMeasureSpecs.get(0))).isEqualTo(AT_MOST);
8991
assertThat(getSize(child.widthMeasureSpecs.get(0)))
9092
.isEqualTo(Math.max(activity.getResources().getDisplayMetrics().widthPixels, 1));
91-
assertThat(getMode(child.widthMeasureSpecs.get(1))).isEqualTo(EXACTLY);
92-
assertThat(getSize(child.widthMeasureSpecs.get(1))).isEqualTo(finalWidth(activity));
93-
assertThat(getMode(child.heightMeasureSpecs.get(0))).isEqualTo(AT_MOST);
93+
assertThat(getMode(child.heightMeasureSpecs.get(0))).isEqualTo(EXACTLY);
9494
assertThat(getSize(child.heightMeasureSpecs.get(0))).isEqualTo(Math.max(resolveActionBarSize(activity), 1));
95-
assertThat(getMode(child.heightMeasureSpecs.get(1))).isEqualTo(EXACTLY);
96-
assertThat(getSize(child.heightMeasureSpecs.get(1))).isEqualTo(CHILD_HEIGHT);
9795
}
9896

9997
@Test
100-
public void rtlMissingDimensionsUseBoundedSpecs() {
98+
public void rtlMissingDimensionsUseBoundedWidthAndFilledHeight() {
10199
Activity activity = newActivity();
102100
TitleBarReactButtonView uut = createView(activity, new ComponentOptions());
103101
RecordingContentView child = new RecordingContentView(activity);
@@ -106,28 +104,11 @@ public void rtlMissingDimensionsUseBoundedSpecs() {
106104

107105
uut.measure(makeMeasureSpec(PARENT_WIDTH, AT_MOST), makeMeasureSpec(PARENT_HEIGHT, AT_MOST));
108106

109-
assertThat(child.widthMeasureSpecs.size()).isEqualTo(2);
107+
assertThat(child.widthMeasureSpecs.size()).isEqualTo(1);
110108
assertThat(getMode(child.widthMeasureSpecs.get(0))).isEqualTo(AT_MOST);
111109
assertThat(getSize(child.widthMeasureSpecs.get(0))).isEqualTo(PARENT_WIDTH);
112-
assertThat(getMode(child.widthMeasureSpecs.get(1))).isEqualTo(EXACTLY);
113-
assertThat(getSize(child.widthMeasureSpecs.get(1))).isEqualTo(finalWidth(activity));
114-
assertThat(getMode(child.heightMeasureSpecs.get(1))).isEqualTo(EXACTLY);
115-
assertThat(getSize(child.heightMeasureSpecs.get(1))).isEqualTo(CHILD_HEIGHT);
116-
}
117-
118-
@Test
119-
public void contentRemainsCenteredWhenMenuCellLaysButtonOutTallerThanMeasuredHeight() {
120-
Activity activity = newActivity();
121-
TitleBarReactButtonView uut = createView(activity, new ComponentOptions());
122-
RecordingContentView child = new RecordingContentView(activity);
123-
setContentView(uut, child);
124-
125-
uut.measure(makeMeasureSpec(PARENT_WIDTH, AT_MOST), makeMeasureSpec(PARENT_HEIGHT, AT_MOST));
126-
uut.layout(0, 0, uut.getMeasuredWidth(), PARENT_HEIGHT);
127-
128-
assertThat(uut.getMeasuredHeight()).isEqualTo(CHILD_HEIGHT);
129-
assertThat(child.getTop()).isEqualTo((PARENT_HEIGHT - CHILD_HEIGHT) / 2);
130-
assertThat(child.getBottom()).isEqualTo((PARENT_HEIGHT + CHILD_HEIGHT) / 2);
110+
assertThat(getMode(child.heightMeasureSpecs.get(0))).isEqualTo(EXACTLY);
111+
assertThat(getSize(child.heightMeasureSpecs.get(0))).isEqualTo(PARENT_HEIGHT);
131112
}
132113

133114
@Test
@@ -170,10 +151,6 @@ private int resolveActionBarSize(Activity activity) {
170151
return UiUtils.dpToPx(activity, 48);
171152
}
172153

173-
private int finalWidth(Activity activity) {
174-
return CHILD_WIDTH + (int) Math.ceil(UiUtils.dpToPx(activity, 1f));
175-
}
176-
177154
private static class RecordingContentView extends View {
178155
final List<Integer> widthMeasureSpecs = new ArrayList<>();
179156
final List<Integer> heightMeasureSpecs = new ArrayList<>();

0 commit comments

Comments
 (0)