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

Update to new version of behaviors #3960

Merged
merged 3 commits into from
Nov 21, 2023
Merged

Update to new version of behaviors #3960

merged 3 commits into from
Nov 21, 2023

Conversation

pksjce
Copy link
Collaborator

@pksjce pksjce commented Nov 21, 2023

Closes https://github.com/github/accessibility-audits/issues/4515

Changelog

The calculations to position the action menu next to it's trigger anchor point had a bug where if the available vertical viewport space was lesser than the height of the action menu it would cut off a portion of the top menu above the viewport.

We fixed this by adding tweaks to the calculations and verifying through new and old test cases in the @primer/behaviors repository. In this PR I'm updating to newer version of @primer/behaviors with my fixes.

Before screenshot
Screenshot 2023-11-21 at 5 06 25 pm

After screenshot
Screenshot 2023-11-21 at 5 07 34 pm

Rollout strategy

This will be a patch release since it's a bug fix.

  • Patch release
  • Minor release
  • Major release; if selected, include a written rollout or migration plan

Testing & Reviewing

This can be tested in this storybook. Keep the devtools window open so as to reduce viewport height to be less than action menu height. https://primer.style/react/storybook/?path=/story/components-actionmenu-examples--multiple-sections

Merge checklist

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

@pksjce pksjce requested review from a team and mperrotti November 21, 2023 06:13
Copy link

changeset-bot bot commented Nov 21, 2023

🦋 Changeset detected

Latest commit: c74ad0d

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

This PR includes changesets to release 1 package
Name Type
@primer/react 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

Copy link
Contributor

Uh oh! @pksjce, the image you shared is missing helpful alt text. Check your pull request body.

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

Copy link
Contributor

github-actions bot commented Nov 21, 2023

size-limit report 📦

Path Size
dist/browser.esm.js 104.38 KB (+0.09% 🔺)
dist/browser.umd.js 104.92 KB (+0.08% 🔺)

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.

Left a tiny optional comment, approving in advance!

Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com>
@pksjce pksjce added this pull request to the merge queue Nov 21, 2023
Merged via the queue into main with commit ec8a2ca Nov 21, 2023
29 checks passed
@pksjce pksjce deleted the pk/update-behavior branch November 21, 2023 12:11
@primer primer bot mentioned this pull request Nov 21, 2023
@pksjce
Copy link
Collaborator Author

pksjce commented Nov 21, 2023

Tagging @jonrohan @JelloBagel as they have authored 1.3.5/1.3.6 and 1.40/1.50 respectively

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.

2 participants