Skip to content

[Design System] Toast supports actions#63

Merged
metonym merged 1 commit intodevfrom
toast-interactions
Jul 27, 2025
Merged

[Design System] Toast supports actions#63
metonym merged 1 commit intodevfrom
toast-interactions

Conversation

@metonym
Copy link
Contributor

@metonym metonym commented Jul 26, 2025

Stacked on #62, but can be merged directly to dev.

Addresses https://app.asana.com/1/1108348036672214/project/1210891919698604/task/1210891919700765

Figma reference: https://www.figma.com/design/lrko7TMR4xy6Xb0w5GwzCQ/Common-Sense?node-id=214-1962&m=dev

Introduces an optional actions prop to the Toast component, allowing one or two action buttons to be rendered within toasts. Updates the storybook to demonstrate toasts with action buttons.

Component API Design

  • Add an optional prop to the existing toast object
  • actions is typed as a one-tuple or two-tuple (accepting either one CTA or two). Each item must be a React node, allowing composition
  • If title is not specified, the single-line design is used. The message is truncated, while actions and icon remain full-width.

Screenshot 2025-07-26 at 1 21 30 PM Screenshot 2025-07-26 at 1 29 42 PM Screenshot 2025-07-26 at 3 06 36 PM

@vercel
Copy link
Contributor

vercel bot commented Jul 26, 2025

@metonym is attempting to deploy a commit to the One Project team on Vercel, but is not a member of this team. To resolve this issue, you can:

  • Make your repository public. Collaboration is free for open source and public repositories.
  • Add @metonym as a member. A Pro subscription is required to access Vercel's collaborative features.
    • If you're the owner of the team, click here and add @metonym as a member.
    • If you're the user who initiated this build request, click here to request access.
    • If you're already a member of the One Project team, make sure that your Vercel account is connected to your GitHub account.

To read more about collaboration on Vercel, click here.

@scazan
Copy link
Collaborator

scazan commented Jul 26, 2025

looks, good. Are we also able to support the horizontal mode in this part of the Figma? Screenshot_20250726_232652_Figma.jpg

@metonym metonym force-pushed the toast-interactions branch 3 times, most recently from 3c558a8 to c8fe82e Compare July 26, 2025 22:34
@metonym
Copy link
Contributor Author

metonym commented Jul 26, 2025

@scazan Pushed a commit:

  • If title is not specified, the single-line design is used, which truncates the message.
  • Added stories for various scenarios (non-dismissble, no actions, single action, two actions)

Screenshot 2025-07-26 at 3 06 36 PM Screenshot 2025-07-26 at 3 06 32 PM Screenshot 2025-07-26 at 3 28 27 PM Screenshot 2025-07-26 at 3 34 09 PM

@metonym metonym marked this pull request as ready for review July 27, 2025 16:25
@claude
Copy link

claude bot commented Jul 27, 2025

Claude finished @metonym's task —— View job


Code Review Complete ✅

Reviewed PR #63: [Design System] Toast supports actions

Todo List:

  • Read and analyze Toast.tsx changes
  • Read and analyze Toast.stories.tsx changes
  • Review code quality and best practices
  • Check for potential bugs or issues
  • Evaluate performance considerations
  • Assess security concerns
  • Provide comprehensive feedback with specific references

🎯 Overview

This PR successfully adds action button support to the Toast component with clean implementation and comprehensive test coverage. The single-line design when no title is provided is a nice UX enhancement.

✅ Strengths

Well-Structured Component Architecture

  • Clean separation with ToastWrapper, ToastBody, ToastActions components
  • Good conditional rendering logic for single-line vs multi-line layouts
  • Proper use of cn() utility for conditional styling

Solid TypeScript Implementation

  • Proper tuple typing: actions?: [React.ReactNode, React.ReactNode?] enforces 1-2 actions limit
  • Good component prop interfaces with clear optional/required fields
  • Consistent use of ReactNode for flexible content

Comprehensive Test Coverage

  • Excellent Storybook stories covering all scenarios: no actions, single action, two actions, single-line variants
  • Good edge case testing with long text truncation scenarios

⚠️ Issues to Address

Type Inconsistency (Minor)

// In success toast - packages/ui/src/components/Toast.tsx:147
message?: ReactNode;

// In error toast - packages/ui/src/components/Toast.tsx:186  
message?: string;

Recommendation: Standardize on ReactNode for both to maintain consistency and flexibility.

Missing React Keys (Minor)

The actions array items are rendered without keys in packages/ui/src/components/Toast.tsx:171-172:

{actions && (
  <ToastActions isSingleLine={isSingleLine}>{actions}</ToastActions>
)}

Recommendation: Add keys when mapping over actions to prevent React warnings.

Styling Workaround

There's a TODO comment at packages/ui/src/components/Toast.tsx:193 indicating Tailwind conflicts being resolved with hardcoded RGB values:

// TODO: some odd behavior with Tailwind text-white an text-title-base conflicting here
style: {
  background: 'rgb(203 57 5)', // bg-functional-redBlack equivalent
}

Recommendation: Consider resolving this Tailwind conflict properly rather than using hardcoded values.

🚀 Performance Considerations

  • Good: Short-lived toast components don't need heavy optimization
  • Good: Conditional rendering prevents unnecessary DOM nodes
  • Acceptable: Inline function creation is fine for toast usage patterns

🔒 Security Assessment

  • Low Risk: ReactNode actions provide flexibility but require careful usage
  • Recommendation: Document that actions should not accept unsanitized user input
  • Good: No innerHTML usage, React handles content safely

📋 Minor Suggestions

Component Prop Consistency

Consider adding className support to ToastTitle for consistency (currently has unused className prop):

const ToastTitle = ({ className, children }: { className?: string; children: React.ReactNode }) => {
  return (
    <div className={cn("font-serif text-title-base text-neutral-black", className)}>
      {children}
    </div>
  );
};

Action Validation

Consider adding runtime validation for action elements to ensure they're interactive components.

🎉 Conclusion

This is a well-implemented feature that follows good React patterns and provides excellent flexibility. The implementation is clean, the API design is intuitive, and the test coverage is comprehensive. The identified issues are minor and don't block the functionality.

Approval Status: ✅ Approved - Ready to merge with optional improvements above.

Base automatically changed from ds-audit to dev July 27, 2025 16:36
- Add an optional prop to the existing `toast` object.
- `actions` is typed as a one-tuple or two-tuple (accepting either one CTA or two). Each item must be a React node, allowing composition.
- If `title` is not specified, the single-line design is used. The `message` is truncated, while actions and icon remain full-width.
@metonym metonym force-pushed the toast-interactions branch from c8fe82e to a7b61c0 Compare July 27, 2025 16:42
@metonym
Copy link
Contributor Author

metonym commented Jul 27, 2025

Rebased on dev and made the following revisions based on the Claude Code review:

  • Key the mapped actions
  • Make message also React.ReactNode for error toast for consistency

@metonym metonym merged commit 85e1bb2 into dev Jul 27, 2025
0 of 3 checks passed
@metonym metonym deleted the toast-interactions branch July 27, 2025 16:48
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.

2 participants