Skip to content

Replace Alert component "other" as default style#10779

Merged
aduth merged 2 commits intomainfrom
aduth-alert-other-default
Jun 21, 2024
Merged

Replace Alert component "other" as default style#10779
aduth merged 2 commits intomainfrom
aduth-alert-other-default

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Jun 7, 2024

🛠 Summary of changes

Updates AlertComponent to replace the explicit :other variant to be used as the default, instead of :info.

For additional context, refer to 18F/identity-design-system#449

Stylistically, there is visual difference in the previous type: :other compared with the new default appearance, since we never actually applied the usa-alert--other class. This works to our benefit, since there's effectively no change as a result of the upcoming design system release, and no logic changes needed to remove the application of a usa-alert--other class.

As part of these changes, I searched code for existing usage of AlertComponent not specifying type with the assumption of rendering an :info alert banner and passed an explicit type: :info to avoid regressions.

📜 Testing Plan

Verify there are no regressions in the impacted code previously assuming default :info alert type.

Verify the "Other" type styles are now shown as the "Default" (type: nil) styles in the component preview:

  1. Go to https://review-aduth-aler-cgle14.review-app.identitysandbox.gov/components/inspect/alert/preview (local: http://localhost:3000/components/inspect/alert/preview)

👀 Screenshots

Before After
before after

@aduth aduth requested a review from nickttng June 7, 2024 18:09
@aduth aduth force-pushed the aduth-alert-other-default branch from 88a7d08 to 001f103 Compare June 21, 2024 12:23
@aduth aduth merged commit f86f48d into main Jun 21, 2024
@aduth aduth deleted the aduth-alert-other-default branch June 21, 2024 12:41
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