-
-
Notifications
You must be signed in to change notification settings - Fork 288
feat: rename RepositoriesCard to RepositoryCard and add cursor pointe… #2490
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: rename RepositoriesCard to RepositoryCard and add cursor pointe… #2490
Conversation
Summary by CodeRabbit
WalkthroughRenamed component and types: Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
arkid15r
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please resolve the conflicts first.
44b3f29 to
250907c
Compare
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/__tests__/unit/components/RepositoryCard.test.tsx (1)
180-198: Consider combining the two cursor-pointer tests for better specificity.The tests verify cursor-pointer behavior, but they could be more robust by checking that the repository title button specifically has the cursor-pointer class.
Consider merging these into a single, more specific test:
- it('repository title button has cursor-pointer class', () => { + it('repository title button has cursor-pointer class and is clickable', () => { const repositories = [createMockRepository(0)] - - const { container } = render(<RepositoryCard repositories={repositories} />) - - const button = container.querySelector('button.cursor-pointer') - expect(button).toBeInTheDocument() - expect(button).toHaveClass('cursor-pointer') - }) - - it('repository title is rendered as a button element', () => { - const repositories = [createMockRepository(0)] - render(<RepositoryCard repositories={repositories} />) const repositoryButton = screen.getByText('Repository 0').closest('button') expect(repositoryButton).toBeInTheDocument() expect(repositoryButton?.tagName).toBe('BUTTON') + expect(repositoryButton).toHaveClass('cursor-pointer') })
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
frontend/__tests__/unit/components/CardDetailsPage.test.tsx(1 hunks)frontend/__tests__/unit/components/RepositoryCard.test.tsx(19 hunks)frontend/src/components/CardDetailsPage.tsx(2 hunks)frontend/src/components/RepositoryCard.tsx(3 hunks)frontend/src/types/project.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/tests/unit/components/CardDetailsPage.test.tsx
- frontend/src/types/project.ts
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/components/RepositoryCard.tsx (1)
frontend/src/types/project.ts (1)
RepositoryCardListProps(49-52)
🔇 Additional comments (4)
frontend/src/components/RepositoryCard.tsx (2)
5-5: LGTM! Component rename executed correctly.The import, type annotations, and export have been updated consistently to reflect the rename from
RepositoriesCardtoRepositoryCard, and the props type correctly usesRepositoryCardListProps.Also applies to: 11-11, 76-76
45-45: LGTM! Cursor pointer enhancement improves UX.The
cursor-pointerclass correctly indicates that the repository title is clickable, meeting the PR requirement.frontend/src/components/CardDetailsPage.tsx (1)
32-32: LGTM! Import and usage updated correctly.The component import and usage have been properly updated to reflect the
RepositoryCardrename, with props remaining unchanged.Also applies to: 282-282
frontend/__tests__/unit/components/RepositoryCard.test.tsx (1)
7-7: LGTM! All test references updated consistently.The import, describe block, and all render calls have been correctly updated to reference
RepositoryCardinstead ofRepositoriesCard.Also applies to: 43-43, 77-77, 84-84, 95-95, 105-105, 113-113, 121-121, 145-145, 157-157, 177-178, 183-183, 193-193, 207-207, 218-218, 228-228, 241-241, 253-253, 265-265, 278-278, 292-292, 304-304, 315-315, 334-334
kasya
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good! Thank you @SiddharthJiyani 👍🏼



Proposed change
RepositoriesCardcomponent toRepositoryCardand add cursor pointer #2447Renamed the
RepositoriesCardcomponent toRepositoryCard(singular) and addedcursor-pointerclass to repository title buttons as requested in the issue.Changes:
RepositoriesCard.tsx→RepositoryCard.tsxRepositoriesCard→RepositoryCardcursor-pointerTailwind class to repository title button elementRepositoryItemProps→RepositoryCardPropsfor consistency with component namingCardDetailsPage.tsxandpage.tsxRepositoriesCard.test.tsx→RepositoryCard.test.tsxFiles modified: 9 files (5 modified, 2 renamed, 2 created as part of rename)
The cursor now changes to a pointer when hovering over repository titles, improving UX as requested.
Checklist
make check-testlocally; all checks and tests passed.nest_issue.mp4