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

Greatly improve the tooltip component #3987

Merged
merged 2 commits into from Jun 11, 2023

Conversation

ivarnakken
Copy link
Member

Description

It is now no longer super buggy!

Firstly it will not overflow out of view when there is no space.
Through the popper library, it will find the most suitable postion.
No more need for the renderDirection prop and hacky calculations.

Another nice feature is that it will point to the right direction even
when the trigger component is flexed, which it often is. This is most
noticeable on tables.

Some minor styling changes as well, but that's not really the important
part of this change. Worth noting that there is no longer an annoying
buggy line between the arrow and the tooltip whenever there's a bad
render.

Result

Screen.Recording.2023-06-09.at.18.33.37.mov

Testing

  • I have thoroughly tested my changes.

Went through all uses of tooltips. Tried all viewports, hard refreshes and other "stress testing".


Resolves webkom/lego#2849

@ivarnakken ivarnakken added enhancement Pull requests that make enhancements, instead of just purely new features future-was-2-years-ago Pull requests that introduce technologies which should have been "long" ago review-needed Pull requests that need review bug-fix Pull requests that fix a bug labels Jun 9, 2023
@ivarnakken ivarnakken requested a review from a team June 9, 2023 16:38
@ivarnakken ivarnakken self-assigned this Jun 9, 2023
@linear
Copy link

linear bot commented Jun 9, 2023

ABA-226 Fix Tooltip

There are multiple issues with it:

  • overflow issues
  • alignment (especially if the trigger component is flexed)
  • an annoying line between the carot and content
  • it needs some box-shadow

@ivarnakken ivarnakken changed the title Rewrite Tooltip to a functional component Greatly improve the tooltip component Jun 9, 2023
@ivarnakken ivarnakken force-pushed the ivarnakken/aba-226-fix-tooltip branch 3 times, most recently from 097305e to 23d1220 Compare June 9, 2023 19:04
Copy link
Member

@eikhr eikhr left a comment

Choose a reason for hiding this comment

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

I don't love adding more dependencies, but it might be worth it in this case. It works significantly better than before, and the code is way simpler. Looks great!

I think it might be worth keeping the possibility to override the popover position though, f.ex. on the event attendees I think it looks better with the popover over the profile picture.

app/components/Tooltip/index.tsx Outdated Show resolved Hide resolved
@ivarnakken ivarnakken added technical-debt Pull requests that reduces technical debt dependencies Pull requests that update a dependency file labels Jun 10, 2023
@ivarnakken ivarnakken force-pushed the ivarnakken/aba-226-fix-tooltip branch from 23d1220 to 8ff56e3 Compare June 10, 2023 21:37
It is now no longer super buggy!

Firstly it will not overflow out of view when there is no space.
Through the `popper` library, it will find the most suitable postion.
No more need for the `renderDirection` prop and hacky calculations.

Another nice feature is that it will point to the right direction even
when the trigger component is flexed, which it often is. This is most
noticeable on tables.

Some minor styling changes as well, but that's not really the important
part of this change. Worth noting that there is no longer an annoying
buggy line between the arrow and the tooltip whenever there's a bad
render.
@ivarnakken ivarnakken force-pushed the ivarnakken/aba-226-fix-tooltip branch from 8ff56e3 to 0bccb78 Compare June 10, 2023 21:38
@ivarnakken ivarnakken added the approved Pull requests that have been approved label Jun 10, 2023
Copy link
Contributor

@ollfkaih ollfkaih left a comment

Choose a reason for hiding this comment

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

Looks good!

@ivarnakken ivarnakken added ready-to-merge Pull requests that have been approved and are ready to be merged and removed review-needed Pull requests that need review labels Jun 11, 2023
@ivarnakken ivarnakken merged commit 939fa55 into master Jun 11, 2023
4 checks passed
@ivarnakken ivarnakken deleted the ivarnakken/aba-226-fix-tooltip branch June 11, 2023 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Pull requests that have been approved bug-fix Pull requests that fix a bug dependencies Pull requests that update a dependency file enhancement Pull requests that make enhancements, instead of just purely new features future-was-2-years-ago Pull requests that introduce technologies which should have been "long" ago ready-to-merge Pull requests that have been approved and are ready to be merged technical-debt Pull requests that reduces technical debt
Projects
None yet
3 participants