Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NO-TICKET: Expose 'informational' as part of the AlertVariation type #3301

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Josh68
Copy link
Contributor

@Josh68 Josh68 commented Oct 29, 2024

Summary

When using Design System types, directly, in a project, the AlertVariation type is missing the string for the default Alert style, which is "informational," according to the docs. This update just adds that string to the union, but excludes it from the prop typing, since it's the default.

We are passing this detail via Adobe Analytics, so would like to have the "informational" typing exposed.

How to test

There is no impact to the use of the component. When built, a project that imports AlertVariation from Design System types should see "informational" as one of the options (in our case, for analytics purposes).

Checklist

  • Prefixed the PR title with the Jira ticket number as [WNMGDS-####] Title or [NO-TICKET] if this is unticketed work.
  • Selected appropriate Type (only one) label for this PR, if it is a breaking change, label should only be Type: Breaking
  • Selected appropriate Impacts, multiple can be selected.
  • Selected appropriate release milestone

If this is a change to design:

  • If visual regression image references have been changed, design MUST be assigned to review. In this instance, designer approval is a requirement before the PR can be merged.

If this is a change to code:

  • Created or updated unit tests to cover any new or modified code
  • If necessary, updated unit-test snapshots (yarn test:unit:update) and browser-test snapshots (yarn test:browser:update)

If this is a change to documentation:

  • Checked for spelling and grammatical errors

When using Design System types, directly, in a project, the AlertVariation type is missing the string for the default Alert style, which is "informational," according to the docs. This update just adds that string to the union, but excludes it from the prop typing, since it's the default.
@Josh68 Josh68 changed the title Explose 'informational' as part of the AlertVariation type NO-TICKET: Explose 'informational' as part of the AlertVariation type Oct 29, 2024
@Josh68 Josh68 changed the title NO-TICKET: Explose 'informational' as part of the AlertVariation type NO-TICKET: Expose 'informational' as part of the AlertVariation type Oct 29, 2024
@kim-cmsds
Copy link
Collaborator

Jenkins, not a wolf

@jack-ryan-nava-pbc
Copy link
Collaborator

@kim-cmsds @tamara-corbalt This seems like a perfectly fine thing to include. Probably we want to spin up another PR that replicates the changes here and includes the appropriate labels and milestones. What do you too think?

Thanks for this suggestion @Josh68. If I understand this change correctly it seems that the benefit of this inclusion will be available to Typescript for hints?

@Josh68
Copy link
Contributor Author

Josh68 commented Dec 18, 2024

Thanks for this suggestion @Josh68. If I understand this change correctly it seems that the benefit of this inclusion will be available to Typescript for hints?

Correct, @jack-ryan-nava-pbc. Wherever possible (and hopefully always), we want to use types for design system components from your repo. In this case, for sending alert interaction events to analytics, we want to send the variation. While informational is a valid variation, it's the default and isn't exposed publicly, It should be exposed.

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