Skip to content

feat: [DSRN] Update KeyValueRow#1023

Open
brianacnguyen wants to merge 11 commits intomainfrom
dsrn/keyvaluepair
Open

feat: [DSRN] Update KeyValueRow#1023
brianacnguyen wants to merge 11 commits intomainfrom
dsrn/keyvaluepair

Conversation

@brianacnguyen
Copy link
Copy Markdown
Contributor

@brianacnguyen brianacnguyen commented Mar 31, 2026

Description

This PR refactors React Native KeyValueRow to match the newer key/value pair API and shared typing (ADR-0003 / ADR-0004).

Why

  • The previous API used nested field / value objects (label config, tooltip, icon sides, etc.), which was heavier than what product patterns need and harder to align with shared cross-platform contracts.
  • The implementation was split across KeyValueRowRoot, KeyValueSection, and KeyValueLabel; the new layout composes BoxHorizontal and Box instead.

What changed

  • @metamask/design-system-shared: adds KeyValueRowVariant (summary → 40px / h-10, input → 48px / h-12) and KeyValueRowPropsShared (keyLabel, value, start/end accessories, optional variant), and exports them from the package root.
  • KeyValueRow (RN): flat props extending the shared shape plus RN-specific ViewProps (minus children), twClassName, keyTextProps / valueTextProps, and keyEndButtonIconProps / valueEndButtonIconProps (when iconName is set, a ButtonIcon is rendered and overrides the corresponding *EndAccessory).
  • Removed: old stub exports (KeyValueRowStubs), tooltip-on-label behavior, and the old field/value icon-side model; internal files KeyValueRowRoot and KeyValueLabel (and the old section-based structure) are removed.
  • Docs & QA: README and Storybook stories updated for the new API; unit tests rewritten to cover layout variants, accessories, button-icon precedence, text defaults, and twClassName merging.

Breaking change

  • Call sites must migrate from field / value objects to keyLabel / value and the new accessory / *ButtonIconProps props. There is no drop-in compatibility with the previous prop shape.

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/DSYS-566


Manual testing steps

  1. From the repo root, run React Native Storybook (e.g. yarn storybook:ios or yarn storybook:android).
  2. Open Components → KeyValueRow and confirm Default, VariantSummary, VariantInput, accessory stories, button icon stories, overflow, and SingleColumnStack look correct.
  3. Run unit tests for the package:
    yarn workspace @metamask/design-system-react-native run jest KeyValueRow
    (or your usual scoped Jest command).

Screenshots/Recordings

Add Storybook before/after or device screenshots if your process requires visual evidence.

Before

After

Simulator.Screen.Recording.-.iPhone.15.Pro.Max.-.2026-04-06.at.17.52.18.mov

Pre-merge author checklist

  • I've followed MetaMask Contributor Docs
  • I've completed the PR template to the best of my ability
  • I’ve included tests if applicable
  • I’ve documented my code using JSDoc format if applicable
  • I’ve applied the right labels on the PR (see labeling guidelines). 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.

Note

Medium Risk
This is a breaking API change to a reusable UI component, removing the old field/value object model (including tooltip/icon-side behavior) and changing exports/types, which can cause widespread downstream compile/runtime UI differences if not fully migrated.

Overview
Refactors React Native KeyValueRow to a new flat, shared-props API. The component now takes keyLabel/value, optional variant (Summary/Input), and start/end accessories, with RN-specific keyTextProps/valueTextProps and keyEndButtonIconProps/valueEndButtonIconProps (rendering a ButtonIcon that overrides the corresponding end accessory).

Removes the previous composable/stub API and related types/constants. Deletes KeyValueRowStubs, section/root/label subcomponents, tooltip support, icon-side enums, and legacy type exports; adds a new KeyValueRowVariant + KeyValueRowPropsShared in @metamask/design-system-shared and re-exports KeyValueRowVariant from the RN package.

Updates Storybook stories, unit tests, and adds a KeyValueRow README plus migration docs for 0.14.0 → 0.15.0 to guide downstream updates.

Reviewed by Cursor Bugbot for commit f1ceb72. Bugbot is set up for automated code reviews on this repo. Configure here.

@brianacnguyen brianacnguyen requested a review from a team as a code owner March 31, 2026 21:12
@brianacnguyen brianacnguyen marked this pull request as draft March 31, 2026 21:12
@github-actions
Copy link
Copy Markdown
Contributor

📖 Storybook Preview

Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

@brianacnguyen brianacnguyen changed the title [DRAFT] KeyValuePair feat: [DSRN] Add KeyValuePair Apr 2, 2026
@brianacnguyen brianacnguyen self-assigned this Apr 2, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

📖 Storybook Preview

@brianacnguyen brianacnguyen marked this pull request as ready for review April 2, 2026 07:23
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

📖 Storybook Preview

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

📖 Storybook Preview

@brianacnguyen brianacnguyen changed the title feat: [DSRN] Add KeyValuePair feat: [DSRN] Update KeyValueRow Apr 7, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

📖 Storybook Preview

amandaye0h
amandaye0h previously approved these changes Apr 7, 2026
Copy link
Copy Markdown
Contributor

@amandaye0h amandaye0h left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Copy link
Copy Markdown
Contributor

@georgewrmarshall georgewrmarshall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ADR-0003/0004 compliance is solid — shared types, const object, correct two-file export pattern, good test coverage. A few things need addressing before merge:

Missing MIGRATION.md — this is a large, breaking API change. The entire field/value object API has been replaced, and the following exports have been removed from the public API:

  • KeyValueRowStubs (sub-component stubs)
  • KeyValueRowFieldIconSides, KeyValueRowSectionAlignments, TooltipSizes, IconSizes (const objects)
  • KeyValueRowTooltip, KeyValueRowField, PreDefinedKeyValueRowLabel, KeyValueRowLabelProps, KeyValueRowRootProps, KeyValueSectionProps (types)

packages/design-system-react-native/MIGRATION.md needs a section showing the before/after API with realistic examples. The release that ships this will also need a coordinated effort to migrate all existing KeyValueRow usages in downstream consumers (extension, mobile) away from the old field/value object API — and any KeyValueColumn usages if applicable. This should be called out explicitly in the migration guide.

Inline comments cover the styling rule violation, stories convention gaps, and README template issues.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

📖 Storybook Preview

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

📖 Storybook Preview

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

📖 Storybook Preview

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants