refactor: Aligned TextField with cursor rules#1081
Conversation
📖 Storybook Preview |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
📖 Storybook Preview |
📖 Storybook Preview |
There was a problem hiding this comment.
Nice update! One aspect of the component API that differs between React Native and React is that in React Native, ...props is spread to the child input element, while the outer wrapper receives the pressableProps object. In Extension, it’s the other way around: https://github.com/MetaMask/metamask-extension/blob/main/ui/components/component-library/text-field/text-field.tsx#L129-L158. I think in general we follow the convention of spreading props to the outermost layer, so I’m not sure whether we want to adopt that here for alignment. It’s also how Material UI approaches their TextField component: https://github.com/mui/material-ui/blob/master/packages/mui-material/src/TextField/TextField.js#L237
📖 Storybook Preview |
📖 Storybook Preview |
📖 Storybook Preview |
georgewrmarshall
left a comment
georgewrmarshall
left a comment
There was a problem hiding this comment.
A few non-blocking questions from comparing this refactor against the current RN TextField behavior and the extension text-field shape.
-
My understanding is that
Inputstill exposes the nativeTextInputPropssurface, includingplaceholderTextColor, but this change makes the theme placeholder color win over any caller-provided value. Is that intentional? If so, it may be worth documenting explicitly since directInputconsumers could reasonably expect nativeplaceholderTextColorto still work. -
My understanding is that this refactor removes the wrapper tap-to-focus behavior that the current RN
TextFieldhas, and that extension still has through the wrapper click handler. Is that intentional? I wanted to flag it because it changes the interaction model a bit and seems like a place where we may want stronger alignment across RN and extension if we can preserve it without compromising the nativeTextInputAPI. -
On the docs side, I may be reading this wrong, but the README seems to suggest top-level accessibility props can be used on the root container, while the root is currently rendered with
accessible={false}. Is the intent that those props live on the root container anyway, or should the docs steer people towardinputPropsfor the accessible input surface?
The inputProps direction still looks like the right long-term API to me, especially for things like testID and better alignment with extension. These comments are mainly about making sure the behavior changes here are intentional and clearly represented in the API/docs.
1/ Yes I don't think the placeholderTextColor should be overridable like it has previously. What do you think? Is there a legit case where we'd want to allow devs to override placeholderTextColor. The breaking change is documented in the migration doc I believe |
There was a problem hiding this comment.
Looking good! Left one suggestion, we should also check with @amandaye0h. In the extension version of the TextField selecting the accessories initiate the focus state unless they're a button. You can test it in the story I think we'd want the same behaviour for mobile
focus.mov
If we want to align with this, it will make the component more complicated, especially since it will require either the wrapping container to use Pressable along with all the props associate it with. Is that worth it? IMO I don't think it's worth it since the extra Pressable is only there to mainly make sure the startAccessory can focus the input. Additionally, aside from search, there arent that many other inputs with startAccessory, so I don't think its worth the extra complexity IMO. I did try the solution of keeping the Pressable while arranging the API around so we'd have some inputProps upleveled while keeping ...restProps for Pressable, and it just felt overly complicated for a case that's barely needed |
📖 Storybook Preview |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0fdaa99. Configure here.
georgewrmarshall
left a comment
georgewrmarshall
left a comment
There was a problem hiding this comment.
I took another pass on this with the migration path in mind, especially current mobile and extension usage. The overall direction looks good, and a few of the earlier behavior questions now seem intentionally documented. The remaining feedback is mostly about tightening the public API shape and making the new contracts around accessibility, refs, and test targeting a little clearer before this settles into the shared surface.
Sounds good. Hopefully mobile is less sensitive to the inactive hit area of the TextInput. Let's monitor incase it becomes an issue hitarea.mov |
📖 Storybook Preview |
📖 Storybook Preview |
📖 Storybook Preview |
📖 Storybook Preview |
## Release 37.0.0 This release focuses on consumer-facing changelog cleanup for the packages being published, plus migration-guide coverage for the React Native `TextField` API changes included in this release. ### 📦 Package Versions - `@metamask/design-system-shared`: **0.15.0** - `@metamask/design-system-react`: **0.20.0** - `@metamask/design-system-react-native`: **0.22.0** ### 🔄 Shared Type Updates (0.15.0) #### Shared type additions ([#1034](#1034), [#1038](#1038), [#1081](#1081)) **What Changed:** - Added shared `ButtonBaseSize`, `ButtonSize`, `ButtonHeroSize`, `ButtonVariant`, `ButtonBasePropsShared`, and `ButtonPropsShared` - Added shared `ButtonIconSize`, `ButtonIconVariant`, and `ButtonIconPropsShared` - Added shared `TextFieldPropsShared` for the controlled text-field contract used across platforms **Impact:** - Continues ADR-0003 and ADR-0004 migration toward shared const-object plus string-union exports - Gives React and React Native one shared source of truth for these component contracts ### 🌐 React Web Updates (0.20.0) #### Changed - **BREAKING:** Updated `Button`, `ButtonBase`, and `ButtonHero` size and variant exports to use shared const-object + string-union types rather than platform-local enum-based definitions ([#1034](#1034)) - No migration required for typical usage; continue importing from `@metamask/design-system-react` as before - **BREAKING:** Updated `ButtonIconSize` and `ButtonIconVariant` to use shared const-object + string-union types rather than platform-local enum-based definitions ([#1038](#1038)) - No migration required for typical usage; continue importing from `@metamask/design-system-react` as before - Updated Figma Code Connect to the live `MMDS Components` file and aligned `ButtonIcon` and `TextButton` mappings with the current component APIs shown in Dev Mode ([#1109](#1109)) - Expanded the `TextButton` migration guide for extension consumers replacing `ButtonLink` and `ButtonVariant.Link` with the current design-system APIs ([#1098](#1098)) ### 📱 React Native Updates (0.22.0) #### Changed - **BREAKING:** Updated `Button`, `ButtonBase`, and `ButtonHero` size and variant exports to use shared const-object + string-union types rather than platform-local enum-based definitions ([#1034](#1034)) - No migration required for typical usage; continue importing from `@metamask/design-system-react-native` as before - **BREAKING:** Updated `ButtonIconSize` and `ButtonIconVariant` to use shared const-object + string-union types rather than platform-local enum-based definitions ([#1038](#1038)) - No migration required for typical usage; continue importing from `@metamask/design-system-react-native` as before - **BREAKING:** `TextField` and `TextFieldSearch` now use a root `Box`/`View`, require native `TextInput` props under `inputProps`, rename `isReadonly` to `isReadOnly`, and use `inputRef` for the inner input ref ([#1081](#1081)) - Updated Figma Code Connect to the live `MMDS Components` file and aligned `ButtonIcon` and `TextButton` mappings with the current component APIs shown in Dev Mode ([#1109](#1109)) ###⚠️ Breaking Changes #### Shared button size and variant exports (Both Platforms) **What Changed:** - `ButtonVariant`, `ButtonBaseSize`, `ButtonSize`, `ButtonHeroSize`, `ButtonIconSize`, and `ButtonIconVariant` now come from shared ADR-0003/ADR-0004 definitions instead of platform-local enums - Runtime values and standard imports remain stable for typical usage **Impact:** - Affects TypeScript consumers that depended on enum-specific behavior rather than the exported members themselves #### React Native TextField prop layering (React Native) **What Changed:** - `TextField` and `TextFieldSearch` now treat the root as a `Box`/`View` - Native `TextInput` props must move under `inputProps` - `isReadonly` is renamed to `isReadOnly` - `ref` now targets the outer container and `inputRef` targets the inner `TextInput` **Migration:** ```tsx // Before (0.21.0) <TextField value={query} onChangeText={setQuery} placeholder="Search" keyboardType="default" secureTextEntry onFocus={handleFocus} /> // After (0.22.0) <TextField value={query} onChangeText={setQuery} placeholder="Search" onFocus={handleFocus} inputProps={{ keyboardType: 'default', secureTextEntry: true, }} /> ``` **Impact:** - Affects React Native consumers passing `TextInput` props or pressable-only props at the top level of `TextField` / `TextFieldSearch` See migration guides for complete instructions: - [React Native Migration Guide](./packages/design-system-react-native/MIGRATION.md#from-version-0210-to-0220) ### ✅ 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.14.0` → `0.15.0`) - shared type additions - [x] design-system-react: minor (`0.19.0` → `0.20.0`) - includes breaking changes under pre-1.0 semver - [x] design-system-react-native: minor (`0.21.0` → `0.22.0`) - includes breaking changes under pre-1.0 semver - [x] Breaking changes documented with migration guidance where needed - [x] Migration guides updated with before/after examples for the React Native `TextField` 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 - [ ] 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 where migration guidance is required - [ ] All unreleased changes are accounted for in changelogs <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Release-only change: bumps package versions and updates changelog/migration documentation without modifying runtime code. Low risk aside from potential downstream confusion if documented breaking changes don’t match what was actually published. > > **Overview** > Bumps the monorepo release to `37.0.0` and increments package versions for `@metamask/design-system-shared` (`0.15.0`), `@metamask/design-system-react` (`0.20.0`), and `@metamask/design-system-react-native` (`0.22.0`). > > Updates consumer-facing release notes: adds new changelog entries (including documented *breaking* type-export contract changes and RN `TextField` API migration details), refreshes the RN migration guide section for `0.21.0` → `0.22.0`, and updates changelog compare links accordingly. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 2140b12. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
2 participants
Description
This pull request aligns TextField (
@metamask/design-system-react-native) with our Cursor rules so the component and its docs/tests stay consistent with design-system conventions.Related issues
Fixes: N/A
Manual testing steps
yarn workspace @metamask/design-system-react-native run jest src/components/TextField/TextField.test.tsx --watchAll=false --coverage=falseand confirm all tests pass.yarn storybook:iosoryarn storybook:android(whichever you use for RN), open the TextField stories, and confirm default, error, disabled, accessories, and tap-to-focus behavior still match expectations.pressableProps={{ hitSlop: { top: 8, bottom: 8, left: 8, right: 8 } }}and confirm the larger touch target feels correct and does not break focus/blur.Screenshots/Recordings
Before
After
Everything is still functional
Simulator.Screen.Recording.-.iPhone.15.Pro.Max.-.2026-04-14.at.21.18.02.mov
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Medium Risk
This is a breaking public API refactor (prop layering,
reftarget change, removal of Pressable behavior) that can impact downstream call sites and imperative focus/testing selectors. Behavior changes to placeholder color precedence may also subtly affect UI theming expectations.Overview
Refactors React Native
TextField/TextFieldSearchto a layered API: the root is now aBox(View) and nativeTextInputprops must be passed via the newinputPropsbag, withreftargeting the root container and a newinputReffor imperative access to the inner input. As part of this, the root is no longer aPressable(tap-to-focus,hitSlop, and other Pressable-only props are removed), while focus/blur/error/disabled styling behavior is preserved.Renames
isReadonlytoisReadOnlyacrossInput,TextField, andTextFieldSearch, and changesInputso themeplaceholderTextColoralways overrides any caller-provided value. Updates Storybook stories, tests, READMEs, andMIGRATION.md, and introduces a sharedTextFieldPropsSharedtype indesign-system-sharedto align cross-platform prop surfaces.Reviewed by Cursor Bugbot for commit dfac4d5. Bugbot is set up for automated code reviews on this repo. Configure here.