fix: stats layout and add tooltips #101
Conversation
✅ Deploy Preview for public-github-stats ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
theflucs
changed the title
| ( | ||
| { pullRequest: { state, title, id, url } }: any, | ||
| index: number | ||
| ) => ( | ||
| <div | ||
| key={id} | ||
| className={`flex items-center justify-between gap-2 tooltip text-left ${ | ||
| index === 0 ? "tooltip-bottom" : "tooltip-top" | ||
| }`} | ||
| data-tip={title} |
There was a problem hiding this comment.
This is a workaround for correctly displaying the tooltips.
I noticed that if it is set to tooltip-top the first item's tooltip doesn't show, as well as if it is set to tooltip-bottom the last one doesn't appear either. Feel free to discard these lines if you know a cleaner way to achieve the correct displaying of the tooltips.
There was a problem hiding this comment.
The workaround is fine if you have 2+ PRs, but in case you only have one it won't appear. We need to find a different solution
There was a problem hiding this comment.
Ouch, I didn't notice it. And it makes sense it doesn't work. We need to find another way 🤔
@CBID2 you can take over this if you like
| <div className="max-h-[22rem] hide-scrollbar overflow-auto flex flex-col gap-1"> | ||
| {nodes?.map( | ||
| ( | ||
| { pullRequest: { state, title, id, url } }: any, |
There was a problem hiding this comment.
We should probably add a better type here than any, I think there's already a type in types/github.ts for that
There was a problem hiding this comment.
For the most part, it looks good, but I do see some of the long titles not be in-line (see the card that says "Accessible For All" for reference in the screenshot below)
.
Is there a way you can make the tooltip work for all long titled-PRs @theflucs? :) What do you think @Balastrong?
|
@CBID2 are you sure your code is aligned with this branch? The PR title shows indeed the 3 dots, if it is too long, and also the contribution icons on the right should be aligned with the PR title pill, they don't look like. The tooltip part I think is fixed for the PR title but not for the contribution title, as mentioned above by @Balastrong. You can have a look at it and see if you come up with a solution. Thanks! |
@Balastrong this deployment needs approval again. |
|
@CBID2 the deploy is not working at the moment, it redirects to an old version. In order to test this branch you should checkout it locally and run the app there :) |
There was a problem hiding this comment.
Overall it looks good to me as the scope of the PR was only on PR titles actually, I'd go with merging it so we keep small changes.
@CBID2 thanks for the review! If you find something else that needs to be tweaked feel free to create new issues so we can start from there!
|
@all-contributors please add @CBID2 for review |
|
I've put up a pull request to add @CBID2! 🎉 |
@Balastrong, you used my name instead of @theflucs. Did you mean to do that? |
|
Yes, I added you for the review :D |
Fix responsive card layout
Add tooltips to pr and contribution title (removed title tag), so fixing the issue #95