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

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

packages/react/src/Dialog/Dialog.dev.stories.tsx

@@ -137,7 +137,7 @@ export const WithSx = ({width, height, subtitle}: DialogStoryProps) => {
            {buttonType: 'danger', content: 'Delete the universe', onClick: onDialogClose},
            {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-actions bot added the integration-tests: recommended label Dec 13, 2024

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

github-actions bot temporarily deployed to storybook-preview-5436 December 13, 2024 16:27 Inactive

TylerJDev added the update snapshots label Dec 13, 2024

github-actions bot removed the update snapshots label Dec 13, 2024

primer bot temporarily deployed to github-pages December 13, 2024 16:48 Inactive

github-actions bot temporarily 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 tbenning, camertron and langermank 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-actions bot temporarily deployed to storybook-preview-5436 January 2, 2025 21:33 Inactive

This comment was marked as spam.

Sign in to view

github-actions bot 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 94a8eb0 Compare March 17, 2025 23:06

TylerJDev added the update snapshots label Mar 17, 2025

test(vrt): update snapshots

01c3f09

github-actions bot temporarily deployed to storybook-preview-5436 March 17, 2025 23:41 Inactive

primer bot deployed to github-pages March 18, 2025 12:54 View deployment

github-actions bot temporarily 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-actions bot temporarily deployed to storybook-preview-5436 March 18, 2025 14:15 Inactive

TylerJDev added the update snapshots label Mar 18, 2025

test(vrt): update snapshots

3a39dd9

github-actions bot removed the update snapshots label Mar 18, 2025

primer bot deployed to github-pages March 18, 2025 18:43 View deployment

github-actions bot temporarily 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-actions bot temporarily 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 label Mar 19, 2025

github-actions bot temporarily deployed to storybook-preview-5436 March 19, 2025 12:27 Inactive

test(vrt): update snapshots

bdfdc66

github-actions bot removed the update snapshots label Mar 19, 2025

primer bot deployed to github-pages March 19, 2025 12:53 View deployment

github-actions bot deployed to storybook-preview-5436 March 19, 2025 12:53 View deployment

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 11b8d4c Mar 20, 2025
44 checks passed

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

primer bot mentioned this pull request Mar 20, 2025

Release tracking #5802

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

TylerJDev commented Dec 13, 2024 •

edited

Loading

changeset-bot bot commented Dec 13, 2024 •

edited

Loading

github-actions bot commented Dec 13, 2024

github-actions bot commented Dec 13, 2024 •

edited

Loading

TylerJDev commented Dec 13, 2024

langermank left a comment

broccolinisoup commented Dec 17, 2024

This comment was marked as spam.

github-actions bot commented Mar 9, 2025

TylerJDev Mar 19, 2025

langermank left a comment

Dialog: Show Tooltip on "close" button #5436

Dialog: Show Tooltip on "close" button #5436

Conversation

TylerJDev commented Dec 13, 2024 • edited Loading

Changelog

Removed

Rollout strategy

Testing & Reviewing

Merge checklist

changeset-bot bot commented Dec 13, 2024 • edited Loading

🦋 Changeset detected

github-actions bot commented Dec 13, 2024

github-actions bot commented Dec 13, 2024 • edited Loading

size-limit report 📦

TylerJDev commented Dec 13, 2024

langermank left a comment

Choose a reason for hiding this comment

broccolinisoup commented Dec 17, 2024

This comment was marked as spam.

github-actions bot commented Mar 9, 2025

TylerJDev Mar 19, 2025

Choose a reason for hiding this comment

langermank left a comment

Choose a reason for hiding this comment

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

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

TylerJDev commented Dec 13, 2024 •

edited

Loading

changeset-bot bot commented Dec 13, 2024 •

edited

Loading

github-actions bot commented Dec 13, 2024 •

edited

Loading