Skip to content

fix: Polish React Toast background, description color and border radius - #1392

Open
amandaye0h wants to merge 3 commits into
mainfrom
polish/toast-style-react
Open

fix: Polish React Toast background, description color and border radius#1392
amandaye0h wants to merge 3 commits into
mainfrom
polish/toast-style-react

Conversation

@amandaye0h

@amandaye0h amandaye0h commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Description

Polish React Toast surface styling so it stays aligned with the React Native Toast treatment from #1391.

Why: Toast was always using section background, 12px radius, and default description color, which didn’t match the desired visual spec.

What changed:

  • Light theme: background.default + medium shadow (shadow-md)
  • Dark theme: background.section (no shadow)
  • Theme resolved from the closest [data-theme] ancestor first; falls back to .dark, then document.documentElement (React has no useTheme like RN). Nested data-theme="light" inside a distant .dark ancestor keeps light polish.
  • Description defaults to TextColor.TextAlternative
  • Border radius updated from 12px (rounded-xl) to 16px (rounded-2xl)
  • Included unit tests for theme background/shadow, nested theme precedence, callback refs, radius, and description color

Related issues

Related: #1391

Manual testing steps

  1. Run yarn storybook and open Components → Toast
  2. In light theme, confirm toast uses default background, medium shadow, and 16px corners
  3. In dark theme, confirm toast uses section background with no shadow
  4. In Storybook “both” mode, confirm each panel’s toast matches that panel’s theme
  5. Confirm description text uses the alternative text color
  6. Spot-check Default, severity, action button, and close stories still behave as before

Screenshots/Recordings

Before

Screen.Recording.2026-07-21.at.4.48.58.PM.mov

After

Screen.Recording.2026-07-21.at.4.49.58.PM.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

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

Low Risk
Visual and presentation-only changes to Toast with unit test coverage; no auth, data, or API behavior changes.

Overview
Aligns the React Toast surface with the RN treatment: light uses default background, shadow-md, and 16px corners (rounded-2xl); dark uses section background with no shadow.

Theme is resolved at runtime via resolveIsDarkTheme (closest [data-theme], then .dark, then document.documentElement), with a MutationObserver so changes to theme attributes stay in sync. Nested data-theme="light" inside a .dark ancestor keeps light styling.

Description text now defaults to TextColor.TextAlternative through descriptionProps. Ref forwarding is merged into a callback ref so theme detection still works with object and function refs.

Tests cover light/dark/nested theme classes, description color, and callback refs.

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

Align web Toast with the React Native light/dark surface treatment from #1391.

Co-authored-by: Cursor <cursoragent@cursor.com>
@amandaye0h
amandaye0h requested a review from a team as a code owner July 21, 2026 08:46

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

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 1 potential issue.

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.

Reviewed by Cursor Bugbot for commit 850be70. Configure here.

Comment thread packages/design-system-react/src/components/Toast/Toast.tsx Outdated
@github-actions

Copy link
Copy Markdown
Contributor

📖 Storybook Links

Compare the preview for this pull request with the latest Storybooks from the main branch.

🔀 Pull Request Preview

🌳 Main Branch (Latest)

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

Copy link
Copy Markdown
Contributor

📖 Storybook Links

Compare the preview for this pull request with the latest Storybooks from the main branch.

🔀 Pull Request Preview

🌳 Main Branch (Latest)

Ensure nested light surfaces keep default background and shadow when
inside a .dark ancestor, and cover callback refs plus theme edge cases.

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

Copy link
Copy Markdown
Contributor

📖 Storybook Links

Compare the preview for this pull request with the latest Storybooks from the main branch.

🔀 Pull Request Preview

🌳 Main Branch (Latest)

@georgewrmarshall

georgewrmarshall commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Thanks for picking up the React parity for #1391 — the test coverage for the theme cases is solid.

A few things to flag:

descriptionProps default parameter (same issue as #1391)

descriptionProps = { color: TextColor.TextAlternative },

If a consumer passes any descriptionProps (e.g. { data-testid: 'description' }), the default color is silently dropped. Should be merged in JSX instead:

descriptionProps={{ color: TextColor.TextAlternative, ...descriptionProps }}

Architectural concern — flagged in DSYS-932

This PR is one of the first instances in the design system where a component switches which token it uses based on the current theme (bg-default vs bg-section), rather than relying on a single token to handle its own light/dark values via CSS custom properties.

Historically we haven't done this — the CSS token layer handles theming so components don't need to think about it. There are legitimate cases for this approach, but it carries risk if it spreads without guardrails (see the team message for context). It's also why we haven't needed a theme provider for React web before — and if we start applying colors this way more broadly, we'll need to build one to avoid fragile DOM-inspection patterns like the MutationObserver + closest() approach here.

Tracking the exploration in DSYS-932 — the goal is to establish clear rules for when this is appropriate before it becomes a wider pattern.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants