Skip to content

Prevents inactive indicator icon/tooltip from appearing in ActionMenu items - #4770

Merged
broccolinisoup merged 3 commits into
mainfrom
mp/fix-inactive-indicators-in-menus
Jul 24, 2024
Merged

Prevents inactive indicator icon/tooltip from appearing in ActionMenu items#4770
broccolinisoup merged 3 commits into
mainfrom
mp/fix-inactive-indicators-in-menus

Conversation

@mperrotti

@mperrottimperrotti commented Jul 23, 2024

Copy link
Copy Markdown
Contributor

No issue exists for this bug. I think the bug was introduced as the result of a bad merge.

Screenshots

Before:

Kapture.2024-07-23.at.13.51.25.mp4

After:

Kapture.2024-07-23.at.13.53.27.mp4

Changelog

New

Changed

  • Inactive ActionMenu items no longer render a ⚠️ icon w/ the inactive warning message. The warning message is only rendered at the bottom of the ActionMenu item

Removed

Rollout strategy

  • Patch release
  • Minor release
  • Major release; if selected, include a written rollout or migration plan
  • None; if selected, include a brief description as to why

Testing & Reviewing

  • For ActionList items: Ensure that the ⚠️ icon still appears and shows a tooltip on hover
  • For ActionMenu and NavList items: Ensure that no⚠️ icon appears - only warning text at the bottom of the item

Merge checklist

  • [n/a] Added/updated tests
    • This will be caught by visual regression tests
  • [n/a] Added/updated documentation
  • [n/a] Added/updated previews (Storybook)
  • Changes are SSR compatible
  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari
  • Tested in Edge
  • (GitHub staff only) Integration tests pass at github/github (Learn more about how to run integration tests)

@mperrotti
mperrotti requested a review from a team as a code ownerJuly 23, 2024 17:59
@changeset-bot

changeset-botBot commented Jul 23, 2024•
edited
Loading

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 8c4fedf

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

This PR includes changesets to release 1 package
NameType
@primer/reactPatch

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

@github-actions

github-actionsBot commented Jul 23, 2024•
edited
Loading

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
packages/react/dist/browser.esm.js92.48 KB (+0.13% 🔺)
packages/react/dist/browser.umd.js92.68 KB (-0.01% 🔽)

@github-actions
github-actionsBottemporarily deployed to storybook-preview-4770 July 23, 2024 18:03 Inactive
broccolinisoup
broccolinisoup approved these changes Jul 24, 2024
View reviewed changes

@broccolinisoupbroccolinisoup left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wonderful! Thanks for fixing this 🙌🏻

@broccolinisoupbroccolinisoup added the update snapshots 🤖 Command that updates VRT snapshots on the pull request label Jul 24, 2024
@primer
primerBot requested a review from a team as a code ownerJuly 24, 2024 05:09
@primer
primerBot requested a review from maximedegreveJuly 24, 2024 05:09
@github-actionsgithub-actionsBot removed the update snapshots 🤖 Command that updates VRT snapshots on the pull request label Jul 24, 2024
@github-actions
github-actionsBottemporarily deployed to storybook-preview-4770 July 24, 2024 05:13 Inactive
@broccolinisoup
broccolinisoup added this pull request to the merge queueJul 24, 2024
Merged via the queue into main with commit 8574027Jul 24, 2024
@broccolinisoup
broccolinisoup deleted the mp/fix-inactive-indicators-in-menus branch July 24, 2024 23:26
@primerprimerBot mentioned this pull request Jul 24, 2024
Merged
@Mapuppy09Mapuppy09 mentioned this pull request Aug 19, 2024
Merged
This was referenced Sep 13, 2024
Open
Open
Open
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@broccolinisoupbroccolinisoupbroccolinisoup approved these changes
@maximedegrevemaximedegreveAwaiting requested review from maximedegrevemaximedegreve was automatically assigned from primer/design-reviewers
+1 more reviewer
@langermanklangermanklangermank approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mperrotti@broccolinisoup@langermank