Skip to content

Reimplemented tooltips using the Popover API. - #2804

Closed
radglob wants to merge 48 commits into
mainfrom
tooltip-popover-api
Closed

Reimplemented tooltips using the Popover API.#2804
radglob wants to merge 48 commits into
mainfrom
tooltip-popover-api

Conversation

@radglob

@radglobradglob commented Jan 23, 2023

Copy link
Copy Markdown
Contributor

Reimplemented the Tooltip component using the Popover API. This should not change anything about the tooltip component from a user's perspective.

Bring this in line with the PVC implementation, this doesn't support the align prop. Users should use the diagonal directions for this instead.

Since the Popover API is not yet available in stable browsers, this includes a polyfill as well as an update to styled-components from 4.x to 5.x to take advantage of the shouldForwardProp feature. That update will block merging to main, since we would need to update all dependent applications to use 5.x. Primer currently supports either 4.x or 5.x.

Closes#2124.

Screenshots

Please provide before/after screenshots for any visual changes

Merge checklist

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

@changeset-bot

changeset-botBot commented Jan 23, 2023

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 505dbc5

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

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

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

colebemisand others added 3 commits January 25, 2023 15:26
* Revert Truncate
* Revert Label
* Fix lint error
* Revert Avatar and Flash
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@radglob
radglob marked this pull request as ready for review January 25, 2023 20:28
@radglob
radglob requested review from a team and josepmartinsJanuary 25, 2023 20:28
@radglob

Copy link
Copy Markdown
ContributorAuthor

This is going to be held up for the moment. There's a conflict in transitive dependencies (specifically some packages that storybook uses) that can't be updated without bumping a major version on Storybook.

@radglobradglob added the 🚧 blocked Someone or something is preventing this from moving forward label Jan 27, 2023
@mperrottimperrotti mentioned this pull request Jan 27, 2023
6 tasks
@github-actions

github-actionsBot commented Feb 7, 2023

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
dist/browser.esm.js91.08 KB (+1.56% 🔺)
dist/browser.umd.js91.15 KB (+1.55% 🔺)

@radglob
radglob temporarily deployed to github-pages February 8, 2023 15:19 — with GitHub Actions Inactive
@github-actions
github-actionsBot temporarily deployed to storybook-preview-2804 February 8, 2023 15:19 Inactive
@radglob
radglob temporarily deployed to github-pages February 8, 2023 15:30 — with GitHub Actions Inactive
@github-actions
github-actionsBottemporarily deployed to storybook-preview-2804 February 8, 2023 15:30 Inactive
@github-actions

Copy link
Copy Markdown
Contributor

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.

@github-actions
github-actionsBot deleted the tooltip-popover-api branch April 16, 2023 17:04
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🚧 blockedSomeone or something is preventing this from moving forwardStale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tooltip should use Popover API to avoid z-index issues

4 participants

@radglob@colebemis@broccolinisoup@primer-css