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

Make dismiss action on Banner changeable #2455

Merged
merged 1 commit into from
Dec 15, 2023

Conversation

HDinger
Copy link
Contributor

@HDinger HDinger commented Dec 13, 2023

What are you trying to accomplish?

The Alpha::Banner component has the option to dismiss the banner via a button showing an "x". This button has an aria-label attribute with the hard coded text "Dismiss". This text is not changable. This PR aims to make the dismiss action customizable, e.g to open it for translations or different texts if the context requires it.

List the issues that this change affects.

Closes #2453

Risk Assessment

  • Low risk the change is small, highly observable, and easily rolled back.
  • Medium risk changes that are isolated, reduced in scope or could impact few users. The change will not impact library availability.
  • High risk changes are those that could impact customers and SLOs, low or no test coverage, low observability, or slow to rollback.

Merge checklist

  • Added/updated tests
  • Added/updated documentation
  • Added/updated previews (Lookbook)
  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari
  • Tested in Edge

@HDinger HDinger requested review from a team and keithamus December 13, 2023 14:38
Copy link

changeset-bot bot commented Dec 13, 2023

🦋 Changeset detected

Latest commit: cb6fc94

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/view-components Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@HDinger
Copy link
Contributor Author

HDinger commented Dec 13, 2023

Hi @keithamus do you have any hint for me what I can do about the failing CI steps? It seems like they are unrelated or checking the branch from the wrong origin but maybe I am missing something here?

@camertron
Copy link
Contributor

camertron commented Dec 13, 2023

Hey @HDinger, thanks for this!

  1. The system test failure is a known flake. I re-ran it, hopefully it'll pass this time. I'm planning to look into it this week.
  2. I think the Triage / semver label check is failing because you don't have the right permissions. Don't worry about this one.
  3. The Triage / Check for changeset check is failing because this PR doesn't have a changeset. Please run npx changeset in the repo root, follow the prompts, and commit and push the result.

@HDinger
Copy link
Contributor Author

HDinger commented Dec 14, 2023

Hi @camertron

thanks for the feedback! Regarding the changeset: I already added one via npx changeset. Is it possible, that this is due to missing permissions as well?

Copy link
Member

@jonrohan jonrohan left a comment

Choose a reason for hiding this comment

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

nice change 👍🏻

@jonrohan jonrohan merged commit 6ca4ac4 into primer:main Dec 15, 2023
26 of 29 checks passed
@primer primer bot mentioned this pull request Dec 15, 2023
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.

Dismiss action of Alpha::Banner is not changable
3 participants