fix(sidebar): align category counts and show actions on hover without layout shift#77
Conversation
📝 WalkthroughWalkthroughUpdates the CategorySidebar component's category row UI layout by replacing quote styles and refactoring the badge and action button rendering. The count badge now hides on hover via opacity, while edit/delete/hide buttons are repositioned absolutely and revealed on hover, reducing layout impact. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/CategorySidebar.tsx (1)
166-244:⚠️ Potential issue | 🟠 MajorAvoid nested
<button>elements in the category row.Line 166 wraps the row with a
<button>, and Lines 209-241 add child<button>s inside it. This is invalid interactive nesting and can break click/focus behavior across browsers.Possible structure fix
- <button + <div onDragOver={...} onDragLeave={...} onDrop={(event) => handleDropOnCategory(event, category)} className="group shrink-0 lg:shrink relative" - > + > + <button + type="button" + onClick={() => onCategorySelect(category.id)} + className="relative flex min-w-[140px] items-center justify-between px-3 py-2.5 rounded-lg text-left transition-colors lg:w-full" + > ... - {category.id !== 'all' && ( - <div className="absolute ..."> + </button> + {category.id !== 'all' && ( + <div className="absolute ..."> <button type="button" ... /> <button type="button" ... /> </div> )} - </button> + </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/CategorySidebar.tsx` around lines 166 - 244, The category row currently nests interactive <button> children inside an outer <button> (in CategorySidebar), which is invalid; change the outer clickable container used where onClick calls onCategorySelect and onDrop calls handleDropOnCategory from a <button> to a non-interactive element (e.g., <div> or <li> with role="button" and appropriate keyboard handlers) so inner buttons (calling handleEditCategory, handleDeleteCategory, handleHideDefaultCategory) remain real <button>s; ensure the outer element preserves accessibility by forwarding onClick, onKeyDown (Enter/Space) and drag handlers (onDragOver/onDragLeave/onDrop) and retains the same className/title logic so styling and behavior are unchanged.
🤖 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/CategorySidebar.tsx`:
- Around line 201-208: The action-controls div inside CategorySidebar currently
uses only group-hover classes (e.g., "group-hover:opacity-100" and
"group-hover:pointer-events-auto") so keyboard users can't reveal them; update
the visibility/pointer-event utility classes to also respond to keyboard focus
by using focus/focus-within variants (e.g., replace or add
"group-focus-within:opacity-100" and "group-focus-within:pointer-events-auto"
alongside the existing group-hover variants) and ensure the interactive children
(buttons/links) are focusable so they become reachable when the group receives
focus; target the div with the class string containing "absolute right-3 ...
opacity-0 group-hover:opacity-100 transition-opacity pointer-events-none
group-hover:pointer-events-auto" in CategorySidebar and add the corresponding
focus/focus-within class variants.
- Around line 193-204: The count badge is hidden on hover via the CSS token
"group-hover:opacity-0" which causes the `all` category (which has no action
buttons) to lose its count; change the span's class generation so
"group-hover:opacity-0" is only applied for non‑all rows. Add a boolean (e.g.
isAll = category.id === 'all' or whichever identifier you use for the all
category) and update the span's className that currently composes
isSelected/isDragTarget to conditionally include `group-hover:opacity-0` only
when !isAll (e.g. `${!isAll ? 'group-hover:opacity-0' : ''}`) so the badge
remains visible for the all category.
---
Outside diff comments:
In `@src/components/CategorySidebar.tsx`:
- Around line 166-244: The category row currently nests interactive <button>
children inside an outer <button> (in CategorySidebar), which is invalid; change
the outer clickable container used where onClick calls onCategorySelect and
onDrop calls handleDropOnCategory from a <button> to a non-interactive element
(e.g., <div> or <li> with role="button" and appropriate keyboard handlers) so
inner buttons (calling handleEditCategory, handleDeleteCategory,
handleHideDefaultCategory) remain real <button>s; ensure the outer element
preserves accessibility by forwarding onClick, onKeyDown (Enter/Space) and drag
handlers (onDragOver/onDragLeave/onDrop) and retains the same className/title
logic so styling and behavior are unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 23c82f89-777b-4185-a181-d9461ab397ee
📒 Files selected for processing (1)
src/components/CategorySidebar.tsx
| {/* 数字 badge - 正常状态显示,hover 时隐藏 */} | ||
| <span | ||
| className={`text-xs px-2 py-1 rounded-full shrink-0 transition-opacity ${ | ||
| isSelected | ||
| ? 'bg-blue-200 text-blue-800 dark:bg-blue-800 dark:text-blue-200' | ||
| : isDragTarget | ||
| ? 'bg-green-200 text-green-800 dark:bg-green-800 dark:text-green-200' | ||
| : 'bg-gray-200 text-gray-600 dark:bg-gray-600 dark:text-gray-400' | ||
| }`}> | ||
| <span>{count}</span> | ||
| </div> | ||
| } group-hover:opacity-0`} | ||
| > | ||
| {count} | ||
| </span> |
There was a problem hiding this comment.
Keep the count visible for the all category.
Line 201 hides the badge on hover for every row, but all has no action buttons (Lines 207-243). Hovering all currently removes the count with nothing replacing it.
Conditional badge hide
- } group-hover:opacity-0`}
+ } ${category.id !== 'all' ? 'group-hover:opacity-0 group-focus-within:opacity-0' : ''}`}📝 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.
| {/* 数字 badge - 正常状态显示,hover 时隐藏 */} | |
| <span | |
| className={`text-xs px-2 py-1 rounded-full shrink-0 transition-opacity ${ | |
| isSelected | |
| ? 'bg-blue-200 text-blue-800 dark:bg-blue-800 dark:text-blue-200' | |
| : isDragTarget | |
| ? 'bg-green-200 text-green-800 dark:bg-green-800 dark:text-green-200' | |
| : 'bg-gray-200 text-gray-600 dark:bg-gray-600 dark:text-gray-400' | |
| }`}> | |
| <span>{count}</span> | |
| </div> | |
| } group-hover:opacity-0`} | |
| > | |
| {count} | |
| </span> | |
| {/* 数字 badge - 正常状态显示,hover 时隐藏 */} | |
| <span | |
| className={`text-xs px-2 py-1 rounded-full shrink-0 transition-opacity ${ | |
| isSelected | |
| ? 'bg-blue-200 text-blue-800 dark:bg-blue-800 dark:text-blue-200' | |
| : isDragTarget | |
| ? 'bg-green-200 text-green-800 dark:bg-green-800 dark:text-green-200' | |
| : 'bg-gray-200 text-gray-600 dark:bg-gray-600 dark:text-gray-400' | |
| } ${category.id !== 'all' ? 'group-hover:opacity-0 group-focus-within:opacity-0' : ''}`} | |
| > | |
| {count} | |
| </span> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/CategorySidebar.tsx` around lines 193 - 204, The count badge
is hidden on hover via the CSS token "group-hover:opacity-0" which causes the
`all` category (which has no action buttons) to lose its count; change the
span's class generation so "group-hover:opacity-0" is only applied for non‑all
rows. Add a boolean (e.g. isAll = category.id === 'all' or whichever identifier
you use for the all category) and update the span's className that currently
composes isSelected/isDragTarget to conditionally include
`group-hover:opacity-0` only when !isAll (e.g. `${!isAll ?
'group-hover:opacity-0' : ''}`) so the badge remains visible for the all
category.
| } group-hover:opacity-0`} | ||
| > | ||
| {count} | ||
| </span> | ||
|
|
||
| {category.id !== 'all' && ( | ||
| {/* 操作按钮 - 绝对定位,hover 时显示,不占位 */} | ||
| {category.id !== 'all' && ( | ||
| <div className="absolute right-3 top-1/2 -translate-y-1/2 flex items-center gap-1 opacity-0 group-hover:opacity-100 transition-opacity pointer-events-none group-hover:pointer-events-auto"> |
There was a problem hiding this comment.
Show action controls on keyboard focus, not hover only.
Line 208 uses group-hover only. Keyboard users won’t reliably discover/reach these controls without a focus-based visibility path.
Accessibility-focused class update
- } group-hover:opacity-0`}
+ } group-hover:opacity-0 group-focus-within:opacity-0`}
- <div className="absolute right-3 top-1/2 -translate-y-1/2 flex items-center gap-1 opacity-0 group-hover:opacity-100 transition-opacity pointer-events-none group-hover:pointer-events-auto">
+ <div className="absolute right-3 top-1/2 -translate-y-1/2 flex items-center gap-1 opacity-0 group-hover:opacity-100 group-focus-within:opacity-100 transition-opacity pointer-events-none group-hover:pointer-events-auto group-focus-within:pointer-events-auto">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/CategorySidebar.tsx` around lines 201 - 208, The
action-controls div inside CategorySidebar currently uses only group-hover
classes (e.g., "group-hover:opacity-100" and "group-hover:pointer-events-auto")
so keyboard users can't reveal them; update the visibility/pointer-event utility
classes to also respond to keyboard focus by using focus/focus-within variants
(e.g., replace or add "group-focus-within:opacity-100" and
"group-focus-within:pointer-events-auto" alongside the existing group-hover
variants) and ensure the interactive children (buttons/links) are focusable so
they become reachable when the group receives focus; target the div with the
class string containing "absolute right-3 ... opacity-0 group-hover:opacity-100
transition-opacity pointer-events-none group-hover:pointer-events-auto" in
CategorySidebar and add the corresponding focus/focus-within class variants.
Summary
Before
After