Skip to content

Conversation

@twjeffery
Copy link
Collaborator

Summary

Updates the Temporary Notification (Snackbar) component to use a
comprehensive token-driven architecture. This component previously had all
styling hardcoded - this PR converts all CSS properties to use design
tokens with proper fallback values for V1 compatibility.

Classification: Token-Only (no version prop needed)

Changes

Token Integration

Converted all hardcoded CSS values to use design tokens with fallback
pattern:

property: var(--goa-temporary-notification-[property], [v1-fallback]);

Example conversions:

  • Border radius: var(--goa-border-radius-m) →
    var(--goa-temporary-notification-borderRadius, var(--goa-border-radius-m))
  • Padding: var(--goa-space-m) var(--goa-space-l) →
    var(--goa-temporary-notification-padding, var(--goa-space-m)
    var(--goa-space-l))
  • Colors: Direct global tokens → Component tokens with fallbacks

Spacing Improvements

Flexbox Gap Structure:

  • Changed from single gap to row-gap and column-gap for better control
  • Row gap: 12px (when elements wrap to new rows)
  • Column gap: 16px (between elements on same row)

Icon Spacing:

  • Implemented negative margin pattern for variable spacing
  • Icon to message: 12px (via icon-margin)
  • Message to action button: 16px (via column-gap)

No Version Prop Required

This component is Token-Only - no structural or behavioral changes between V1 and V2:

  • Same DOM structure
  • Same animation behavior
  • Same notification types (basic, success, failure, indeterminate,
    progress)
  • Visual differences handled entirely through design token values

Technical Implementation

All 22 tokens created with fallback pattern ensures:

  • V1 environments: Use fallback values (preserves exact existing behavior)
  • V2 environments: Use new token values (applies V2 styling)
  • Same component code: Works in both without version prop

Related

@twjeffery twjeffery changed the title feat(#3096: update Temporary Notification component for V2 feat(#3096): update Temporary Notification component for V2 Oct 14, 2025
@twjeffery twjeffery marked this pull request as ready for review October 14, 2025 19:53
@twjeffery twjeffery linked an issue Oct 14, 2025 that may be closed by this pull request
Copy link
Collaborator

@bdfranck bdfranck 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! There are a couple spots where we can keep those tokens.

@twjeffery twjeffery force-pushed the v2-temporary-notification-update branch from c445b0a to 9b67464 Compare October 16, 2025 20:44
@twjeffery
Copy link
Collaborator Author

@bdfranck I committed your suggested changes and squashed it back into one commit for us to merge

Copy link
Collaborator

@bdfranck bdfranck 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! I just have two more questions about the margins around the icon.

@twjeffery twjeffery force-pushed the v2-temporary-notification-update branch from 9b67464 to 1a665b5 Compare October 23, 2025 21:16
@twjeffery
Copy link
Collaborator Author

Looks good! I just have two more questions about the margins around the icon.

@bdfranck I appreciate the review, it should be fixed now, I've pushed updates to both this PR and the related design tokens PR GovAlta/design-tokens#98.

@twjeffery twjeffery requested a review from bdfranck October 23, 2025 21:19
Copy link
Collaborator

@bdfranck bdfranck left a comment

Choose a reason for hiding this comment

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

I looked at the latest changes and see that the icon is properly alined.

Image

Looks good to me! 👍

@twjeffery twjeffery requested a review from chrisolsen November 4, 2025 21:27
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.

Temporary notification 2.0 Update

3 participants