FEPLT-2129: Replace react-popper by floating-ui #4494
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Storybook staging is available at https://kiwicom-orbit-dsil-replace-popper-by-floating.surge.sh |
|
Size Change: +194 B (+0.04%) Total Size: 460 kB
ℹ️ View Unchanged
|
Deploying orbit with
|
| Latest commit: |
33aa3ad
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://24f2cce1.orbit.pages.dev |
| Branch Preview URL: | https://dsil-replace-popper-by-float.orbit.pages.dev |
DSil
force-pushed
the
dsil/replace-popper-by-floating
branch
2 times, most recently
from
cd22f0d to
441e005
Compare
| LEFT_START = "left-start", | ||
| LEFT_END = "left-end", | ||
| } | ||
| export const isFixedPlacement = (placement: string): placement is Placement => |
There was a problem hiding this comment.
Why didn't you type placement as Placement | AutoPlacement but only as string?
There was a problem hiding this comment.
Because I want to accept any string and then make sure it is a Placement
There was a problem hiding this comment.
I'm still slightly confused, why string and not Placement | AutoPlacement? this function is used in TooltipContent and ContentWrapper, where the placement prop is Placement | AutoPlacement anyway. Is it for possible future purposes?
There was a problem hiding this comment.
By having this on a common place, I don't expect the function to be aware of where it is called, that is why I prefer to receive a more generic type as arg
packages/orbit-components/src/Popover/components/ContentWrapper.tsx
Outdated
Show resolved
Hide resolved
packages/orbit-components/src/Popover/components/ContentWrapper.tsx
Outdated
Show resolved
Hide resolved
packages/orbit-components/src/primitives/TooltipPrimitive/components/TooltipContent.tsx
Outdated
Show resolved
Hide resolved
packages/orbit-components/src/ErrorFormTooltip/Tooltip/index.tsx
Outdated
Show resolved
Hide resolved
|
The Tooltip vs TooltipPrimitive difference was quite tricky to spot but I have found out that it was actually the story that misled me, by having With that, I noticed that I was compensating the arrow size for the main axis, but not the cross axis, that seems to be needed as well. I added this and stories all look good now. But we should have visual tests… I will create a task for it. |
DSil
force-pushed
the
dsil/replace-popper-by-floating
branch
3 times, most recently
from
d385012 to
2dbe2f8
Compare
RobinCsl
force-pushed
the
dsil/replace-popper-by-floating
branch
from
2dbe2f8 to
0151151
Compare
RobinCsl
reviewed
Oct 17, 2024
DSil
force-pushed
the
dsil/replace-popper-by-floating
branch
from
0151151 to
72bb32b
Compare
RobinCsl
reviewed
Oct 17, 2024
packages/orbit-components/src/primitives/TooltipPrimitive/TooltipPrimitive.stories.tsx
Show resolved
Hide resolved
This allows for better usage of the types and enums across all components that need it
With the migration from react-popper to floating-ui, some snapshots needed a small adjustment
DSil
force-pushed
the
dsil/replace-popper-by-floating
branch
from
72bb32b to
33aa3ad
Compare
domihustinova
approved these changes
Oct 17, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Dependency from react-popper removed and replaced by floating-ui