Agentic Sidebar: Polish Site Rows, Chat Indicators, and Action Alignment#3894
Conversation
56f1e40 to
520c63a
Compare
📊 Performance Test ResultsComparing dbb591d vs trunk app-size
site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff) |
bcotrim
left a comment
There was a problem hiding this comment.
Reviewed locally on the branch — really nice polish 👏 The alignment system holds up: site icons + footer gravatar share the leading column, and site titles + chat labels + display name land on the next.
Ran it: eslint ✅, typecheck ✅, site-list test ✅ (2/2), vite build ✅. Dark mode looks great.
RTL doesn't mirror — but this is a pre-existing apps/ui gap, not this PR. apps/ui loads translations (setLocaleData in main.tsx) yet never sets document.documentElement.dir, the way apps/studio does in use-localization-support.ts. So strings localize but the layout stays LTR. This PR is actually RTL-ready (logical properties throughout — the only physical bit is the macOS traffic-light padding, which should stay physical). Worth a separate issue to wire dir=rtl across apps/ui; shouldn't block here.
Left a few inline notes — one cleanup plus a couple of NITs/confirmations. Nothing blocking.
⚠️ Visual change: still needs a light-mode pass (dark ✅ confirmed).
| @@ -16,7 +16,6 @@ export function SidebarHeader( { onToggleSidebar }: Props ) { | |||
| const navigate = useNavigate(); | |||
| return ( | |||
| <div className={ `${ styles.root } ${ isFullscreen ? styles.fullscreen : '' }` }> | |||
There was a problem hiding this comment.
Confirming intent: the header no longer renders the 'Studio' title, so the app name does not appear in the sidebar anymore — just the create/hide buttons now. 👍 if deliberate (it is not called out in the PR description, so flagging).
There was a problem hiding this comment.
I actually quite liked having the title there but that might be a personal preference
There was a problem hiding this comment.
Yup, this was an intentional design decision to remove the Studio title. Its not very common to see apps that have this on macOS, as the app's name appears in the Dock and the Menubar. Windows and Linux both show a title bar, which includes the "Studio" label.
| .actions { | ||
| display: flex; | ||
| align-items: center; | ||
| margin-inline-start: auto; |
There was a problem hiding this comment.
NIT: along with the alignment change, the action buttons lost their resting opacity: 0.65 (and hover/focus → 1) and are now always full opacity. Assuming that's intended alongside 'Align Actions' — just confirming the dimming was dropped on purpose.
There was a problem hiding this comment.
Also intentional. With the opacity, the top action buttons were lighter than all the other actions in the sidebar. Removing the opacity makes all actions the same color.
2b4709b to
dbb591d
Compare
katinthehatsite
left a comment
There was a problem hiding this comment.
The changes look good 👍
How AI was used in this PR
Used Codex to iteratively implement requested visual refinements to the new agentic sidebar and run verification after the changes.
Proposed Changes
Sidebar Header
Site List
Sidebar Footer
Testing Instructions
npm run typecheck.npm test -- apps/ui/src.Pre-merge Checklist