Skip to content
This repository has been archived by the owner on Oct 23, 2024. It is now read-only.

Refactoring TourPoint to use react-tiny-popover #232

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

Boughtmanatee5
Copy link
Contributor

@Boughtmanatee5 Boughtmanatee5 commented Feb 24, 2023

What this PR does

Replaces iris useAnchor with react-tiny-popover. This will allow us to fix issues like https://github.com/vimeo/iris/issues/54 without having to refactor useAnchor. Right now this PR adds no new features to TourPoint it should behave the same as it does currently. Once this is tested we can follow up by adding support for the features react-tiny-popover has like passing an array of positions in priority of how we want the be positioned.

How it does that

  1. Replace useAnchor with the Popover component from react-tiny popover.
  2. Convert our attach prop to position and align for react-tiny-popover with convertAttachToPositionAlign
  3. Update Prop types.

Testing

We should probably have this QA'd pretty throughly since this is an important component.

@Boughtmanatee5 Boughtmanatee5 force-pushed the 2023-02-24-tourpoint-react-tiny-popover-poc branch from caaa921 to edd2cb5 Compare February 24, 2023 22:36

type Props = any;

new Image().src = 'http://placekitten.com/320/213';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥 preloading the image into the browser cache 🔥 I like thattttttt 😮‍💨

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copied it from the story Sean wrote!

@juliewongbandue
Copy link
Collaborator

Ohh exciting! thank you for taking this on!

@Boughtmanatee5 Boughtmanatee5 force-pushed the 2023-02-24-tourpoint-react-tiny-popover-poc branch from edd2cb5 to 13b228a Compare March 3, 2023 17:23
@Boughtmanatee5
Copy link
Contributor Author

@juliewongbandue Almost have feature parity with the current Tourpoint there's just an alignment bug when the Tourpoint is within a popover!

@Boughtmanatee5 Boughtmanatee5 force-pushed the 2023-02-24-tourpoint-react-tiny-popover-poc branch 3 times, most recently from b607541 to 601c5ee Compare March 10, 2023 19:55
@Boughtmanatee5 Boughtmanatee5 marked this pull request as ready for review March 10, 2023 20:05
@Boughtmanatee5 Boughtmanatee5 requested review from a team as code owners March 10, 2023 20:05
@Boughtmanatee5 Boughtmanatee5 force-pushed the 2023-02-24-tourpoint-react-tiny-popover-poc branch from 601c5ee to d469e73 Compare March 13, 2023 21:00
@juliewongbandue
Copy link
Collaborator

hey @Boughtmanatee5 ! I just merged this PR that migrates to pnpm: #257, feel free to rebase!

@juliewongbandue juliewongbandue mentioned this pull request May 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants