Skip to content

android: size custom top-bar component buttons to content (Fabric collapse fix) #8328

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.

Sign up for GitHub

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

Merged
yedidyak merged 7 commits into from
Jun 14, 2026
Merged

android: size custom top-bar component buttons to content (Fabric collapse fix) #8328

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
81a0a9c
android: re-measure custom top bar button when async React content re…
israelko Jun 9, 2026
32c5316
chore: re-trigger CI
israelko Jun 9, 2026
663731d
remove `require-label.yml` workflow (as Yogi suggested 😀)
israelko Jun 10, 2026
81f1791
test: make collapsed-width assertion density-independent
israelko Jun 10, 2026
8388fd9
test: invalidate child measure cache before re-measure assertion
israelko Jun 10, 2026
8e50802
android: size custom top-bar component buttons to content via a singl…
israelko Jun 11, 2026
854131a
android: measure custom top-bar button height EXACTLY to keep content…
israelko Jun 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 0 additions & 13 deletions .github/workflows/require-label.yml
View file
Open in desktop

This file was deleted.

35 changes: 11 additions & 24 deletions ...n/java/com/reactnativenavigation/views/stack/topbar/titlebar/TitleBarReactButtonView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

@SuppressLint("ViewConstructor")
public class TitleBarReactButtonView extends ReactView {
private static final float FINAL_WIDTH_PADDING_DP = 1f;
private final ComponentOptions component;

public TitleBarReactButtonView(Context context, ComponentOptions component) {
Expand All @@ -45,27 +44,19 @@ protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
this.setId(View.NO_ID);
}

int initialWidthSpec = component.width.hasValue()
// Measure the hosted React surface with bounded (AT_MOST) specs and let it size itself to its
// content. Under Fabric, ReactSurfaceView reports max-of-children under AT_MOST (i.e. the
// laid-out content size) and pushes these constraints to the async Fabric layout. We must NOT
// follow this with a forced EXACTLY pass: re-pushing a forced box (which, before the content
// has laid out, is collapsed to ~1px) makes Fabric lay the content out into that collapsed box
// and the button never recovers (#8320/#8326 attempted exactly that and regressed under the
// New Architecture). When Fabric mounts/sizes the content, ReactSurfaceView re-requests layout
// and this measure runs again against the now-non-zero content.
int widthSpec = component.width.hasValue()
? createExactSpec(component.width)
: makeMeasureSpec(resolveAvailableWidth(widthMeasureSpec), AT_MOST);
int initialHeightSpec = createHeightSpec(heightMeasureSpec, component.height);

// First discover the content size without forcing every custom button to actionBarSize.
super.onMeasure(initialWidthSpec, initialHeightSpec);

if (component.width.hasValue() && component.height.hasValue()) {
return;
}

// Then give RN/Yoga a stable exact final box for compatibility with centered button layouts.
// A small allowance avoids clipping implicit RN padding/subpixel layout while staying content-based.
int finalWidth = component.width.hasValue()
? MeasureSpec.getSize(initialWidthSpec)
: resolveFinalWidth(getMeasuredWidth());
int finalHeight = component.height.hasValue()
? MeasureSpec.getSize(initialHeightSpec)
: Math.max(getMeasuredHeight(), 1);
super.onMeasure(makeMeasureSpec(finalWidth, EXACTLY), makeMeasureSpec(finalHeight, EXACTLY));
int heightSpec = createHeightSpec(heightMeasureSpec, component.height);
super.onMeasure(widthSpec, heightSpec);
}

private int createHeightSpec(int measureSpec, Number dimension) {
Expand All @@ -81,10 +72,6 @@ private int resolveAvailableWidth(int measureSpec) {
return availableSize > 0 ? availableSize : Math.max(getResources().getDisplayMetrics().widthPixels, 1);
}

private int resolveFinalWidth(int measuredContentWidth) {
return Math.max(measuredContentWidth + (int) Math.ceil(dpToPx(getContext(), FINAL_WIDTH_PADDING_DP)), 1);
}

private int createExactSpec(Number dimension) {
return makeMeasureSpec(MeasureSpec.getSize(dpToPx(getContext(), dimension.get())), EXACTLY);
}
Expand Down
34 changes: 13 additions & 21 deletions android/src/test/java/com/reactnativenavigation/views/TitleBarReactButtonViewTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,26 +32,26 @@ public class TitleBarReactButtonViewTest extends BaseTest {
private static final int CHILD_WIDTH = 24;
private static final int CHILD_HEIGHT = 16;

// Without explicit dimensions the button measures the hosted React surface ONCE with bounded
// (AT_MOST) specs and lets it size itself to its content. It deliberately does not follow up with
// a forced EXACTLY pass — under the New Architecture that re-pushes a (initially collapsed) box to
// the async Fabric layout and the button never recovers its real size.
@Test
public void missingDimensionsMeasureToContentThenRemeasureExactForStableAlignment() {
public void missingDimensionsMeasureToContentWithASingleBoundedPass() {
Activity activity = newActivity();
TitleBarReactButtonView uut = createView(activity, new ComponentOptions());
RecordingContentView child = new RecordingContentView(activity);
setContentView(uut, child);

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

assertThat(uut.getMeasuredWidth()).isEqualTo(finalWidth(activity));
assertThat(uut.getMeasuredWidth()).isEqualTo(CHILD_WIDTH);
assertThat(uut.getMeasuredHeight()).isEqualTo(CHILD_HEIGHT);
assertThat(child.widthMeasureSpecs.size()).isEqualTo(2);
assertThat(child.widthMeasureSpecs.size()).isEqualTo(1);
assertThat(getMode(child.widthMeasureSpecs.get(0))).isEqualTo(AT_MOST);
assertThat(getSize(child.widthMeasureSpecs.get(0))).isEqualTo(PARENT_WIDTH);
assertThat(getMode(child.heightMeasureSpecs.get(0))).isEqualTo(AT_MOST);
assertThat(getSize(child.heightMeasureSpecs.get(0))).isEqualTo(PARENT_HEIGHT);
assertThat(getMode(child.widthMeasureSpecs.get(1))).isEqualTo(EXACTLY);
assertThat(getSize(child.widthMeasureSpecs.get(1))).isEqualTo(finalWidth(activity));
assertThat(getMode(child.heightMeasureSpecs.get(1))).isEqualTo(EXACTLY);
assertThat(getSize(child.heightMeasureSpecs.get(1))).isEqualTo(CHILD_HEIGHT);
}

@Test
Expand Down Expand Up @@ -84,16 +84,12 @@ public void zeroParentSpecsFallbackToBoundedAtMostSpecs() {

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

assertThat(child.widthMeasureSpecs.size()).isEqualTo(2);
assertThat(child.widthMeasureSpecs.size()).isEqualTo(1);
assertThat(getMode(child.widthMeasureSpecs.get(0))).isEqualTo(AT_MOST);
assertThat(getSize(child.widthMeasureSpecs.get(0)))
.isEqualTo(Math.max(activity.getResources().getDisplayMetrics().widthPixels, 1));
assertThat(getMode(child.widthMeasureSpecs.get(1))).isEqualTo(EXACTLY);
assertThat(getSize(child.widthMeasureSpecs.get(1))).isEqualTo(finalWidth(activity));
assertThat(getMode(child.heightMeasureSpecs.get(0))).isEqualTo(AT_MOST);
assertThat(getSize(child.heightMeasureSpecs.get(0))).isEqualTo(Math.max(resolveActionBarSize(activity), 1));
assertThat(getMode(child.heightMeasureSpecs.get(1))).isEqualTo(EXACTLY);
assertThat(getSize(child.heightMeasureSpecs.get(1))).isEqualTo(CHILD_HEIGHT);
}

@Test
Expand All @@ -106,13 +102,11 @@ public void rtlMissingDimensionsUseBoundedSpecs() {

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

assertThat(child.widthMeasureSpecs.size()).isEqualTo(2);
assertThat(child.widthMeasureSpecs.size()).isEqualTo(1);
assertThat(getMode(child.widthMeasureSpecs.get(0))).isEqualTo(AT_MOST);
assertThat(getSize(child.widthMeasureSpecs.get(0))).isEqualTo(PARENT_WIDTH);
assertThat(getMode(child.widthMeasureSpecs.get(1))).isEqualTo(EXACTLY);
assertThat(getSize(child.widthMeasureSpecs.get(1))).isEqualTo(finalWidth(activity));
assertThat(getMode(child.heightMeasureSpecs.get(1))).isEqualTo(EXACTLY);
assertThat(getSize(child.heightMeasureSpecs.get(1))).isEqualTo(CHILD_HEIGHT);
assertThat(getMode(child.heightMeasureSpecs.get(0))).isEqualTo(AT_MOST);
assertThat(getSize(child.heightMeasureSpecs.get(0))).isEqualTo(PARENT_HEIGHT);
}

@Test
Expand All @@ -125,6 +119,8 @@ public void contentRemainsCenteredWhenMenuCellLaysButtonOutTallerThanMeasuredHei
uut.measure(makeMeasureSpec(PARENT_WIDTH, AT_MOST), makeMeasureSpec(PARENT_HEIGHT, AT_MOST));
uut.layout(0, 0, uut.getMeasuredWidth(), PARENT_HEIGHT);

// The button hugs the content height, and when a taller cell lays it out the content stays
// vertically centered via the CENTER_VERTICAL gravity applied in onViewAdded.
assertThat(uut.getMeasuredHeight()).isEqualTo(CHILD_HEIGHT);
assertThat(child.getTop()).isEqualTo((PARENT_HEIGHT - CHILD_HEIGHT) / 2);
assertThat(child.getBottom()).isEqualTo((PARENT_HEIGHT + CHILD_HEIGHT) / 2);
Expand Down Expand Up @@ -170,10 +166,6 @@ private int resolveActionBarSize(Activity activity) {
return UiUtils.dpToPx(activity, 48);
}

private int finalWidth(Activity activity) {
return CHILD_WIDTH + (int) Math.ceil(UiUtils.dpToPx(activity, 1f));
}

private static class RecordingContentView extends View {
final List<Integer> widthMeasureSpecs = new ArrayList<>();
final List<Integer> heightMeasureSpecs = new ArrayList<>();
Expand Down