feat(DSYS-488): Migrate Input to ADR-0003 and ADR-0004#1043
feat(DSYS-488): Migrate Input to ADR-0003 and ADR-0004#1043georgewrmarshall merged 8 commits intomainfrom
Conversation
📖 Storybook Preview |
|
@cursoragent rebase and resolve all conflicts now other components have moved to shared double check to make sure all imports are coming from design-system-shared and not ../../types |
  |
|
@cursoragent well that was a massive fail. You've made a bad rebase and now have over 16,000 code changes in this PR |
|
Acknowledged. I corrected the bad rebase and reduced the PR surface:
If you want me to force-correct this PR branch to the minimal diff, I can either:
|
georgewrmarshall
force-pushed
the
cursor/enum-shared-type-migration-6cf4
branch
from
73f8e67 to
3b2e5ea
Compare
📖 Storybook Preview |
📖 Storybook Preview |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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 966c791. Configure here.
georgewrmarshall
force-pushed
the
cursor/enum-shared-type-migration-6cf4
branch
from
966c791 to
7e10637
Compare
📖 Storybook Preview |
📖 Storybook Preview |
georgewrmarshall
force-pushed
the
cursor/enum-shared-type-migration-6cf4
branch
from
657178a to
11f4ac2
Compare
📖 Storybook Preview |
📖 Storybook Preview |
| expect(input).toHaveStyle(tw`border-primary-default`); | ||
| }); | ||
|
|
||
| it('clears focused state when input becomes disabled', () => { |
There was a problem hiding this comment.
This focused-then-disabled test captures the user-visible transition that justified the refactor. It verifies the field does not keep stale focused styling after becoming disabled.
| ); | ||
|
|
||
| /* istanbul ignore next: handler body covered by focus/blur tests */ | ||
| useEffect(() => { |
There was a problem hiding this comment.
This is an architectural improvement, not just a test fix. @brianacnguyen We previously relied on istanbul ignore around the old disabled focus guards, which made the file look fully covered without exercising the unreachable branch. I would rather lower coverage on an individual component in that situation so the reported coverage stays honest and leaves us room to come back and refactor the code into something genuinely reachable and testable, which is what this change does.
| import type { InputPropsShared } from '@metamask/design-system-shared'; | ||
| import type { TextInputProps } from 'react-native'; | ||
|
|
||
| export type InputProps = Omit< |
There was a problem hiding this comment.
This keeps the shared contract limited to cross-platform Input props and leaves RN-only TextInput behavior in the platform layer. The split is important because the event and base-prop surfaces still differ meaningfully between native and web.
| # Input | ||
|
|
||
| Input is a light-weight, controlled-only borderless input used inside of TextField. | ||
| Input is a light-weight, controlled-only borderless input for use inside of `TextField`. Use `TextField` instead when you need labels, error states, or accessories. |
There was a problem hiding this comment.
The README now frames Input as a controlled, foundation-level primitive rather than a full field abstraction. That should help first-time reviewers and consumers understand why TextField remains the recommended public entry point for labels, errors, and accessories.
| * Input component shared props (ADR-0004) | ||
| * Platform-independent properties shared across React and React Native | ||
| */ | ||
| export type InputPropsShared = { |
There was a problem hiding this comment.
The shared type is intentionally narrow: only the props that are semantically the same on both platforms moved here. Event handlers and platform base props stayed out of shared because that API convergence belongs at the future TextField layer, not in the low-level primitive.
| import type { InputProps } from './Input.types'; | ||
| import README from './README.mdx'; | ||
|
|
||
| function ControlledInput(props: InputProps) { |
There was a problem hiding this comment.
The local ControlledInput wrapper keeps editable stories interactive after making value part of the required public contract. Without it, the examples would render frozen inputs and hide the intended behavior from reviewers.
| import type { InputPropsShared } from '@metamask/design-system-shared'; | ||
| import type { ComponentPropsWithoutRef } from 'react'; | ||
|
|
||
| export type InputProps = Omit< |
There was a problem hiding this comment.
Removing defaultValue and value from the inherited DOM props is the type-level part of the controlled-only change on web. This keeps consumers from accidentally mixing the old uncontrolled browser surface back into the shared API.
| // TextField types (ADR-0004) | ||
| export { type TextFieldPropsShared } from './types/TextField'; | ||
|
|
||
| // Input types (ADR-0004) |
There was a problem hiding this comment.
This root export matters for adoption more than implementation detail. It lets consumers and wrapper components import the shared Input contract the same way they already import other ADR-0004 shared types.
| @@ -0,0 +1 @@ | |||
| export { type InputPropsShared } from './Input.types'; | |||
There was a problem hiding this comment.
This barrel keeps Input aligned with the shared-type export pattern used throughout the package. It makes the new type discoverable from both the feature folder and the shared package root.
|
|
||
| ## Props | ||
|
|
||
| ### `value` |
There was a problem hiding this comment.
Calling out value as required in the README is important because this is a breaking change from the old web usage pattern. The docs now match the stories and tests, so first-time reviewers can see the controlled-only contract without reading the type definitions.
| @@ -8,7 +8,7 @@ const TEST_ID = 'input'; | |||
|
|
|||
| describe('Input', () => { | |||
| it('renders with default props', () => { | |||
There was a problem hiding this comment.
These test updates are mostly contract-locking for the breaking change: web Input is now treated as controlled-only, so every test passes value explicitly. That makes the new API surface visible in the spec instead of only in the type file.
| @@ -23,11 +24,13 @@ export const Input = forwardRef<HTMLInputElement, InputProps>( | |||
| ) => { | |||
| const mergedClassName = twMerge( | |||
There was a problem hiding this comment.
isStateStylesDisabled is intentionally suppressing the input chrome for focus and disabled states on web so TextField can own those states without double styling. This is the same composition role the native implementation already relied on.
| } | ||
|
|
||
| describe('Input', () => { | ||
| const tw = renderHook(() => useTailwind()).result.current; |
There was a problem hiding this comment.
This rewrite is intentionally moving the native Input tests away from implementation-detail helpers and toward contract-level assertions. Using built-in matchers like toHaveStyle, toBeDisabled, and toBeOnTheScreen keeps the tests closer to real usage and makes the coverage signal more trustworthy.
|
|
||
| ## Version Updates | ||
|
|
||
| ### From version 0.X.0 to 0.X.0 |
There was a problem hiding this comment.
This placeholder section is intentionally staging unreleased Input migration guidance without rewriting shipped 0.22.0 history. The explicit 0.X.0 marker should also make the release-prep version update easy to spot when this PR is included in a real package release.
📖 Storybook Preview |
| onBlur?.(e); | ||
| } | ||
| setIsFocused(false); | ||
| onBlur?.(e); |
There was a problem hiding this comment.
We probably still wait to not focus when it’s disabled no?
There was a problem hiding this comment.
I don’t think we need the extra isDisabled guard in the handlers because native TextInput already owns the interaction blocking through editable={!isDisabled && !isReadOnly}. The remaining responsibility here is just local state correctness: if the field is already focused and then becomes disabled or read-only, we still need to clear isFocused so we don’t keep stale focused styling after the native control stops being editable.
📖 Storybook Preview |
## Release 38.0.0 This release updates the shared, web, native, tokens, and Tailwind packages with new cross-platform input and avatar-group contracts, new modal building blocks for React, a breaking React Native Toast API redesign, and Tailwind CSS v4 support for web consumers. ### 📦 Package Versions - `@metamask/design-system-shared`: **0.16.0** - `@metamask/design-system-react`: **0.21.0** - `@metamask/design-system-react-native`: **0.23.0** - `@metamask/design-system-tailwind-preset`: **0.7.0** - `@metamask/design-tokens`: **8.4.0** ### 🔄 Shared Type Updates (0.16.0) #### Input and AvatarGroup shared contracts ([#1043](#1043), [#1067](#1067)) **What Changed:** - Added shared `Input` contracts for controlled `value`, `isReadOnly`, and `isStateStylesDisabled` - Added shared `AvatarGroup` size, variant, and prop contracts - Added the shared `Merge` icon export ([#1155](#1155)) **Impact:** - React and React Native consumers can build against one aligned input and avatar-group API surface - Cross-platform wrappers can depend on `@metamask/design-system-shared` instead of maintaining platform-specific type assumptions ### 🌐 React Web Updates (0.21.0) #### Added - Added `ModalOverlay`, `ModalBody`, `ModalFocus`, and `ModalFooter` to support Extension modal migrations into `@metamask/design-system-react` ([#1120](#1120), [#1121](#1121), [#1128](#1128), [#1132](#1132)) - Added the `Merge` icon to the React icon set ([#1155](#1155)) #### Changed - Updated `Input` to follow the shared controlled input API and use `isReadOnly` as the public readonly prop name ([#1043](#1043)) - Updated `AvatarGroup` to use shared cross-platform size and variant contracts ([#1067](#1067)) ### 📱 React Native Updates (0.23.0) #### Added - Added the `Merge` icon to the React Native icon set ([#1155](#1155)) #### Changed - **BREAKING:** Redesigned `Toast` to use a single mounted `<Toast />` plus static `Toast.show(...)` and `Toast.hide()` methods for application usage ([#1104](#1104)) - Removed `ToastContext`, `ToastContextWrapper`, and `ToastContextParams` from the public export surface - Renamed `ToastVariants` to `ToastVariant`, changed icon-only close buttons to `ToastCloseButtonVariant.Icon`, and renamed `customBottomOffset` to `bottomOffset` - `Toast.show()` and `Toast.hide()` now throw a descriptive error if called before `<Toast />` mounts - See the [React Native Migration Guide](./packages/design-system-react-native/MIGRATION.md#from-version-0220-to-0230) - Updated `Input` to follow the shared controlled input API and rename `isReadonly` to `isReadOnly` ([#1043](#1043)) - Updated `AvatarGroup` to use shared cross-platform size and variant contracts ([#1067](#1067)) ### 🎨 Tokens and Tailwind Updates #### `@metamask/design-tokens` 8.4.0 - Added `@metamask/design-tokens/tailwind/theme.css` for Tailwind CSS v4 consumers, providing a single import for token variables, theme values, typography, fonts, and shadow utilities ([#1117](#1117)) #### `@metamask/design-system-tailwind-preset` 0.7.0 - Added a `fade-in` animation utility so consumers can use `animate-fade-in` for simple opacity entrance transitions, including the new `ModalOverlay` web migration path ([#1120](#1120)) - Clarified that Tailwind CSS v3 consumers should keep using this preset, while Tailwind CSS v4 consumers should migrate to `@metamask/design-tokens/tailwind/theme.css` ([#1117](#1117)) ###⚠️ Breaking Changes #### Toast API redesign (React Native) **What Changed:** - `Toast` application usage moved from context/service patterns to static `Toast.show(...)` and `Toast.hide()` methods - `ToastContext`, `ToastContextWrapper`, and `ToastContextParams` were removed from the public API - `ToastVariants` was renamed to `ToastVariant` - Icon-only close buttons now use `ToastCloseButtonVariant.Icon` - `customBottomOffset` was renamed to `bottomOffset` **Impact:** - Existing `@metamask/design-system-react-native` consumers using the old Toast context flow need import, root-mount, and call-site updates - Existing app code must ensure `<Toast />` is mounted before invoking `Toast.show()` / `Toast.hide()` See migration guides for complete instructions: - [React Native Migration Guide](./packages/design-system-react-native/MIGRATION.md#from-version-0220-to-0230) ### ✅ Checklist - [x] Changelogs updated with human-readable descriptions - [x] Changelog validation passed (`yarn workspace <package> changelog:validate`) - [x] Version bumps follow semantic versioning - [x] design-system-shared: minor (`0.15.0` → `0.16.0`) - new shared `Input`, `AvatarGroup`, and icon exports - [x] design-system-react: minor (`0.20.0` → `0.21.0`) - new modal components, icon, and shared API alignment - [x] design-system-react-native: minor (`0.22.0` → `0.23.0`) - breaking Toast redesign, icon, and shared API alignment - [x] design-system-tailwind-preset: minor (`0.6.1` → `0.7.0`) - new `fade-in` utility and Tailwind CSS v4 migration guidance - [x] design-tokens: minor (`8.3.0` → `8.4.0`) - Tailwind CSS v4 `theme.css` export - [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** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) - [ ] 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 with examples - [ ] All unreleased changes are accounted for in changelogs
2 participants
Description
This PR migrates
Inputto align with ADR-0003 (String Unions) and ADR-0004 (Centralized Types Architecture) as part of epicDSYS-468.It also folds in the breaking API alignment work we decided to land with this migration instead of deferring:
Inputis now controlled-only on both React and React NativeisReadonlyis renamed toisReadOnlyisStateStylesDisabledis available on both platformsChanges
Shared types
In
@metamask/design-system-shared:TextVariantis centralized as the ADR-0003 const-object + string-union export used by both platformsInputPropsSharedis added and exported from the shared package rootInputPropsSharednow contains the shared cross-platformInputcontract:value: stringtextVariantisDisabledisReadOnlyisStateStylesDisabledReact
InputIn
@metamask/design-system-react:Input.types.tsnow extendsInputPropsShareddefaultValueis removed from the publicInputtype surfacereadOnly/isReadonlyalignment is normalized toisReadOnlyisStateStylesDisablednow suppresses the input's own focus/disabled state classesvaluepropsReact Native
InputIn
@metamask/design-system-react-native:Input.types.tsnow extendsInputPropsSharedInputremains controlled-only and now shares the same readonly/state-style prop names as webTextInput.editable, while local focused state is explicitly cleared when the input becomes disabled or read-onlyBreaking changes
Consumers should expect these
InputAPI changes:valueis required on webInput; uncontrolleddefaultValueusage is no longer part of the public type contractisReadonlyis renamed toisReadOnlyisStateStylesDisabledis now the shared prop name across both platformsRelated issues
Fixes: DSYS-488
Manual testing steps
yarn buildyarn lintyarn testyarn storybookyarn test:storybookTextVariantis importable from@metamask/design-system-sharedInputPropsSharedis importable from@metamask/design-system-sharedInputstories remain editable where intendedInputclears focused styling when it becomes disabled or read-onlyCoverage note
The native
Inputbranch coverage gap from the old disabled focus guards is resolved withoutistanbul ignorecomments.Input.tsxwas reworked so the disabled/read-only behavior is simpler and the resulting focused coverage run for that file is100%for statements, branches, functions, and lines.Screenshots/Recordings
After
after720.mov
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Medium Risk
Introduces breaking API changes to
Inputacross web and native (controlled-onlyvalue,isReadonly→isReadOnly, newisStateStylesDisabled) and tweaks native focus handling, which can impact downstream form wrappers and styling.Overview
Aligns
Inputacross@metamask/design-system-reactand@metamask/design-system-react-nativeto a new sharedInputPropsSharedcontract exported from@metamask/design-system-shared.Breaking API alignment: web
Inputis now controlled-only (public types dropdefaultValueand requirevalue),isReadonlyis renamed toisReadOnly, andisStateStylesDisabledis supported on both platforms (suppresses focus/disabled styling).Updates docs, stories, and tests to match the new contract, and adjusts React Native
Inputfocus state handling to clear focused styling when the input becomes disabled or read-only.Reviewed by Cursor Bugbot for commit 554ea6e. Bugbot is set up for automated code reviews on this repo. Configure here.