refactor: simplify HeaderRoot slot guards to direct conditionals#1076
Merged
georgewrmarshall merged 15 commits intoApr 14, 2026
Conversation
Contributor
📖 Storybook Preview |
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Title row renders when
title={false}withtitleAccessory- Restored explicit guard
title !== falseand usedisReactNodeRenderablefor title/accessory so the row is suppressed when title is false; tests pass.
- Restored explicit guard
Or push these changes by commenting:
@cursor push ed03fb450e
Preview (ed03fb450e)
diff --git a/packages/design-system-react-native/src/components/HeaderRoot/HeaderRoot.tsx b/packages/design-system-react-native/src/components/HeaderRoot/HeaderRoot.tsx
--- a/packages/design-system-react-native/src/components/HeaderRoot/HeaderRoot.tsx
+++ b/packages/design-system-react-native/src/components/HeaderRoot/HeaderRoot.tsx
@@ -7,6 +7,7 @@
import { BoxRow } from '../BoxRow';
import { ButtonIcon, ButtonIconSize } from '../ButtonIcon';
import { TextVariant } from '../Text';
+import { isReactNodeRenderable } from '@metamask/design-system-shared';
// Internal dependencies.
import type { HeaderRootProps } from './HeaderRoot.types';
@@ -49,16 +50,21 @@
if (children) {
return children;
}
- if (title || titleAccessory) {
+ const hasTitleContent =
+ title !== false &&
+ (isReactNodeRenderable(title) || isReactNodeRenderable(titleAccessory));
+ if (hasTitleContent) {
return (
<BoxRow
- endAccessory={titleAccessory}
+ endAccessory={
+ isReactNodeRenderable(titleAccessory) ? titleAccessory : undefined
+ }
textProps={{
variant: TextVariant.HeadingLg,
...titleProps,
}}
>
- {title || null}
+ {isReactNodeRenderable(title) ? title : null}
</BoxRow>
);
}You can send follow-ups to the cloud agent here.
Contributor
📖 Storybook Preview |
12 tasks
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Flex-1 wrapper removed breaks children layout
- Wrapped the children path in a flex-1 Box with alignItems start to restore fill and alignment behavior.
Or push these changes by commenting:
@cursor push fc8c76a8bd
Preview (fc8c76a8bd)
diff --git a/packages/design-system-react-native/src/components/HeaderRoot/HeaderRoot.tsx b/packages/design-system-react-native/src/components/HeaderRoot/HeaderRoot.tsx
--- a/packages/design-system-react-native/src/components/HeaderRoot/HeaderRoot.tsx
+++ b/packages/design-system-react-native/src/components/HeaderRoot/HeaderRoot.tsx
@@ -47,7 +47,11 @@
const renderLeftSection = () => {
if (children) {
- return children;
+ return (
+ <Box alignItems={BoxAlignItems.Start} twClassName="flex-1">
+ {children}
+ </Box>
+ );
}
if (title || titleAccessory) {
return (You can send follow-ups to the cloud agent here.
7 tasks
Contributor
📖 Storybook Preview |
Contributor
📖 Storybook Preview |
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: titleAccessory alone no longer triggers title row rendering
- Restored guard to render the title row when either title or titleAccessory is renderable while skipping when title === false.
Or push these changes by commenting:
@cursor push 85a2998c85
Preview (85a2998c85)
diff --git a/packages/design-system-react-native/src/components/HeaderRoot/HeaderRoot.tsx b/packages/design-system-react-native/src/components/HeaderRoot/HeaderRoot.tsx
--- a/packages/design-system-react-native/src/components/HeaderRoot/HeaderRoot.tsx
+++ b/packages/design-system-react-native/src/components/HeaderRoot/HeaderRoot.tsx
@@ -3,6 +3,7 @@
import { useSafeAreaInsets } from 'react-native-safe-area-context';
// External dependencies.
+import { isReactNodeRenderable } from '@metamask/design-system-shared';
import { Box, BoxAlignItems, BoxFlexDirection } from '../Box';
import { BoxRow } from '../BoxRow';
import { ButtonIcon, ButtonIconSize } from '../ButtonIcon';
@@ -49,7 +50,10 @@
if (children) {
return children;
}
- if (title) {
+ const hasTitleContent =
+ title !== false &&
+ (isReactNodeRenderable(title) || isReactNodeRenderable(titleAccessory));
+ if (hasTitleContent) {
return (
<BoxRow
endAccessory={titleAccessory}You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 96b0c4a. Configure here.
Contributor
📖 Storybook Preview |
Contributor
📖 Storybook Preview |
Contributor
📖 Storybook Preview |
Contributor
📖 Storybook Preview |
| @@ -1,5 +1,4 @@ | |||
| // Third party dependencies. | |||
| import { isReactNodeRenderable } from '@metamask/design-system-shared'; | |||
| import React from 'react'; | |||
Contributor
Author
There was a problem hiding this comment.
Removed isReactNodeRenderable from @metamask/design-system-shared — this was the only React Native component importing it. React already treats null, undefined, false, and '' as nothing when rendering, so the utility added a shared package dependency and a type import without solving a real problem. The !!prop implicit coercion that plain if (prop) performs is correct and sufficient for all realistic slot prop inputs.
| @@ -27,14 +26,6 @@ export const HeaderRoot = ({ | |||
| }: HeaderRootProps) => { | |||
| const insets = useSafeAreaInsets(); | |||
Contributor
Author
There was a problem hiding this comment.
The three guard variables that lived here (hasRenderableChildren, hasTitleContent, shouldRenderTitleRow) were computing booleans that if (children) and if (title) now express directly. The most problematic was hasTitleContent, which combined isReactNodeRenderable(title) || isReactNodeRenderable(titleAccessory)
| if (shouldRenderTitleRow) { | ||
| const titleNode = | ||
| isReactNodeRenderable(title) && title !== '' ? title : null; | ||
| if (title) { |
Contributor
Author
There was a problem hiding this comment.
titleAccessory is intentionally tied to title here — it is an accessory to the title row, not an independent slot. If you need a fully custom left section without a title string, use children instead. This makes the contract explicit: children owns its own layout, title + titleAccessory compose a managed row.
| </Box> | ||
| {hasEndSection ? ( | ||
| <Box flexDirection={BoxFlexDirection.Row} gap={2}> | ||
| {renderLeftSection()} |
Contributor
Author
There was a problem hiding this comment.
The outer Box wrapper (with style={{ flex: 1 }}) has been removed — children is now responsible for its own layout, and BoxRow gets twClassName="flex-1" directly. Using inline style on a wrapper that children couldn't see or control was the wrong layer to express flex growth; moving it onto BoxRow keeps the responsibility with the node that actually needs it.
| <Box flexDirection={BoxFlexDirection.Row} gap={2}> | ||
| {renderLeftSection()} | ||
| {endSectionContent ? ( | ||
| <Box flexDirection={BoxFlexDirection.Row} gap={2} twClassName="ml-auto"> |
Contributor
Author
There was a problem hiding this comment.
ml-auto replaces the old approach of letting the left section's flex: 1 push the end section to the right. Without the wrapper providing flex growth, the end section needs to position itself — ml-auto in a flex row achieves exactly that and is the idiomatic Tailwind pattern.
| @@ -78,31 +78,6 @@ describe('HeaderRoot', () => { | |||
| expect(getByTestId(LEFT_CHILDREN_TEST_ID)).toBeOnTheScreen(); | |||
| }); | |||
Contributor
Author
There was a problem hiding this comment.
These two tests were asserting behaviour that was never intentionally designed. Removing the tests is the correct response because the behaviour they described was wrong, not because the feature was cut.
|
|
||
| ### From version 0.18.0 to 0.19.0 | ||
|
|
||
| #### HeaderRoot: `titleAccessory` no longer renders without `title` |
Contributor
Author
There was a problem hiding this comment.
The titleAccessory standalone rendering change is a genuine breaking change worth calling out clearly here. The children escape hatch is the right migration path — it gives consumers full control over the left section layout without depending on the component's internal title row structure.
Contributor
📖 Storybook Preview |
Contributor
📖 Storybook Preview |
| */ | ||
| export function isReactNodeRenderable(node: ReactNode): boolean { | ||
| return node !== null && node !== undefined && typeof node !== 'boolean'; | ||
| } |
Contributor
Author
There was a problem hiding this comment.
The utility file and its test are fully deleted rather than deprecated — there is no transition period because !!prop is a zero-effort replacement. Keeping the utility would maintain a public API commitment for logic that plain JavaScript already provides, and the shared package is the wrong place for React rendering semantics anyway.
| }); | ||
|
|
||
| it('returns true for zero', () => { | ||
| expect(isReactNodeRenderable(0)).toBe(true); |
Contributor
Author
There was a problem hiding this comment.
The test suite is worth noting in the diff: it passed for isReactNodeRenderable('') returning true and isReactNodeRenderable(0) returning true — both treated as intended behaviour. In practice, consumers always pass pre-formatted strings (e.g. '$0.00', not 0) and empty string titles should never render a title row. The tests were accurately describing the utility's behaviour; the behaviour itself was the problem.
|
|
||
| export { isReactNodeRenderable } from './utils/isReactNodeRenderable'; | ||
|
|
||
| // AvatarBase types (ADR-0003 + ADR-0004) |
Contributor
Author
There was a problem hiding this comment.
Removing the export here is the only source-breaking change for consumers of @metamask/design-system-shared. Any downstream code that imports isReactNodeRenderable will get a compile-time or runtime resolution error, which is the intended signal to migrate. The MIGRATION.md documents the exact replacement.
|
|
||
| `isReactNodeRenderable` has been removed from the public API. The utility was introduced to guard conditional slot props (`children`, `title`, `titleAccessory`) against falsy values, but it solves a problem React already handles natively — `null`, `undefined`, `false`, and `''` all render as nothing in JSX without any guard. | ||
|
|
||
| The utility also introduced a subtle inconsistency: `isReactNodeRenderable('')` returned `true` (it only excluded `null`, `undefined`, and booleans), which required immediate workarounds like `&& title !== ''` at every callsite. |
Contributor
Author
There was a problem hiding this comment.
First MIGRATION.md for @metamask/design-system-shared, following the same structure as the react-native and react packages (Table of Contents, Version Updates wrapper, What Changed / Migration / Impact sections). The empty string inconsistency is called out explicitly because it was the concrete evidence that the utility's design was wrong — not just unnecessary.
Contributor
📖 Storybook Preview |
Comment on lines
+85
to
93
| twClassName="flex-1" // flex-1 ensures the text is truncated | ||
| > | ||
| <Text variant={TextVariant.BodyMd} twClassName="text-default"> | ||
| Imported Account 1 | ||
| <Text | ||
| ellipsizeMode="tail" | ||
| numberOfLines={1} | ||
| twClassName="flex-1" // flex-1 ensures the text is truncated | ||
| > | ||
| Imported Account 1 with a really long name | ||
| </Text> |
Contributor
Author
There was a problem hiding this comment.
Updating custom component to show truncation
Contributor
📖 Storybook Preview |
Remove isReactNodeRenderable from HeaderRoot in favour of plain truthiness checks. The named boolean variables (hasRenderableChildren, hasTitleContent, shouldRenderTitleRow, hasEndSection) and the shared utility were all aliases for logic the conditional statements already express directly. renderLeftSection now reads as plain priority order: children first, title row second, nothing otherwise. Every value that existed was a named wrapper for something the if/ternary already does. Partial fix for #1075.
f93f441 to
dd810b3
Compare
Contributor
📖 Storybook Preview |
Contributor
📖 Storybook Preview |
brianacnguyen
approved these changes
Apr 14, 2026
Contributor
📖 Storybook Preview |
georgewrmarshall
added a commit
that referenced
this pull request
Apr 21, 2026
## Release 34.0.0 This release adds new shared `TitleHub` and `Checkbox` type contracts, expands React 19 support for web and shared packages, and publishes a React Native release that aligns its supported runtime with the React Native 0.76 / Storybook 10 stack. ### 📦 Package Versions - `@metamask/design-system-shared`: **0.12.0** - `@metamask/design-system-react`: **0.17.1** - `@metamask/design-system-react-native`: **0.19.0** - `@metamask/design-system-twrnc-preset`: **0.4.2** ### 🔄 Shared Type Updates (0.12.0) #### Shared contract additions and API cleanup ([#1052](#1052), [#1040](#1040), [#1076](#1076), [#1089](#1089)) **What Changed:** - Added `TitleHubPropsShared` and `CheckboxPropsShared` to `@metamask/design-system-shared` - Removed `isReactNodeRenderable` from the public shared API - Expanded the shared package `react` peer dependency range to support React 19 **Impact:** - Enables consistent `TitleHub` and `Checkbox` implementations across React and React Native - Consumers importing `isReactNodeRenderable` must replace that import with plain truthy checks - Shared consumers on React 19 can now satisfy peer dependency requirements without overrides ### 🌐 React Web Updates (0.17.1) #### Changed - Expanded the `react` and `react-dom` peer dependency ranges to support React 19 consumers without changing the public component API ([#1089](#1089)) ### 📱 React Native Updates (0.19.0) #### Added - Added `TitleHub` for stacked title, amount, and bottom-label layouts with optional accessory slots ([#1052](#1052)) #### Changed - **BREAKING:** Raised the minimum supported peer dependency versions to React Native `>=0.76.0`, `react-native-gesture-handler >=2.25.0`, `react-native-reanimated >=3.17.0`, and `react-native-safe-area-context >=5.0.0` ([#844](#844)) - **BREAKING:** `HeaderRoot` now renders `titleAccessory` only when `title` is present; use `children` for fully custom accessory-only title rows ([#1076](#1076)) - **BREAKING:** `IconProps` now align with the underlying SVG component props instead of `ViewProps`; move `View`-specific props to a wrapper view if TypeScript flags them after upgrading ([#1090](#1090)) ### 🎨 TWRNC Preset Updates (0.4.2) #### Changed - Expanded the `react` peer dependency range to `>=18.2.0`, allowing the preset to install alongside newer React Native 0.76 and React 19 app stacks ([#844](#844)) ###⚠️ Breaking Changes #### `isReactNodeRenderable` removal (Shared) **What Changed:** - Removed `isReactNodeRenderable` from `@metamask/design-system-shared` - Shared consumers should replace this helper with standard truthy checks **Migration:** ```tsx // Before (0.11.0) import { isReactNodeRenderable } from '@metamask/design-system-shared'; if (isReactNodeRenderable(title)) { return <Header title={title} />; } // After (0.12.0) if (title) { return <Header title={title} />; } ``` **Impact:** - Any import of `isReactNodeRenderable` will fail after upgrading to `0.12.0` - See [Shared Migration Guide](./packages/design-system-shared/MIGRATION.md#from-version-0110-to-0120) #### React Native 0.76 peer minimums (React Native) **What Changed:** - Raised the minimum supported peer dependency versions to the React Native 0.76 family used by the Storybook 10 migration **Migration:** ```tsx // Before (0.18.0) // Compatible with older app stacks such as: // react-native: 0.72.x // react-native-gesture-handler: 2.12.x // react-native-reanimated: 3.3.x // react-native-safe-area-context: 4.x // After (0.19.0) // Consumers must be on at least: // react-native: 0.76.x // react-native-gesture-handler: 2.25.x // react-native-reanimated: 3.17.x // react-native-safe-area-context: 5.x ``` **Impact:** - Apps on older React Native stacks will no longer satisfy peer dependency requirements - See [React Native Migration Guide](./packages/design-system-react-native/MIGRATION.md#from-version-0180-to-0190) #### `HeaderRoot` accessory rendering and `IconProps` typing (React Native) **What Changed:** - `HeaderRoot` no longer renders `titleAccessory` unless `title` is present - `IconProps` now align with SVG props instead of `ViewProps` **Migration:** ```tsx // Before (0.18.0) <HeaderRoot titleAccessory={<Icon name={IconName.Info} />} /> <Icon name={IconName.Lock} onLayout={handleLayout} /> // After (0.19.0) <HeaderRoot title="Settings" titleAccessory={<Icon name={IconName.Info} />} /> <View onLayout={handleLayout}> <Icon name={IconName.Lock} /> </View> ``` **Impact:** - Accessory-only `HeaderRoot` title rows must switch to `children` or provide `title` - `View`-specific props on `Icon` must move to a wrapper `View` - See [React Native Migration Guide](./packages/design-system-react-native/MIGRATION.md#from-version-0180-to-0190) ### ✅ Checklist - [x] Changelogs updated with human-readable descriptions - [x] Changelog validation passed (`yarn changelog:validate`) - [x] Version bumps follow semantic versioning - [x] design-system-shared: minor (`0.11.0` → `0.12.0`) - pre-1.0 breaking shared API cleanup plus new shared type exports - [x] design-system-react: patch (`0.17.0` → `0.17.1`) - non-breaking compatibility update that widens React peer support to include v19 - [x] design-system-react-native: minor (`0.18.0` → `0.19.0`) - pre-1.0 breaking peer minimum and API behavior/type changes - [x] design-system-twrnc-preset: patch (`0.4.1` → `0.4.2`) - non-breaking peer range expansion - [x] Breaking changes documented with migration guidance - [x] Migration guides updated with before/after examples (if breaking changes) - [x] PR references included in changelog entries ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) - [x] I've reviewed the [Release Workflow](./.cursor/rules/release-workflow.md) cursor rule - [x] All tests pass (`yarn build && yarn test && yarn lint`) - [x] Changelog validation passes (`yarn changelog:validate`) ## **Pre-merge reviewer checklist** - [ ] I've reviewed the [Reviewing Release PRs](./docs/reviewing-release-prs.md) guide - [ ] Package versions follow semantic versioning - [ ] Changelog entries are consumer-facing (not commit message regurgitation) - [ ] Breaking changes are documented in MIGRATION.md with examples - [ ] All unreleased changes are accounted for in changelogs
georgewrmarshall
added a commit
that referenced
this pull request
Apr 27, 2026
## **Description**
`HeaderRoot` was using `isReactNodeRenderable` from
`@metamask/design-system-shared` and several intermediate named boolean
guards (`hasRenderableChildren`, `hasTitleContent`,
`shouldRenderTitleRow`) to decide what to render in the left section.
This logic was over-engineered — React already treats `null`,
`undefined`, `false`, and `''` as nothing when rendering, so `!!prop` is
the correct and sufficient check.
The guards also introduced a real bug: `isReactNodeRenderable('')`
returns `true` (it only excludes `null`, `undefined`, and booleans),
which forced an additional `&& title !== ''` workaround inline.
Replacing everything with direct `if (children)` / `if (title)`
conditionals removes the utility dependency, the workarounds, and the
accidental complexity.
**Changes:**
- Removed `isReactNodeRenderable` import and all intermediate guard
variables from `HeaderRoot`
- Replaced with direct `if (children)` / `if (title)` conditionals in
`renderLeftSection`
- Removed the inner `Box` wrapper around the left section; `BoxRow` now
renders directly as a flex child with `twClassName="flex-1"`
- End section container gains `ml-auto` to push it to the right without
the wrapper
- Removed stale tests that asserted `titleAccessory` renders without a
`title` (behaviour that was never intentional and is now correctly
absent)
- Updated `MIGRATION.md` to document both breaking changes for 0.19.0
**Breaking changes:**
1. `titleAccessory` no longer renders when `title` is falsy — it is
decoration on the title row, not a standalone slot
2. The intermediate `Box` wrapper around the left section is removed
(structural change, no visual impact)
Related issue tracking the full `isReactNodeRenderable` cleanup: #1075
## **Related issues**
Fixes: #1075 (partial — `HeaderRoot` only; `TitleHub` and shared package
cleanup tracked in the issue)
## **Manual testing steps**
1. Run `yarn storybook:ios` or `yarn storybook:android`
2. Navigate to `Components/HeaderRoot`
3. Verify all existing stories render correctly (Default, Title,
TitleAccessory, Children, EndAccessory, EndButtonIconProps,
EndButtonIconPropsMultiple)
4. Verify `titleAccessory` renders alongside `title` and does not render
when `title` is omitted
## **Screenshots/Recordings**
<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->
### **Before**
No visual changes
https://github.com/user-attachments/assets/7e6e0db4-0a45-4a16-8b47-2a4016cd8f64
### **After**
https://github.com/user-attachments/assets/ea45dfb5-8a09-4b03-9504-c918347df7d5
## **Pre-merge author checklist**
- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs)
- [x] I've completed the PR template to the best of my ability
- [x] I've included tests if applicable
- [x] I've documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I've applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
## **Pre-merge reviewer checklist**
- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Introduces a small breaking behavior change in `HeaderRoot`
(`titleAccessory` no longer renders without a truthy `title`) and
removes `isReactNodeRenderable` from `@metamask/design-system-shared`,
which can break downstream imports.
>
> **Overview**
> Simplifies `HeaderRoot` slot rendering by removing
`isReactNodeRenderable` and the derived guard booleans, switching to
direct `if (children)` / `if (title)` conditionals.
>
> This also changes behavior so `titleAccessory` only renders when
`title` is truthy, and slightly restructures layout by removing the left
wrapper `Box` and using `twClassName="ml-auto"` to right-align the end
section.
>
> In `@metamask/design-system-shared`, removes the
`isReactNodeRenderable` utility (and its tests) from the public exports
and adds migration docs for the removal; the React Native package
migration guide now documents the `HeaderRoot` breaking change and
structural tweak.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
6463fb1. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
georgewrmarshall
added a commit
that referenced
this pull request
Apr 27, 2026
## Release 34.0.0 This release adds new shared `TitleHub` and `Checkbox` type contracts, expands React 19 support for web and shared packages, and publishes a React Native release that aligns its supported runtime with the React Native 0.76 / Storybook 10 stack. ### 📦 Package Versions - `@metamask/design-system-shared`: **0.12.0** - `@metamask/design-system-react`: **0.17.1** - `@metamask/design-system-react-native`: **0.19.0** - `@metamask/design-system-twrnc-preset`: **0.4.2** ### 🔄 Shared Type Updates (0.12.0) #### Shared contract additions and API cleanup ([#1052](#1052), [#1040](#1040), [#1076](#1076), [#1089](#1089)) **What Changed:** - Added `TitleHubPropsShared` and `CheckboxPropsShared` to `@metamask/design-system-shared` - Removed `isReactNodeRenderable` from the public shared API - Expanded the shared package `react` peer dependency range to support React 19 **Impact:** - Enables consistent `TitleHub` and `Checkbox` implementations across React and React Native - Consumers importing `isReactNodeRenderable` must replace that import with plain truthy checks - Shared consumers on React 19 can now satisfy peer dependency requirements without overrides ### 🌐 React Web Updates (0.17.1) #### Changed - Expanded the `react` and `react-dom` peer dependency ranges to support React 19 consumers without changing the public component API ([#1089](#1089)) ### 📱 React Native Updates (0.19.0) #### Added - Added `TitleHub` for stacked title, amount, and bottom-label layouts with optional accessory slots ([#1052](#1052)) #### Changed - **BREAKING:** Raised the minimum supported peer dependency versions to React Native `>=0.76.0`, `react-native-gesture-handler >=2.25.0`, `react-native-reanimated >=3.17.0`, and `react-native-safe-area-context >=5.0.0` ([#844](#844)) - **BREAKING:** `HeaderRoot` now renders `titleAccessory` only when `title` is present; use `children` for fully custom accessory-only title rows ([#1076](#1076)) - **BREAKING:** `IconProps` now align with the underlying SVG component props instead of `ViewProps`; move `View`-specific props to a wrapper view if TypeScript flags them after upgrading ([#1090](#1090)) ### 🎨 TWRNC Preset Updates (0.4.2) #### Changed - Expanded the `react` peer dependency range to `>=18.2.0`, allowing the preset to install alongside newer React Native 0.76 and React 19 app stacks ([#844](#844)) ###⚠️ Breaking Changes #### `isReactNodeRenderable` removal (Shared) **What Changed:** - Removed `isReactNodeRenderable` from `@metamask/design-system-shared` - Shared consumers should replace this helper with standard truthy checks **Migration:** ```tsx // Before (0.11.0) import { isReactNodeRenderable } from '@metamask/design-system-shared'; if (isReactNodeRenderable(title)) { return <Header title={title} />; } // After (0.12.0) if (title) { return <Header title={title} />; } ``` **Impact:** - Any import of `isReactNodeRenderable` will fail after upgrading to `0.12.0` - See [Shared Migration Guide](./packages/design-system-shared/MIGRATION.md#from-version-0110-to-0120) #### React Native 0.76 peer minimums (React Native) **What Changed:** - Raised the minimum supported peer dependency versions to the React Native 0.76 family used by the Storybook 10 migration **Migration:** ```tsx // Before (0.18.0) // Compatible with older app stacks such as: // react-native: 0.72.x // react-native-gesture-handler: 2.12.x // react-native-reanimated: 3.3.x // react-native-safe-area-context: 4.x // After (0.19.0) // Consumers must be on at least: // react-native: 0.76.x // react-native-gesture-handler: 2.25.x // react-native-reanimated: 3.17.x // react-native-safe-area-context: 5.x ``` **Impact:** - Apps on older React Native stacks will no longer satisfy peer dependency requirements - See [React Native Migration Guide](./packages/design-system-react-native/MIGRATION.md#from-version-0180-to-0190) #### `HeaderRoot` accessory rendering and `IconProps` typing (React Native) **What Changed:** - `HeaderRoot` no longer renders `titleAccessory` unless `title` is present - `IconProps` now align with SVG props instead of `ViewProps` **Migration:** ```tsx // Before (0.18.0) <HeaderRoot titleAccessory={<Icon name={IconName.Info} />} /> <Icon name={IconName.Lock} onLayout={handleLayout} /> // After (0.19.0) <HeaderRoot title="Settings" titleAccessory={<Icon name={IconName.Info} />} /> <View onLayout={handleLayout}> <Icon name={IconName.Lock} /> </View> ``` **Impact:** - Accessory-only `HeaderRoot` title rows must switch to `children` or provide `title` - `View`-specific props on `Icon` must move to a wrapper `View` - See [React Native Migration Guide](./packages/design-system-react-native/MIGRATION.md#from-version-0180-to-0190) ### ✅ Checklist - [x] Changelogs updated with human-readable descriptions - [x] Changelog validation passed (`yarn changelog:validate`) - [x] Version bumps follow semantic versioning - [x] design-system-shared: minor (`0.11.0` → `0.12.0`) - pre-1.0 breaking shared API cleanup plus new shared type exports - [x] design-system-react: patch (`0.17.0` → `0.17.1`) - non-breaking compatibility update that widens React peer support to include v19 - [x] design-system-react-native: minor (`0.18.0` → `0.19.0`) - pre-1.0 breaking peer minimum and API behavior/type changes - [x] design-system-twrnc-preset: patch (`0.4.1` → `0.4.2`) - non-breaking peer range expansion - [x] Breaking changes documented with migration guidance - [x] Migration guides updated with before/after examples (if breaking changes) - [x] PR references included in changelog entries ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) - [x] I've reviewed the [Release Workflow](./.cursor/rules/release-workflow.md) cursor rule - [x] All tests pass (`yarn build && yarn test && yarn lint`) - [x] Changelog validation passes (`yarn changelog:validate`) ## **Pre-merge reviewer checklist** - [ ] I've reviewed the [Reviewing Release PRs](./docs/reviewing-release-prs.md) guide - [ ] Package versions follow semantic versioning - [ ] Changelog entries are consumer-facing (not commit message regurgitation) - [ ] Breaking changes are documented in MIGRATION.md with examples - [ ] All unreleased changes are accounted for in changelogs
cursor Bot
pushed a commit
that referenced
this pull request
Apr 28, 2026
## **Description**
`HeaderRoot` was using `isReactNodeRenderable` from
`@metamask/design-system-shared` and several intermediate named boolean
guards (`hasRenderableChildren`, `hasTitleContent`,
`shouldRenderTitleRow`) to decide what to render in the left section.
This logic was over-engineered — React already treats `null`,
`undefined`, `false`, and `''` as nothing when rendering, so `!!prop` is
the correct and sufficient check.
The guards also introduced a real bug: `isReactNodeRenderable('')`
returns `true` (it only excludes `null`, `undefined`, and booleans),
which forced an additional `&& title !== ''` workaround inline.
Replacing everything with direct `if (children)` / `if (title)`
conditionals removes the utility dependency, the workarounds, and the
accidental complexity.
**Changes:**
- Removed `isReactNodeRenderable` import and all intermediate guard
variables from `HeaderRoot`
- Replaced with direct `if (children)` / `if (title)` conditionals in
`renderLeftSection`
- Removed the inner `Box` wrapper around the left section; `BoxRow` now
renders directly as a flex child with `twClassName="flex-1"`
- End section container gains `ml-auto` to push it to the right without
the wrapper
- Removed stale tests that asserted `titleAccessory` renders without a
`title` (behaviour that was never intentional and is now correctly
absent)
- Updated `MIGRATION.md` to document both breaking changes for 0.19.0
**Breaking changes:**
1. `titleAccessory` no longer renders when `title` is falsy — it is
decoration on the title row, not a standalone slot
2. The intermediate `Box` wrapper around the left section is removed
(structural change, no visual impact)
Related issue tracking the full `isReactNodeRenderable` cleanup: #1075
## **Related issues**
Fixes: #1075 (partial — `HeaderRoot` only; `TitleHub` and shared package
cleanup tracked in the issue)
## **Manual testing steps**
1. Run `yarn storybook:ios` or `yarn storybook:android`
2. Navigate to `Components/HeaderRoot`
3. Verify all existing stories render correctly (Default, Title,
TitleAccessory, Children, EndAccessory, EndButtonIconProps,
EndButtonIconPropsMultiple)
4. Verify `titleAccessory` renders alongside `title` and does not render
when `title` is omitted
## **Screenshots/Recordings**
<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->
### **Before**
No visual changes
https://github.com/user-attachments/assets/7e6e0db4-0a45-4a16-8b47-2a4016cd8f64
### **After**
https://github.com/user-attachments/assets/ea45dfb5-8a09-4b03-9504-c918347df7d5
## **Pre-merge author checklist**
- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs)
- [x] I've completed the PR template to the best of my ability
- [x] I've included tests if applicable
- [x] I've documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I've applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
## **Pre-merge reviewer checklist**
- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Introduces a small breaking behavior change in `HeaderRoot`
(`titleAccessory` no longer renders without a truthy `title`) and
removes `isReactNodeRenderable` from `@metamask/design-system-shared`,
which can break downstream imports.
>
> **Overview**
> Simplifies `HeaderRoot` slot rendering by removing
`isReactNodeRenderable` and the derived guard booleans, switching to
direct `if (children)` / `if (title)` conditionals.
>
> This also changes behavior so `titleAccessory` only renders when
`title` is truthy, and slightly restructures layout by removing the left
wrapper `Box` and using `twClassName="ml-auto"` to right-align the end
section.
>
> In `@metamask/design-system-shared`, removes the
`isReactNodeRenderable` utility (and its tests) from the public exports
and adds migration docs for the removal; the React Native package
migration guide now documents the `HeaderRoot` breaking change and
structural tweak.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
6463fb1. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
cursor Bot
pushed a commit
that referenced
this pull request
Apr 28, 2026
## Release 34.0.0 This release adds new shared `TitleHub` and `Checkbox` type contracts, expands React 19 support for web and shared packages, and publishes a React Native release that aligns its supported runtime with the React Native 0.76 / Storybook 10 stack. ### 📦 Package Versions - `@metamask/design-system-shared`: **0.12.0** - `@metamask/design-system-react`: **0.17.1** - `@metamask/design-system-react-native`: **0.19.0** - `@metamask/design-system-twrnc-preset`: **0.4.2** ### 🔄 Shared Type Updates (0.12.0) #### Shared contract additions and API cleanup ([#1052](#1052), [#1040](#1040), [#1076](#1076), [#1089](#1089)) **What Changed:** - Added `TitleHubPropsShared` and `CheckboxPropsShared` to `@metamask/design-system-shared` - Removed `isReactNodeRenderable` from the public shared API - Expanded the shared package `react` peer dependency range to support React 19 **Impact:** - Enables consistent `TitleHub` and `Checkbox` implementations across React and React Native - Consumers importing `isReactNodeRenderable` must replace that import with plain truthy checks - Shared consumers on React 19 can now satisfy peer dependency requirements without overrides ### 🌐 React Web Updates (0.17.1) #### Changed - Expanded the `react` and `react-dom` peer dependency ranges to support React 19 consumers without changing the public component API ([#1089](#1089)) ### 📱 React Native Updates (0.19.0) #### Added - Added `TitleHub` for stacked title, amount, and bottom-label layouts with optional accessory slots ([#1052](#1052)) #### Changed - **BREAKING:** Raised the minimum supported peer dependency versions to React Native `>=0.76.0`, `react-native-gesture-handler >=2.25.0`, `react-native-reanimated >=3.17.0`, and `react-native-safe-area-context >=5.0.0` ([#844](#844)) - **BREAKING:** `HeaderRoot` now renders `titleAccessory` only when `title` is present; use `children` for fully custom accessory-only title rows ([#1076](#1076)) - **BREAKING:** `IconProps` now align with the underlying SVG component props instead of `ViewProps`; move `View`-specific props to a wrapper view if TypeScript flags them after upgrading ([#1090](#1090)) ### 🎨 TWRNC Preset Updates (0.4.2) #### Changed - Expanded the `react` peer dependency range to `>=18.2.0`, allowing the preset to install alongside newer React Native 0.76 and React 19 app stacks ([#844](#844)) ###⚠️ Breaking Changes #### `isReactNodeRenderable` removal (Shared) **What Changed:** - Removed `isReactNodeRenderable` from `@metamask/design-system-shared` - Shared consumers should replace this helper with standard truthy checks **Migration:** ```tsx // Before (0.11.0) import { isReactNodeRenderable } from '@metamask/design-system-shared'; if (isReactNodeRenderable(title)) { return <Header title={title} />; } // After (0.12.0) if (title) { return <Header title={title} />; } ``` **Impact:** - Any import of `isReactNodeRenderable` will fail after upgrading to `0.12.0` - See [Shared Migration Guide](./packages/design-system-shared/MIGRATION.md#from-version-0110-to-0120) #### React Native 0.76 peer minimums (React Native) **What Changed:** - Raised the minimum supported peer dependency versions to the React Native 0.76 family used by the Storybook 10 migration **Migration:** ```tsx // Before (0.18.0) // Compatible with older app stacks such as: // react-native: 0.72.x // react-native-gesture-handler: 2.12.x // react-native-reanimated: 3.3.x // react-native-safe-area-context: 4.x // After (0.19.0) // Consumers must be on at least: // react-native: 0.76.x // react-native-gesture-handler: 2.25.x // react-native-reanimated: 3.17.x // react-native-safe-area-context: 5.x ``` **Impact:** - Apps on older React Native stacks will no longer satisfy peer dependency requirements - See [React Native Migration Guide](./packages/design-system-react-native/MIGRATION.md#from-version-0180-to-0190) #### `HeaderRoot` accessory rendering and `IconProps` typing (React Native) **What Changed:** - `HeaderRoot` no longer renders `titleAccessory` unless `title` is present - `IconProps` now align with SVG props instead of `ViewProps` **Migration:** ```tsx // Before (0.18.0) <HeaderRoot titleAccessory={<Icon name={IconName.Info} />} /> <Icon name={IconName.Lock} onLayout={handleLayout} /> // After (0.19.0) <HeaderRoot title="Settings" titleAccessory={<Icon name={IconName.Info} />} /> <View onLayout={handleLayout}> <Icon name={IconName.Lock} /> </View> ``` **Impact:** - Accessory-only `HeaderRoot` title rows must switch to `children` or provide `title` - `View`-specific props on `Icon` must move to a wrapper `View` - See [React Native Migration Guide](./packages/design-system-react-native/MIGRATION.md#from-version-0180-to-0190) ### ✅ Checklist - [x] Changelogs updated with human-readable descriptions - [x] Changelog validation passed (`yarn changelog:validate`) - [x] Version bumps follow semantic versioning - [x] design-system-shared: minor (`0.11.0` → `0.12.0`) - pre-1.0 breaking shared API cleanup plus new shared type exports - [x] design-system-react: patch (`0.17.0` → `0.17.1`) - non-breaking compatibility update that widens React peer support to include v19 - [x] design-system-react-native: minor (`0.18.0` → `0.19.0`) - pre-1.0 breaking peer minimum and API behavior/type changes - [x] design-system-twrnc-preset: patch (`0.4.1` → `0.4.2`) - non-breaking peer range expansion - [x] Breaking changes documented with migration guidance - [x] Migration guides updated with before/after examples (if breaking changes) - [x] PR references included in changelog entries ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) - [x] I've reviewed the [Release Workflow](./.cursor/rules/release-workflow.md) cursor rule - [x] All tests pass (`yarn build && yarn test && yarn lint`) - [x] Changelog validation passes (`yarn changelog:validate`) ## **Pre-merge reviewer checklist** - [ ] I've reviewed the [Reviewing Release PRs](./docs/reviewing-release-prs.md) guide - [ ] Package versions follow semantic versioning - [ ] Changelog entries are consumer-facing (not commit message regurgitation) - [ ] Breaking changes are documented in MIGRATION.md with examples - [ ] All unreleased changes are accounted for in changelogs
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
2 participants
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.
Description
HeaderRootwas usingisReactNodeRenderablefrom@metamask/design-system-sharedand several intermediate named boolean guards (hasRenderableChildren,hasTitleContent,shouldRenderTitleRow) to decide what to render in the left section. This logic was over-engineered — React already treatsnull,undefined,false, and''as nothing when rendering, so!!propis the correct and sufficient check.The guards also introduced a real bug:
isReactNodeRenderable('')returnstrue(it only excludesnull,undefined, and booleans), which forced an additional&& title !== ''workaround inline. Replacing everything with directif (children)/if (title)conditionals removes the utility dependency, the workarounds, and the accidental complexity.Changes:
isReactNodeRenderableimport and all intermediate guard variables fromHeaderRootif (children)/if (title)conditionals inrenderLeftSectionBoxwrapper around the left section;BoxRownow renders directly as a flex child withtwClassName="flex-1"ml-autoto push it to the right without the wrappertitleAccessoryrenders without atitle(behaviour that was never intentional and is now correctly absent)MIGRATION.mdto document both breaking changes for 0.19.0Breaking changes:
titleAccessoryno longer renders whentitleis falsy — it is decoration on the title row, not a standalone slotBoxwrapper around the left section is removed (structural change, no visual impact)Related issue tracking the full
isReactNodeRenderablecleanup: #1075Related issues
Fixes: #1075 (partial —
HeaderRootonly;TitleHuband shared package cleanup tracked in the issue)Manual testing steps
yarn storybook:iosoryarn storybook:androidComponents/HeaderRoottitleAccessoryrenders alongsidetitleand does not render whentitleis omittedScreenshots/Recordings
Before
No visual changes
before720.mov
After
after720.mov
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Medium Risk
Introduces breaking behavior changes in
HeaderRoot(no standalonetitleAccessory) and removes a previously exported shared utility, which can break downstream builds/usages if relied upon.Overview
Simplifies
HeaderRootrendering logic and layout. ReplacesisReactNodeRenderable-based guards with directchildren/titleconditionals, removes the left-section wrapperBox, and uses flex utilities (includingml-auto) to keep the end section aligned.Behavior changes are now explicit and documented.
titleAccessoryonly renders whentitleis truthy (tests updated accordingly), Storybook examples were adjusted for truncation, andMIGRATION.mdadds guidance for the 0.19.0 change.Shared package cleanup. Removes the
isReactNodeRenderableutility (and its tests) from@metamask/design-system-sharedexports and adds a shared migration guide noting the removal.Reviewed by Cursor Bugbot for commit 08a0039. Bugbot is set up for automated code reviews on this repo. Configure here.