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

feat(build): Upgrade @popperjs/* to @floating-ui/react-* #67

Merged

Conversation

tulup-conner
Copy link
Collaborator

@tulup-conner tulup-conner commented Apr 27, 2022

Floating UI is the next evolution of Popper.

We currently have a strange bug on Chromium causing Dropdowns/Tooltips to be misaligned with their target. This upgrade happens to resolve #57, as a bonus!

Because Tooltip is used by Dropdown, both components have been upgraded.

There are no breaking changes to the API.

@tulup-conner
Copy link
Collaborator Author

I still need to fix an issue where the tooltip arrow can obstruct content inside the tooltip itself.

@tulup-conner
Copy link
Collaborator Author

Ugh, it looks like the bug is still present, but only in Storybook, and only in Chromium browsers. Because the issue is isolated to Storybook, I'm more inclined it's a bug on their end. Will look into that more.

@tulup-conner tulup-conner force-pushed the feature/upgrade-popper-to-floating-ui branch 2 times, most recently from f131716 to 352a125 Compare April 29, 2022 09:26
src/components/Tooltip/index.tsx Outdated Show resolved Hide resolved
@tulup-conner tulup-conner requested a review from rluders April 29, 2022 22:55
@rluders
Copy link
Collaborator

rluders commented Apr 30, 2022

@tulup-conner may I ask one 'related change'? Could you please change the decorator in this file:

export const decorators = [

It needs to be something like this:

export const decorators = [
  (Story) => (
    <>
      <Style />
      <div className="flex h-screen justify-center items-center">
        <div>
          <Story />
        </div>
      </div>
    </>
  ),
];

The ideia is to centralize all components to the screen, so it would looks nice to preview it, and also 'cause I was having some "issues" (I know that it isn't an issue) to preview the placement to the tooptip when it when out of frame.

Otherwise it looks nice.

These packages replaces `@popperjs/*`.
Replaces `@popperjs/*`.

refactor(component): Re-enable `Tooltip` trigger either `click` or `hover`

refactor(component): Re-enable `Tooltip` arrow

refactor(component): Re-enable Tooltip auto placement

refactor(component): Re-enable `Tooltip` transitions

fix(component): Undo effect requiring arrow in `Tooltip`

`Tooltip`s should be able to be created without
using the arrow.

fix(component): Overlap `Tooltip` arrow with text via `z-index`

Prevents the text inside a `Tooltip` from being
obscured by the `Tooltip` arrow.

fix(component): Resolve `Tooltip` arrow placement when `placement` = `auto`

Retrieves the actual placement calculated by
`floating-ui` from the hook. This is crucial when
the component is created with `auto` placement.
Inside `storybook`, `Tooltip`s were misaligned from
the parent element on `Chromium` browsers.
`storybook` provides a `decorators` object to let
us wrap components rendered in `storybook` with
some HTML.

This change wraps every component in a centered
flex box which is a nicer UX than the top left.

It also adds `MemoryRouter` which is required to
merge themesberg#69.
@tulup-conner tulup-conner force-pushed the feature/upgrade-popper-to-floating-ui branch from 86836a9 to f7015e3 Compare May 1, 2022 00:21
@tulup-conner
Copy link
Collaborator Author

@rluders Done - I also added <MemoryRouter> as a wrapper because it is required for components that use react-router-dom Link. (Will be necessary to merge Sidebar stories in #69)

@rluders rluders merged commit 9ba11ef into themesberg:main May 2, 2022
@tulup-conner tulup-conner deleted the feature/upgrade-popper-to-floating-ui branch May 2, 2022 07:52
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.

Tooltip Component
2 participants