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

[IOPID-2426] Tooltip component #351

Merged
merged 19 commits into from
Nov 14, 2024
Merged

[IOPID-2426] Tooltip component #351

merged 19 commits into from
Nov 14, 2024

Conversation

ChrisMattew
Copy link
Collaborator

@ChrisMattew ChrisMattew commented Nov 8, 2024

Short description

This PR adds the Tooltip component

List of changes proposed in this pull request

  • Created Tooltip component
  • Added Tooltips screen in the Example app to show the Tooltip behaviors

Demo

iOS Android
tooltips-demo.mp4
android-tooltip.mp4
A11Y iOS A11Y Android
a11y-iOS-tooltip.MP4
a11y-android-tooltip.mp4

Important

The left and right positions are calculated after the component is rendered. This implementation causes a minor glitch where the Tooltip repositions itself to achieve a centered alignment relative to the enclosing element.
To avoid this behavior, the opacity value of the component depends on the parameters returned by the onLayout prop. Once the tooltipLayout is defined, the opacity is set to a value of 1.

How to test

Run the example app and test the Tooltip different implementations

@ChrisMattew ChrisMattew self-assigned this Nov 8, 2024
@ChrisMattew ChrisMattew marked this pull request as ready for review November 8, 2024 16:56
@ChrisMattew ChrisMattew requested review from dmnplb and a team as code owners November 8, 2024 16:56
@thisisjp
Copy link

thisisjp commented Nov 8, 2024

Awesome! Why don't we add a very subtle fade in \ fade out transition to gently darken the background? @dmnplb

Copy link
Collaborator

@dmnplb dmnplb left a comment

Choose a reason for hiding this comment

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

As @thisisjp rightly points out, the component feels a bit static in terms of interaction: we could improve both the background and the entry/exit transitions. But I also want to be pragmatic: I think that the first iteration is good enough to release.

Let's not forget that this component is special because it needs to handle different placements and device configurations, so we should integrate it into the main app as soon as possible to do the appropriate viability tests before making any other improvements.

So, in short: LGTM, but let's plan the following refinements:

  • Background and entry/exit smooth transitions, ideally made with reanimated
  • Dark mode support by using the appropriate theme keys

src/components/tooltip/Tooltip.tsx Outdated Show resolved Hide resolved
src/components/tooltip/Tooltip.tsx Outdated Show resolved Hide resolved
src/components/tooltip/Tooltip.tsx Outdated Show resolved Hide resolved
@ChrisMattew ChrisMattew requested a review from Ladirico November 14, 2024 09:45
Copy link
Contributor

@Ladirico Ladirico left a comment

Choose a reason for hiding this comment

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

LGTM!

@Ladirico Ladirico merged commit acab735 into main Nov 14, 2024
6 checks passed
@Ladirico Ladirico deleted the feature/tooltip branch November 14, 2024 16:49
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.

4 participants