fix(#1003): add missing cursor: pointer to interactive ui elements#1059
Open
aniruddhaadak80 wants to merge 1 commit intof:mainfrom
Open
fix(#1003): add missing cursor: pointer to interactive ui elements#1059aniruddhaadak80 wants to merge 1 commit intof:mainfrom
aniruddhaadak80 wants to merge 1 commit intof:mainfrom
Conversation
📝 WalkthroughWalkthroughThis change adds Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to resolve issue #1003 by standardizing cursor: pointer styling across shared UI components so interactive elements are visually indicated as clickable throughout the app.
Changes:
- Adds
cursor-pointerto core interactive primitives (e.g.,Button,TabsTrigger, dropdown/select menu items). - Updates category list items to show a pointer cursor on hover.
- Updates
Badgebase styles to includecursor-pointer.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/ui/button.tsx | Adds cursor-pointer to the base button variant styles. |
| src/components/ui/badge.tsx | Adds cursor-pointer to the base badge variant styles. |
| src/components/ui/tabs.tsx | Adds cursor-pointer to TabsTrigger. |
| src/components/ui/dropdown-menu.tsx | Replaces cursor-default with cursor-pointer on dropdown menu items. |
| src/components/ui/select.tsx | Replaces cursor-default with cursor-pointer on select items and scroll buttons. |
| src/components/categories/category-item.tsx | Adds cursor-pointer to the category item container. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
7
to
9
| const badgeVariants = cva( | ||
| "inline-flex items-center justify-center rounded-[var(--radius)] border px-2 py-0.5 text-xs font-medium w-fit whitespace-nowrap shrink-0 [&>svg]:size-3 gap-1 [&>svg]:pointer-events-none transition-colors overflow-hidden", | ||
| "inline-flex items-center cursor-pointer justify-center rounded-[var(--radius)] border px-2 py-0.5 text-xs font-medium w-fit whitespace-nowrap shrink-0 [&>svg]:size-3 gap-1 [&>svg]:pointer-events-none transition-colors overflow-hidden", | ||
| { |
There was a problem hiding this comment.
Badge renders as a plain span by default, so making the base variant always cursor-pointer causes non-interactive badges (e.g. status/labels) throughout the app to look clickable. Consider removing cursor-pointer from the default badge styles and only applying it for interactive usages (e.g. via asChild + consumer className, or an explicit interactive/clickable variant).
Comment on lines
+20
to
23
| <div className="group flex cursor-pointer items-center justify-between gap-2 border rounded-[var(--radius)] px-3 py-2 bg-card hover:bg-accent/50 transition-colors"> | ||
| <Link | ||
| href={`/categories/${category.slug}`} | ||
| className="flex items-center gap-2 min-w-0 flex-1" |
There was a problem hiding this comment.
This container <div> doesn’t have an onClick and isn’t a link/button, so adding cursor-pointer makes the padded area look clickable even when it isn’t (only the inner <Link> is interactive). Prefer moving the pointer cursor to the <Link> itself, or restructure so the entire row is the link (e.g. wrap the container with Link/asChild) if you intend the whole item to be clickable.
Suggested change
| <div className="group flex cursor-pointer items-center justify-between gap-2 border rounded-[var(--radius)] px-3 py-2 bg-card hover:bg-accent/50 transition-colors"> | |
| <Link | |
| href={`/categories/${category.slug}`} | |
| className="flex items-center gap-2 min-w-0 flex-1" | |
| <div className="group flex items-center justify-between gap-2 border rounded-[var(--radius)] px-3 py-2 bg-card hover:bg-accent/50 transition-colors"> | |
| <Link | |
| href={`/categories/${category.slug}`} | |
| className="flex items-center gap-2 min-w-0 flex-1 cursor-pointer" |
Comment on lines
109
to
114
| <SelectPrimitive.Item | ||
| data-slot="select-item" | ||
| className={cn( | ||
| "focus:bg-accent focus:text-accent-foreground [&_svg:not([class*='text-'])]:text-muted-foreground relative flex w-full cursor-default items-center gap-2 rounded-sm py-1.5 pr-8 pl-2 text-sm outline-hidden select-none data-[disabled]:pointer-events-none data-[disabled]:opacity-50 [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4 *:[span]:last:flex *:[span]:last:items-center *:[span]:last:gap-2", | ||
| "focus:bg-accent focus:text-accent-foreground [&_svg:not([class*='text-'])]:text-muted-foreground relative flex w-full cursor-pointer items-center gap-2 rounded-sm py-1.5 pr-8 pl-2 text-sm outline-hidden select-none data-[disabled]:pointer-events-none data-[disabled]:opacity-50 [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4 *:[span]:last:flex *:[span]:last:items-center *:[span]:last:gap-2", | ||
| className | ||
| )} |
There was a problem hiding this comment.
The PR description says it fixes missing pointer cursors in the base <Select> component, but SelectTrigger still doesn’t set cursor-pointer (only items/scroll buttons were updated). If the reported issue is about the select control itself (e.g. language selector), consider adding the cursor style to SelectTrigger so the primary interactive element is covered.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/categories/category-item.tsx`:
- Line 20: The wrapper div currently includes "cursor-pointer" which makes
non-actionable areas appear clickable; remove "cursor-pointer" from the
wrapper's className in category-item.tsx and instead apply "cursor-pointer" only
to the actionable elements: the Link element that navigates to the category
(referenced as the Link node in this component) and the SubscribeButton (either
add a wrapper with "cursor-pointer" or pass a className/prop down to
SubscribeButton so it shows pointer only over the actual button). Ensure the
wrapper retains other classes like "group", "flex", "items-center", etc., but
not the cursor style.
In `@src/components/ui/badge.tsx`:
- Line 8: The base class string for the Badge component includes
"cursor-pointer" which makes every Badge appear interactive; remove
"cursor-pointer" from the default class list in the Badge component and instead
add an explicit interactive variant (e.g., a boolean prop named interactive on
the Badge component) that, when true, appends "cursor-pointer" to the className
(and optionally sets accessible attributes like role/tabIndex if used as a
control). Update the Badge component's props/type to accept the new interactive
prop and update any call sites that expect clickability to pass
interactive={true}.
In `@src/components/ui/select.tsx`:
- Line 112: The SelectTrigger element (and any other trigger-class definitions)
is missing the pointer cursor, so update the className strings used by the
SelectTrigger component(s) to include a non-disabled cursor class (e.g., add
"cursor-pointer" to the SelectTrigger class list) while preserving the disabled
rule (data-[disabled]:pointer-events-none and data-[disabled]:opacity-50) so
disabled triggers don’t show the pointer; apply the same change to the other
trigger definitions referenced in this file (the other class strings around the
same area noted in the review).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 92e9779c-3dc0-4c3a-aff9-6838ad1b6089
📒 Files selected for processing (6)
src/components/categories/category-item.tsxsrc/components/ui/badge.tsxsrc/components/ui/button.tsxsrc/components/ui/dropdown-menu.tsxsrc/components/ui/select.tsxsrc/components/ui/tabs.tsx
| export function CategoryItem({ category, isSubscribed, showSubscribe }: CategoryItemProps) { | ||
| return ( | ||
| <div className="group flex items-center justify-between gap-2 border rounded-[var(--radius)] px-3 py-2 bg-card hover:bg-accent/50 transition-colors"> | ||
| <div className="group flex cursor-pointer items-center justify-between gap-2 border rounded-[var(--radius)] px-3 py-2 bg-card hover:bg-accent/50 transition-colors"> |
There was a problem hiding this comment.
Don't put cursor-pointer on the non-clickable wrapper.
Only the Link and SubscribeButton are actionable here. With this change, the row shows a pointer over dead space and the count block, which misrepresents what users can click.
Suggested fix
- <div className="group flex cursor-pointer items-center justify-between gap-2 border rounded-[var(--radius)] px-3 py-2 bg-card hover:bg-accent/50 transition-colors">
+ <div className="group flex items-center justify-between gap-2 border rounded-[var(--radius)] px-3 py-2 bg-card hover:bg-accent/50 transition-colors">
<Link
href={`/categories/${category.slug}`}
- className="flex items-center gap-2 min-w-0 flex-1"
+ className="flex min-w-0 flex-1 cursor-pointer items-center gap-2"
>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/categories/category-item.tsx` at line 20, The wrapper div
currently includes "cursor-pointer" which makes non-actionable areas appear
clickable; remove "cursor-pointer" from the wrapper's className in
category-item.tsx and instead apply "cursor-pointer" only to the actionable
elements: the Link element that navigates to the category (referenced as the
Link node in this component) and the SubscribeButton (either add a wrapper with
"cursor-pointer" or pass a className/prop down to SubscribeButton so it shows
pointer only over the actual button). Ensure the wrapper retains other classes
like "group", "flex", "items-center", etc., but not the cursor style.
|
|
||
| const badgeVariants = cva( | ||
| "inline-flex items-center justify-center rounded-[var(--radius)] border px-2 py-0.5 text-xs font-medium w-fit whitespace-nowrap shrink-0 [&>svg]:size-3 gap-1 [&>svg]:pointer-events-none transition-colors overflow-hidden", | ||
| "inline-flex items-center cursor-pointer justify-center rounded-[var(--radius)] border px-2 py-0.5 text-xs font-medium w-fit whitespace-nowrap shrink-0 [&>svg]:size-3 gap-1 [&>svg]:pointer-events-none transition-colors overflow-hidden", |
There was a problem hiding this comment.
Keep the pointer cursor opt-in for Badge.
Badge renders a span by default, so putting cursor-pointer in the base style will make every static badge look interactive. This should stay at interactive call sites or behind a dedicated interactive variant instead.
Suggested fix
- "inline-flex items-center cursor-pointer justify-center rounded-[var(--radius)] border px-2 py-0.5 text-xs font-medium w-fit whitespace-nowrap shrink-0 [&>svg]:size-3 gap-1 [&>svg]:pointer-events-none transition-colors overflow-hidden",
+ "inline-flex items-center justify-center rounded-[var(--radius)] border px-2 py-0.5 text-xs font-medium w-fit whitespace-nowrap shrink-0 [&>svg]:size-3 gap-1 [&>svg]:pointer-events-none transition-colors overflow-hidden",📝 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.
Suggested change
| "inline-flex items-center cursor-pointer justify-center rounded-[var(--radius)] border px-2 py-0.5 text-xs font-medium w-fit whitespace-nowrap shrink-0 [&>svg]:size-3 gap-1 [&>svg]:pointer-events-none transition-colors overflow-hidden", | |
| "inline-flex items-center justify-center rounded-[var(--radius)] border px-2 py-0.5 text-xs font-medium w-fit whitespace-nowrap shrink-0 [&>svg]:size-3 gap-1 [&>svg]:pointer-events-none transition-colors overflow-hidden", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/badge.tsx` at line 8, The base class string for the Badge
component includes "cursor-pointer" which makes every Badge appear interactive;
remove "cursor-pointer" from the default class list in the Badge component and
instead add an explicit interactive variant (e.g., a boolean prop named
interactive on the Badge component) that, when true, appends "cursor-pointer" to
the className (and optionally sets accessible attributes like role/tabIndex if
used as a control). Update the Badge component's props/type to accept the new
interactive prop and update any call sites that expect clickability to pass
interactive={true}.
| data-slot="select-item" | ||
| className={cn( | ||
| "focus:bg-accent focus:text-accent-foreground [&_svg:not([class*='text-'])]:text-muted-foreground relative flex w-full cursor-default items-center gap-2 rounded-sm py-1.5 pr-8 pl-2 text-sm outline-hidden select-none data-[disabled]:pointer-events-none data-[disabled]:opacity-50 [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4 *:[span]:last:flex *:[span]:last:items-center *:[span]:last:gap-2", | ||
| "focus:bg-accent focus:text-accent-foreground [&_svg:not([class*='text-'])]:text-muted-foreground relative flex w-full cursor-pointer items-center gap-2 rounded-sm py-1.5 pr-8 pl-2 text-sm outline-hidden select-none data-[disabled]:pointer-events-none data-[disabled]:opacity-50 [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4 *:[span]:last:flex *:[span]:last:items-center *:[span]:last:gap-2", |
There was a problem hiding this comment.
The main select control still won't show a pointer.
These updates only affect the opened menu. The element users actually hover before opening a select is SelectTrigger, so controls like the navbar language selector will still keep the default cursor.
Suggested fix
function SelectTrigger({
className,
size = "default",
children,
...props
}: React.ComponentProps<typeof SelectPrimitive.Trigger> & {
size?: "sm" | "default"
}) {
return (
<SelectPrimitive.Trigger
data-slot="select-trigger"
data-size={size}
className={cn(
- "border-input data-[placeholder]:text-muted-foreground [&_svg:not([class*='text-'])]:text-muted-foreground focus-visible:border-ring focus-visible:ring-ring/50 aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive dark:bg-input/30 dark:hover:bg-input/50 flex w-fit items-center justify-between gap-2 rounded-md border bg-transparent px-3 py-2 text-sm whitespace-nowrap shadow-xs transition-[color,box-shadow] outline-none focus-visible:ring-[3px] disabled:cursor-not-allowed disabled:opacity-50 data-[size=default]:h-9 data-[size=sm]:h-8 *:data-[slot=select-value]:line-clamp-1 *:data-[slot=select-value]:flex *:data-[slot=select-value]:items-center *:data-[slot=select-value]:gap-2 [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4",
+ "border-input data-[placeholder]:text-muted-foreground [&_svg:not([class*='text-'])]:text-muted-foreground focus-visible:border-ring focus-visible:ring-ring/50 aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive dark:bg-input/30 dark:hover:bg-input/50 flex w-fit cursor-pointer items-center justify-between gap-2 rounded-md border bg-transparent px-3 py-2 text-sm whitespace-nowrap shadow-xs transition-[color,box-shadow] outline-none focus-visible:ring-[3px] disabled:cursor-not-allowed disabled:opacity-50 data-[size=default]:h-9 data-[size=sm]:h-8 *:data-[slot=select-value]:line-clamp-1 *:data-[slot=select-value]:flex *:data-[slot=select-value]:items-center *:data-[slot=select-value]:gap-2 [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4",
className
)}
{...props}
>Also applies to: 148-148, 166-166
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/select.tsx` at line 112, The SelectTrigger element (and any
other trigger-class definitions) is missing the pointer cursor, so update the
className strings used by the SelectTrigger component(s) to include a
non-disabled cursor class (e.g., add "cursor-pointer" to the SelectTrigger class
list) while preserving the disabled rule (data-[disabled]:pointer-events-none
and data-[disabled]:opacity-50) so disabled triggers don’t show the pointer;
apply the same change to the other trigger definitions referenced in this file
(the other class strings around the same area noted in the review).
Author
|
@copilot open a new pull request to apply changes based on the comments in this thread |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request resolves issue #1003 by adding explicit pointer cursor styles to interactive components across the application. The changes involve updating the core user interface elements to ensure they always display a pointer cursor upon hover. Specifically the updates apply to buttons, badges, dropdown menus, selects, tabs, and category items. Previously some of these elements relied on default system settings which did not visually indicate interactivity properly. By applying the appropriate pointer class, the application now provides a completely consistent and expected experience for users. This improvement directly addresses the reported feedback regarding accessibility and navigation clarity.