Dialog: Show `Tooltip` on "close" button by TylerJDev · Pull Request #5436 · primer/react

TylerJDev · 2024-12-13T16:24:52Z

Part of https://github.com/github/primer/issues/3726

Removes unsafeDisableTooltip from IconButton in Dialog, and removes eslint disable along with it.

Dialog component showcasing the dialog's close button with a tooltip. The tooltip is positioned above the button, with the text 'Close'.

Changelog

Removed

Removed unsafeDisableTooltip

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

Merge checklist

Added/updated tests
Added/updated documentation
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)

changeset-bot · 2024-12-13T16:24:57Z

🦋 Changeset detected

Latest commit: bdfdc66

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

github-actions · 2024-12-13T16:25:25Z

👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks!

github-actions · 2024-12-13T16:28:10Z

size-limit report 📦

Path	Size
packages/react/dist/browser.esm.js	105.94 KB (+0.04% 🔺)
packages/react/dist/browser.umd.js	106.23 KB (+0.09% 🔺)

TylerJDev · 2024-12-13T16:50:05Z

Some of the snapshots look a bit odd due to the tooltip placement, but I believe this is just something with how the snapshots were taken (maybe timing?) and not an actual issue with the tooltip, as it cannot be produced locally.

langermank

@TylerJDev those snapshots are kind of scary! Could we maybe adjust the test to add a short delay? Do you think that's the issue? 🤔

broccolinisoup · 2024-12-17T04:54:02Z

@TylerJDev @langermank sorry to barge in 🙃 I saw the Slack notification and wanted to mention that we have one open issue regarding Tooltip misplacement on Dialogs that we probably need to fix it first 😬 #4734

github-actions · 2025-03-09T04:04:53Z

Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days.

TylerJDev · 2025-03-19T16:51:46Z

 {buttonType: 'primary', content: 'Proceed'},
 ]}
- sx={{borderColor: 'accent.emphasis', borderWidth: '1px', borderStyle: 'solid'}}
+ sx={{borderColor: 'accent.emphasis', borderWidth: '1px', borderStyle: 'solid', animation: 'none !important'}}


There was a very small difference between the dev component snapshots (0.02 pixel difference). It seems like this was caused by the animation, but only affected the colorblind theme.
It would sometimes pass/fail, so I marked it off as being flaky via the custom CSS. It never affected the non-dev stories.

langermank

Excellent work solving this very weird, annoying problem!

github-actionsBot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Dec 13, 2024

TylerJDev temporarily deployed to github-pages December 13, 2024 16:26 — with GitHub Actions Inactive

github-actionsBottemporarily deployed to storybook-preview-5436 December 13, 2024 16:27 Inactive

TylerJDev added the update snapshots 🤖 Command that updates VRT snapshots on the pull request label Dec 13, 2024

github-actionsBot removed the update snapshots 🤖 Command that updates VRT snapshots on the pull request label Dec 13, 2024

primerBottemporarily deployed to github-pages December 13, 2024 16:48 Inactive

github-actionsBottemporarily deployed to storybook-preview-5436 December 13, 2024 16:48 Inactive

TylerJDev marked this pull request as ready for review December 13, 2024 16:55

TylerJDev requested review from a team as code owners December 13, 2024 16:55

TylerJDev requested review from camertron, langermank and tbenning December 13, 2024 16:55

langermank approved these changes Dec 16, 2024
View reviewed changes

TylerJDev deployed to github-pages January 2, 2025 21:33 — with GitHub Actions View deployment

github-actionsBottemporarily deployed to storybook-preview-5436 January 2, 2025 21:33 Inactive

This comment was marked as spam.
Sign in to view

github-actionsBot added the Stale label Mar 9, 2025

TylerJDev removed the Stale label Mar 10, 2025

TylerJDev added 2 commits March 17, 2025 19:05

Remove eslint disable, show tooltip
ad661e8

Add small delay
94a8eb0

TylerJDev force-pushed the tylerjdev/remove-unsafetooltip-dialog branch from a3c0fcf to 94a8eb0Compare March 17, 2025 23:06

TylerJDev added the update snapshots 🤖 Command that updates VRT snapshots on the pull request label Mar 17, 2025

test(vrt): update snapshots
01c3f09

github-actionsBottemporarily deployed to storybook-preview-5436 March 17, 2025 23:41 Inactive

primerBotdeployed to github-pages March 18, 2025 12:54 View deployment

github-actionsBottemporarily deployed to storybook-preview-5436 March 18, 2025 12:54 Inactive

Merge branch 'main' into tylerjdev/remove-unsafetooltip-dialog
90fba3d

TylerJDev deployed to github-pages March 18, 2025 14:15 — with GitHub Actions View deployment

github-actionsBottemporarily deployed to storybook-preview-5436 March 18, 2025 14:15 Inactive

TylerJDev added the update snapshots 🤖 Command that updates VRT snapshots on the pull request label Mar 18, 2025

test(vrt): update snapshots
3a39dd9

github-actionsBot removed the update snapshots 🤖 Command that updates VRT snapshots on the pull request label Mar 18, 2025

primerBotdeployed to github-pages March 18, 2025 18:43 View deployment

github-actionsBottemporarily deployed to storybook-preview-5436 March 18, 2025 18:44 Inactive

Add small timeout
f7c3ac1

TylerJDev deployed to github-pages March 18, 2025 19:08 — with GitHub Actions View deployment

github-actionsBottemporarily deployed to storybook-preview-5436 March 18, 2025 19:08 Inactive

Fix flakey test
3c66659

TylerJDev deployed to github-pages March 19, 2025 12:27 — with GitHub Actions View deployment

TylerJDev added the update snapshots 🤖 Command that updates VRT snapshots on the pull request label Mar 19, 2025

github-actionsBottemporarily deployed to storybook-preview-5436 March 19, 2025 12:27 Inactive

test(vrt): update snapshots
bdfdc66

github-actionsBot removed the update snapshots 🤖 Command that updates VRT snapshots on the pull request label Mar 19, 2025

primerBotdeployed to github-pages March 19, 2025 12:53 View deployment

github-actionsBottemporarily deployed to storybook-preview-5436 March 19, 2025 12:53 Inactive

TylerJDev commented Mar 19, 2025
View reviewed changes

TylerJDev requested a review from langermank March 19, 2025 16:52

langermank approved these changes Mar 19, 2025
View reviewed changes

TylerJDev added this pull request to the merge queue Mar 20, 2025

Merged via the queue into main with commit 11b8d4cMar 20, 2025

TylerJDev deleted the tylerjdev/remove-unsafetooltip-dialog branch March 20, 2025 21:12

primerBot mentioned this pull request Mar 20, 2025
Release tracking (37.17.0) #5802
Merged

Navigation Menu

Uh oh!

Dialog: Show `Tooltip` on "close" button - #5436

Dialog: Show `Tooltip` on "close" button#5436
TylerJDev merged 8 commits into
mainfrom
tylerjdev/remove-unsafetooltip-dialog

TylerJDev commented Dec 13, 2024•
edited
Loading

Uh oh!

changeset-botBot commented Dec 13, 2024•
edited
Loading

Uh oh!

github-actionsBot commented Dec 13, 2024

Uh oh!

github-actionsBot commented Dec 13, 2024•
edited
Loading

Uh oh!

TylerJDev commented Dec 13, 2024

Uh oh!

langermank left a comment

Uh oh!

broccolinisoup commented Dec 17, 2024

Uh oh!

This comment was marked as spam.

Uh oh!

github-actionsBot commented Mar 9, 2025

Uh oh!

TylerJDevMar 19, 2025

Uh oh!

langermank left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

TylerJDev commented Dec 13, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog

Removed

Rollout strategy

Testing & Reviewing

Merge checklist

Uh oh!

changeset-botBot commented Dec 13, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actionsBot commented Dec 13, 2024

Uh oh!

github-actionsBot commented Dec 13, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

size-limit report 📦

Uh oh!

TylerJDev commented Dec 13, 2024

Uh oh!

langermank left a comment

Choose a reason for hiding this comment

Uh oh!

broccolinisoup commented Dec 17, 2024

Uh oh!

This comment was marked as spam.

Uh oh!

github-actionsBot commented Mar 9, 2025

Uh oh!

TylerJDevMar 19, 2025

Choose a reason for hiding this comment

Uh oh!

langermank left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

TylerJDev commented Dec 13, 2024•
edited
Loading

changeset-botBot commented Dec 13, 2024•
edited
Loading

github-actionsBot commented Dec 13, 2024•
edited
Loading