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

feat: add custom tooltip component to contributors bar chart #501

Merged
merged 10 commits into from
Oct 10, 2022

Conversation

OgDev-01
Copy link
Contributor

@OgDev-01 OgDev-01 commented Oct 6, 2022

What type of PR is this? (check all applicable)

  • πŸ• Feature
  • πŸ› Bug Fix
  • πŸ“ Documentation Update
  • 🎨 Style
  • πŸ§‘β€πŸ’» Code Refactor
  • πŸ”₯ Performance Improvements
  • βœ… Test
  • πŸ€– Build
  • πŸ” CI
  • πŸ“¦ Chore (Release)
  • ⏩ Revert

Description

This PR creates a custom Tooltip component in the atoms directory... 80% completed but opening this PR to seek help with some CSS troubles πŸ˜‚

Details:
The idea was to create a dynamic tooltip component that can be used across all components...so radix was leveraged to create the tooltip component

had to set some props to make it completely dynamic:

  • content: the tooltip message
  • direction: the direction to display e.g: left, right, top, bottom...

Related Tickets & Documents

Fixes #453

Mobile & Desktop Screenshots/Recordings

image

Added tests?

  • πŸ‘ yes
  • πŸ™… no, because they aren't needed
  • πŸ™‹ no, because I need help

Added to documentation?

  • πŸ“œ README.md
  • πŸ““ docs.opensauced.pizza
  • πŸ• dev.to/opensauced
  • πŸ“• storybook
  • πŸ™… no documentation needed

[optional] Are there any post-deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

@netlify
Copy link

netlify bot commented Oct 6, 2022

βœ… Deploy Preview for design-insights ready!

Name Link
πŸ”¨ Latest commit 432c017
πŸ” Latest deploy log https://app.netlify.com/sites/design-insights/deploys/63444fafa430c90008486d07
😎 Deploy Preview https://deploy-preview-501--design-insights.netlify.app
πŸ“± Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Oct 6, 2022

βœ… Deploy Preview for oss-insights ready!

Name Link
πŸ”¨ Latest commit 432c017
πŸ” Latest deploy log https://app.netlify.com/sites/oss-insights/deploys/63444faf620e9a00087cfe06
😎 Deploy Preview https://deploy-preview-501--oss-insights.netlify.app
πŸ“± Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@OgDev-01
Copy link
Contributor Author

OgDev-01 commented Oct 6, 2022

Keeping this as a draft till the z-index drama is solved...

Copy link
Contributor

@eryc-cc eryc-cc left a comment

Choose a reason for hiding this comment

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

Keeping this as a draft till the z-index drama is solved...

That’s happening because the second card is β€œabove” the first card. So anything inside the first card will be behind the second card.

What you can do is:

  1. Add a base z-index and a position relative to all contributor cards
  2. Then add a higher z-index when the contributor card is hovered. That will make sure the hovered card stays β€œabove” all other cards.
  3. Finally, add a z-index to the tooltip, to make sure it is above all elements inside the card itself.

Let me know if it helps.

@eryc-cc
Copy link
Contributor

eryc-cc commented Oct 6, 2022

@sungoldtech

Actually, as @0-vortex mentioned on Slack, the fix I just told you is a "hack". It works, but it's not ideal.

It's best to use a Tooltip library and style it as needed.

Here's one of the best:

Other tooltips:

@OgDev-01
Copy link
Contributor Author

OgDev-01 commented Oct 7, 2022

@sungoldtech

Actually, as @0-vortex mentioned on Slack, the fix I just told you is a "hack". It works, but it's not ideal.

It's best to use a Tooltip library and style it as needed.

Here's one of the best:

Other tooltips:

Radix Ui now leveraged for the tooltip πŸ•πŸ•

@OgDev-01 OgDev-01 marked this pull request as ready for review October 7, 2022 06:11
Copy link
Contributor

@0-vortex 0-vortex left a comment

Choose a reason for hiding this comment

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

LGTM! πŸ‘

Great job implementing this library in such a short time frame, there are some minor tweaks we can do like delay duration or giving a different position based on element index (left->right, center->center, right->left for example) etc, however that will come with time and us actually having variable tooltips.

For now I think this is ready to go on beta! πŸ• πŸ• πŸ•

Copy link
Contributor

@eryc-cc eryc-cc left a comment

Choose a reason for hiding this comment

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

Great work Sunday! It's a nice implementation.

I added some small adjustments to the UI, so it's more readable and balanced. πŸ™

components/atoms/Tooltip/tooltip.tsx Outdated Show resolved Hide resolved
components/atoms/Tooltip/tooltip.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

PR Compliance Checks

Thank you for your Pull Request! We have run several checks on this pull request in order to make sure it's suitable for merging into this project. The results are listed in the following section.

Watched Files

This pull request modifies specific files that require careful review by the maintainers.

Files Matched

  • npm-shrinkwrap.json
  • package.json

@OgDev-01 OgDev-01 requested a review from eryc-cc October 8, 2022 22:09
Copy link
Contributor

@eryc-cc eryc-cc left a comment

Choose a reason for hiding this comment

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

Aside from that, LGTM πŸ•

…r-chart.tsx

Co-authored-by: Eryc <101464991+pixelsbyeryc@users.noreply.github.com>
Copy link
Contributor

@brandonroberts brandonroberts left a comment

Choose a reason for hiding this comment

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

LGTM

@OgDev-01 OgDev-01 merged commit 855d66b into beta Oct 10, 2022
@OgDev-01 OgDev-01 deleted the 453-languages-in-contributors-expands branch October 10, 2022 17:07
github-actions bot pushed a commit that referenced this pull request Oct 10, 2022
## [1.12.0-beta.2](v1.12.0-beta.1...v1.12.0-beta.2) (2022-10-10)

### πŸ• Features

* add custom tooltip component to contributors bar chart ([#501](#501)) ([855d66b](855d66b))
@github-actions
Copy link
Contributor

πŸŽ‰ This PR is included in version 1.12.0-beta.2 πŸŽ‰

The release is available on GitHub release

Your semantic-release bot πŸ“¦πŸš€

@eryc-cc
Copy link
Contributor

eryc-cc commented Oct 10, 2022

Way to go, Sunday! πŸš€

btw, I love this "co-author" feature πŸ˜›

github-actions bot pushed a commit that referenced this pull request Oct 11, 2022
## [1.12.0](v1.11.0...v1.12.0) (2022-10-11)

### πŸ› Bug Fixes

* update pagination counts for repositories and contributors ([#503](#503)) ([1332fab](1332fab))

### πŸ• Features

* add `PullRequestPieChart` component ([#514](#514)) ([9132a93](9132a93))
* add custom tooltip component to contributors bar chart ([#501](#501)) ([855d66b](855d66b))
@github-actions
Copy link
Contributor

πŸŽ‰ This PR is included in version 1.12.0 πŸŽ‰

The release is available on GitHub release

Your semantic-release bot πŸ“¦πŸš€

0-vortex added a commit that referenced this pull request Oct 17, 2022
* origin/main: (114 commits)
  chore(minor): release 1.14.0 [skip ci]
  chore(patch): release 1.14.0-beta.3 on beta channel [skip ci]
  fix: add root href to header logo component image (#537)
  fix: correct card horizontal tooltip error on storybook (#536)
  chore(patch): release 1.14.0-beta.2 on beta channel [skip ci]
  fix: humanize numbers and use absolute values in highlight cards (#535)
  chore(minor): release 1.14.0-beta.1 on beta channel [skip ci]
  feat: allow for displaying insights based on custom topics (#528)
  fix: show pr velocity in relative days (#534)
  chore(minor): release 1.13.0 [skip ci]
  chore(minor): release 1.13.0-beta.1 on beta channel [skip ci]
  feat: re-enable toggling of bots on scatterchart (#530)
  chore(patch): release 1.12.0-beta.4 on beta channel [skip ci]
  fix: remove duplicate contributors from the scatter chart (#529)
  chore(minor): release 1.12.0-beta.3 on beta channel [skip ci]
  feat: implement InsightPageCard component (#519)
  chore(minor): release 1.12.0 [skip ci]
  chore(minor): release 1.12.0-beta.2 on beta channel [skip ci]
  feat: add custom tooltip component to contributors bar chart (#501)
  chore(minor): release 1.12.0-beta.1 on beta channel [skip ci]
  ...
ElpisHelle added a commit to ElpisHelle/next.js-tailwindcss that referenced this pull request Aug 17, 2023
## [1.12.0-beta.2](open-sauced/app@v1.12.0-beta.1...v1.12.0-beta.2) (2022-10-10)

### πŸ• Features

* add custom tooltip component to contributors bar chart ([#501](open-sauced/app#501)) ([855d66b](open-sauced/app@855d66b))
ElpisHelle added a commit to ElpisHelle/next.js-tailwindcss that referenced this pull request Aug 17, 2023
## [1.12.0](open-sauced/app@v1.11.0...v1.12.0) (2022-10-11)

### πŸ› Bug Fixes

* update pagination counts for repositories and contributors ([#503](open-sauced/app#503)) ([1332fab](open-sauced/app@1332fab))

### πŸ• Features

* add `PullRequestPieChart` component ([#514](open-sauced/app#514)) ([9132a93](open-sauced/app@9132a93))
* add custom tooltip component to contributors bar chart ([#501](open-sauced/app#501)) ([855d66b](open-sauced/app@855d66b))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: Languages in ContributorCard expand when the name is too long
4 participants