Skip to content

fix(ui): larger collapsed-sidebar icons (Projects/History/Exports) - #192

Merged
debpalash merged 1 commit into
mainfrom
fix/sidebar-collapsed-icons
May 30, 2026
Merged

fix(ui): larger collapsed-sidebar icons (Projects/History/Exports)#192
debpalash merged 1 commit into
mainfrom
fix/sidebar-collapsed-icons

Conversation

@debpalash

@debpalash debpalash commented May 30, 2026

Copy link
Copy Markdown
Owner

When the sidebar is collapsed, the Projects/History/Exports tab icons were rendered at size=13 — too small as the sole affordance. Bumped to 18 when collapsed (expanded tab bar keeps 13). typecheck/build ✓.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Sidebar tab icons now intelligently adjust their size based on sidebar state—expanding when collapsed and shrinking when expanded for improved visual clarity.

Review Change Stack

…llapsed

The collapsed sidebar rail showed the tab icons at size 13 — too small to read
as the only affordance. Bump to 18 when collapsed; the expanded tab bar keeps 13.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 30, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 19f68bc9-4121-4b4e-bcf6-f6137d940dc5

📥 Commits

Reviewing files that changed from the base of the PR and between 7d04200 and 5cda0df.

📒 Files selected for processing (1)
  • frontend/src/components/Sidebar.jsx

📝 Walkthrough

Walkthrough

The Sidebar component now renders tab icons at responsive sizes based on the sidebar's collapse state. When collapsed, icons display larger; when expanded, they display smaller. This is implemented by switching the size prop value of the icon component based on the isSidebarCollapsed boolean state.

Changes

Sidebar Icon Sizing

Layer / File(s) Summary
Icon size responsive to sidebar state
frontend/src/components/Sidebar.jsx
The tab icon's size prop is changed from a fixed value to a conditional expression based on isSidebarCollapsed, rendering larger icons when the sidebar is collapsed and smaller when expanded.

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is missing most required template sections including Summary, Changes list, Type checkbox, Testing details, and Checklist items. Add the missing template sections: provide a structured Summary, itemized Changes list, check the appropriate Type box (appears to be 🐛 Bug fix), document Testing approach, and complete the Checklist items.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: increasing icon sizes in the collapsed sidebar for specific UI components.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/sidebar-collapsed-icons

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented May 30, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR bumps the Lucide icon size prop from 13 to 18 in the collapsed-sidebar tab bar to make Projects/History/Exports tabs more usable when text labels are hidden.

  • The one-line change adds isSidebarCollapsed ? 18 : 13 inline, keeping 13 in the expanded state and matching the size={18} already used throughout the rest of the collapsed sidebar (IconTile, sidebar-tile elements).
  • Sidebar.css already contains .sidebar.is-collapsed .sidebar__tab svg { width: 18px; height: 18px; }, which overrides SVG presentation attributes and would have produced 18×18 rendering before this change — so the visual fix may have been a no-op, and the JS ternary now duplicates that CSS rule.

Confidence Score: 4/5

Safe to merge — the change is a one-line prop adjustment with no functional regressions.

The change is minimal and isolated to the tab icon rendering. The only notable finding is that Sidebar.css already enforced 18×18 on these icons via a CSS rule, so the JS ternary is a duplicate source of truth rather than a real fix. No new behaviour is introduced and no existing behaviour is broken.

The CSS rule in Sidebar.css lines 26-29 and the new JS ternary in Sidebar.jsx line 109 both control the same icon dimension — worth keeping in sync if either is changed later.

Important Files Changed

Filename Overview
frontend/src/components/Sidebar.jsx Single-line change bumps tab icon size prop from 13 to 18 when sidebar is collapsed; redundant with a pre-existing CSS rule that already enforces 18×18 on those same SVG elements

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Sidebar renders tab bar] --> B{isSidebarCollapsed?}
    B -- yes --> C["Icon size prop = 18"]
    B -- no --> D["Icon size prop = 13"]
    C --> E["SVG width/height attr = 18"]
    D --> F["SVG width/height attr = 13"]
    E --> G["CSS rule: .is-collapsed .sidebar__tab svg\nwidth: 18px; height: 18px"]
    F --> H["No CSS override — renders at 13px"]
    G --> I["Renders at 18px ✓"]
    style G fill:#faad14,color:#000
    style E fill:#52c41a,color:#000
    style I fill:#52c41a,color:#000
Loading

Fix All in Claude Code

Reviews (1): Last reviewed commit: "fix(ui): larger Projects/History/Exports..." | Re-trigger Greptile

title={`${tabLabel[id]} (${tabCount[id]})`}
>
<Icon size={13} />
<Icon size={isSidebarCollapsed ? 18 : 13} />

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 CSS already enforces 18 px on collapsed tab icons

Sidebar.css already has .sidebar.is-collapsed .sidebar__tab svg { width: 18px; height: 18px; } (lines 26-29), which overrides Lucide's SVG presentation attributes regardless of what the size prop is set to. This means the icons were already rendering at 18×18 when collapsed before this change — the described bug may not have actually existed. The new ternary is harmless and makes the prop value self-documenting, but it is redundant with the CSS rule. Consider removing the CSS override (keeping only the JS prop) or the JS ternary (keeping only the CSS), so the two sources of truth don't diverge silently if either is changed later.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code

@debpalash
debpalash merged commit c70dd4c into main May 30, 2026
15 checks passed
@debpalash
debpalash deleted the fix/sidebar-collapsed-icons branch May 30, 2026 23:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant