Fix tooltip conflicts for duplicate contributors#1059
Conversation
Summary by CodeRabbit
WalkthroughThis change introduces a new Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (5)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/components/Card.tsx(1 hunks)frontend/src/components/ContributorAvatar.tsx(3 hunks)
🧰 Additional context used
🪛 GitHub Actions: Run CI/CD
frontend/src/components/ContributorAvatar.tsx
[error] 5-5: Trailing whitespace found. Pre-commit hook 'trailing-whitespace' failed.
🔇 Additional comments (3)
frontend/src/components/Card.tsx (1)
118-118: AddinguniqueKeyprop to ensure unique tooltip IDs: LGTM!The addition of the
uniqueKeyprop using the index as a unique identifier resolves the tooltip conflict issue when the same contributor appears in multiple cards. This implementation ensures each tooltip has a unique ID based on both the contributor and its position in the array.frontend/src/components/ContributorAvatar.tsx (2)
22-22: LGTM: Props destructuring updated correctly.The component now correctly destructures both the
contributoranduniqueKeyprops.
43-43: LGTM: Tooltip ID now includes the uniqueKey prop.The tooltip ID now includes the
uniqueKeyprop, which ensures unique IDs even when the same contributor appears in multiple places. This fixes the tooltip conflict issue described in PR #1058.
|
|
||
| type ContributorProps = { | ||
| contributor: TopContributorsTypeAlgolia | TopContributorsTypeGraphql | ||
| uniqueKey: string |
There was a problem hiding this comment.
Fix the trailing whitespace in prop definition.
There's trailing whitespace after the uniqueKey: string type definition which is causing a pipeline failure.
- uniqueKey: string
+ uniqueKey: string📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| uniqueKey: string | |
| uniqueKey: string |
|
| return ( | ||
| <Tooltip | ||
| id={`avatar-tooltip-${login}`} | ||
| id={`avatar-tooltip-${login}-${uniqueKey}`} |
There was a problem hiding this comment.
It seems this composite key should include the project part too. This way we avoid the same bug when same contributor is on the same position within multiple projects on the same page. This case is very low probability but still can happen. You can create a separate issue for that as it requires more effort (index update may be needed)/
There was a problem hiding this comment.
Yes I guess the probability is very less but still it can happen again. Though this implementation will work but as you we need update the indexes and create a more robust unique key. I was thinking of using Math.random() but again ig its not a good practise!!
* Fix: Ensure unique tooltip IDs to prevent conflicts * Whitespace
3 participants
Resolves #1058
This PR resolves an issue where tooltips would behave incorrectly if the same contributor appeared in multiple cards. The fix appends a unique identifier to the tooltip id, ensuring each tooltip instance is distinct.
Recording.2025-03-08.175629.mp4