-
-
Notifications
You must be signed in to change notification settings - Fork 287
Adding cursor #2696
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
base: main
Are you sure you want to change the base?
Adding cursor #2696
Conversation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughTwo independent modifications: a Node.js heap memory configuration added to the local Docker compose file for the frontend service, and a cursor-pointer CSS utility class added to ToggleableList item buttons for improved UX affordance on interactive elements. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
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 |
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: 1
🧹 Nitpick comments (1)
frontend/src/components/ToggleableList.tsx (1)
42-42: Good addition for UX affordance, but consider disabled state.The
cursor-pointerclass appropriately signals that the items are interactive, directly addressing issue #2672. However, whenisDisabledis true, the pointer cursor still appears even though clicking has no effect (line 43 conditionally prevents the action). This could mislead users.Consider conditionally applying the cursor based on the disabled state:
- className="cursor-pointer rounded-lg border border-gray-400 px-3 py-1 text-sm hover:bg-gray-200 dark:border-gray-300 dark:hover:bg-gray-700" + className={`rounded-lg border border-gray-400 px-3 py-1 text-sm hover:bg-gray-200 dark:border-gray-300 dark:hover:bg-gray-700 ${!isDisabled ? 'cursor-pointer' : 'cursor-default opacity-50'}`}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docker-compose/local.yaml(1 hunks)frontend/src/components/ToggleableList.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-17T02:42:41.928Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 2288
File: frontend/src/components/ActionButton.tsx:0-0
Timestamp: 2025-09-17T02:42:41.928Z
Learning: In frontend/src/components/ActionButton.tsx, the user Rajgupta36 intentionally changed text-blue-600 to text-[#1D7BD7] to align the text color with the border color (#1D7BD7) for visual consistency, prioritizing design alignment over theme tokens.
Applied to files:
frontend/src/components/ToggleableList.tsx
🔇 Additional comments (1)
frontend/src/components/ToggleableList.tsx (1)
8-27: ToggleableList is confirmed as the component rendering Languages and Topics.The component is imported in
frontend/src/components/CardDetailsPage.tsx(line 36) and used at lines 193–197 for Languages and line 200 for Topics, each with appropriate icons and labels.
|
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.
@Munishgowda Thanks for working on this! Left s request ⬇️
| <button | ||
| key={item} | ||
| className="rounded-lg border border-gray-400 px-3 py-1 text-sm hover:bg-gray-200 dark:border-gray-300 dark:hover:bg-gray-700" | ||
| className="cursor-pointer rounded-lg border border-gray-400 px-3 py-1 text-sm hover:bg-gray-200 dark:border-gray-300 dark:hover:bg-gray-700" |
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.
@Munishgowda This component has a isDisabled prop.
I believe when the click is disabled we should not show the cursor. Can you update this to be a dynamic value?
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.
@Munishgowda please also update tests for this component.



Added Cursor to Languages
Proposed change
Resolves #2672
Add the PR description here.
Checklist
make check-testlocally; all checks and tests passed.