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

Navbar Tooltip #200

Merged
merged 10 commits into from
Aug 11, 2024
Merged

Conversation

Tizzz-555
Copy link
Contributor

Added tooltip component

hburn7
hburn7 previously approved these changes Jul 18, 2024
@hburn7 hburn7 requested a review from AkinariHex July 18, 2024 16:08
@hburn7 hburn7 mentioned this pull request Jul 18, 2024
2 tasks
Copy link
Collaborator

@AkinariHex AkinariHex left a comment

Choose a reason for hiding this comment

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

Overall it's all okay and you did a good work on implementing this with framer motion too.

There is a little strange movement when you remove slowly the hover from the image going between the image and the tooltip (see video below), would like to have this fixed.

  • 1. You could try to increment the timeout timer a little bit or for me it's better to add some margin on top of the tooltip (if I am not mistaken it should catch the hover on the blank space of the margin).
Recording.2024-07-18.185239.mp4
  • 2. Visually I think that can be better to lower a little bit the text size (use a value with rem, it will resize correctly with all the window)
  • 3. May try to add flex to parent of the clickable text and add gap to remove padding from the children.
  • 4. Visually the figma design have a little bit more spacing on left/right of the tooltip
  • 5. The sun/moon icon on the design seems to have more space than the text, maybe increase top space
  • 6. I don't see the shadow on the right of the tooltip like the one on figma, also have you tried how it looks to have the tooltip full white/same background-color that have on design?
  • 7. Use .tsx as extension
  • 8. My bad for not having a global prettier file, I sent you the prettier file in DM, could you please add it on this branch and format the file you changed? This will revert the changes that he found about single quotes changed to double.

Tip for the future: it's better to use <Link>Any name</Link> when there will be a clickable div that will move to another route.
In this case Sign out will not move us to another route so it's fine, but we don't support Friends for now so you can comment that part or remove it.

@Tizzz-555

@AkinariHex AkinariHex dismissed hburn7’s stale review July 18, 2024 17:02

Overall it works as expected but will be great with more adjustments

@AkinariHex AkinariHex changed the title feature/Navbar-tooltip Navbar Tooltip Jul 21, 2024
@AkinariHex AkinariHex self-requested a review July 21, 2024 20:22
@AkinariHex AkinariHex requested a review from hburn7 August 11, 2024 17:56
@hburn7 hburn7 merged commit 6426d2e into osu-tournament-rating:master Aug 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants