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

Remove SelectPanel's backdrop #3027

Merged
merged 3 commits into from
Aug 23, 2024
Merged

Remove SelectPanel's backdrop #3027

merged 3 commits into from
Aug 23, 2024

Conversation

camertron
Copy link
Contributor

@camertron camertron commented Aug 22, 2024

What are you trying to accomplish?

The SelectPanel component should not render a backdrop. See: https://github.slack.com/archives/C06F2US6HU6/p1724361897840679

Screenshots

Before After
A screenshot of the SelectPanel playground preview showing an open panel with a light gray backdrop A screenshot of the SelectPanel playground preview showing an open panel with no backdrop

Integration

No changes necessary in production.

List the issues that this change affects.

Closes https://github.com/github/primer/issues/3878

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.

Accessibility

  • No new axe scan violation - This change does not introduce any new axe scan violations.

Merge checklist

- [ ] Added/updated tests
- [ ] Added/updated documentation
- [ ] Added/updated previews (Lookbook)

  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari
  • Tested in Edge

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

Copy link

changeset-bot bot commented Aug 22, 2024

🦋 Changeset detected

Latest commit: 6969c2e

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

@camertron camertron marked this pull request as ready for review August 22, 2024 22:13
@camertron camertron requested review from a team as code owners August 22, 2024 22:13
@camertron camertron requested review from lukasoppermann and siddharthkp and removed request for a team August 22, 2024 22:13
Copy link
Contributor

⚠️ Visual differences found

Our visual comparison tests found UI differences. Please review the differences by viewing the files changed tab to ensure that the changes were intentional.

Review visual differences

@github-actions github-actions bot requested a review from a team as a code owner August 22, 2024 22:16
Copy link
Member

@siddharthkp siddharthkp left a comment

Choose a reason for hiding this comment

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

LGTM!

Makes sense for the anchored variant.

Not sure if we have a modal variant already, would be useful to check if that one should have a backdrop

@camertron
Copy link
Contributor Author

@siddharthkp you're absolutely right 😄 We actually don't support a modal variant at the moment, but we'll need to consider adding a backdrop if/when we do.

@camertron camertron merged commit 5a205cd into main Aug 23, 2024
40 checks passed
@camertron camertron deleted the no_select_panel_backdrop branch August 23, 2024 17:58
@primer primer bot mentioned this pull request Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants