Conversation
mudler
commented
Feb 17, 2026
- Allow to switch dark/light theme
- put navbar to the left
- improve agent settings view
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
There was a problem hiding this comment.
Pull request overview
This PR implements a comprehensive UI overhaul for LocalAGI, introducing a dark/light theme switcher, a left-aligned navigation sidebar, and improved agent settings views. The changes modernize the application's appearance and improve navigation UX.
Changes:
- Implements dual-theme support (dark/light) with CSS custom properties and a React context-based theme system
- Replaces top navigation bar with a fixed left sidebar containing logo, navigation links, system status, and theme toggle
- Refactors AgentStatus and AgentSettings pages with improved layouts, collapsible sections, and enhanced observable card components
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| theme.css | Restructures CSS variables to support both dark and light themes with separate color definitions |
| ThemeContext.jsx | New React context provider for managing theme state and persistence via localStorage |
| ThemeToggle.jsx | New toggle component with animated switch for theme selection |
| Sidebar.jsx | New (unused) sidebar component - appears to be duplicate implementation |
| App.jsx | Integrates inline sidebar implementation replacing top navbar, adds theme provider integration |
| App.css | Major CSS refactor: adds sidebar layout styles, theme transitions, observable card styles, and responsive mobile support |
| Home.jsx | Removes logo (now in sidebar) |
| AgentStatus.jsx | Major refactor: adds ObservableCard component, improves layout with collapsible sections, enhances observable summaries |
| AgentSettings.jsx | Improves header layout with agent name, status badge, and action buttons |
| main.jsx | Wraps app with ThemeProvider |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {/* Page Header */} | ||
| <header className="page-header"> | ||
| <div className="header-title-section"> | ||
| <div style={{ display: 'flex', alignItems: 'center', gap: '0.75rem' }}> |
There was a problem hiding this comment.
Inline styles should be moved to CSS classes for consistency. Consider creating a CSS class like 'back-link-wrapper' or using existing utility classes.
| <div style={{ display: 'flex', alignItems: 'center', gap: '0.75rem' }}> | |
| <div className="back-link-wrapper"> |
| <div style={{ marginTop: '1rem' }}> | ||
| {(Array.isArray(statusData?.History) && statusData.History.length === 0) && ( | ||
| <div style={{ color: 'var(--color-text-muted)', textAlign: 'center', padding: '2rem' }}> | ||
| <i className="fas fa-inbox" style={{ fontSize: '2rem', marginBottom: '0.5rem', display: 'block' }} /> | ||
| No status history available | ||
| </div> | ||
| )} | ||
| {Array.isArray(statusData?.History) && statusData.History.map((item, idx) => ( | ||
| <div key={idx} className="card" style={{ marginBottom: '0.75rem' }}> | ||
| {typeof item === 'string' | ||
| ? item.replace(/<br\s*\/?>/gi, '\n') | ||
| : JSON.stringify(item, null, 2)} | ||
| </div> | ||
| ))} | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Multiple inline styles throughout the status display. Consider creating CSS classes for these styles (e.g., 'status-history-card', 'empty-state-container') to improve maintainability and enable theme transitions.
| <div style={{ textAlign: 'center', padding: '3rem', color: 'var(--color-text-muted)' }}> | ||
| <i className="fas fa-satellite-dish" style={{ fontSize: '3rem', marginBottom: '1rem', display: 'block', opacity: 0.5 }} /> | ||
| <h3 style={{ marginBottom: '0.5rem' }}>No Observables Yet</h3> | ||
| <p>Connectors will create observables when the agent is triggered.</p> | ||
| </div> |
There was a problem hiding this comment.
Inline styles for empty state styling. Consider creating a CSS class like 'empty-state-content' or 'no-data-message' for better maintainability.
| {hasChildren && ( | ||
| <div className="observable-children"> | ||
| <h4 style={{ marginBottom: '0.75rem', color: 'var(--color-text-secondary)', fontSize: '0.9rem' }}> | ||
| Nested Observables | ||
| </h4> | ||
| {observable.children.map(child => ( | ||
| <div key={child.id} className="observable-card nested"> | ||
| <div className="observable-header" onClick={() => toggleChild(child.id)}> | ||
| <div className="observable-title"> | ||
| <div className="observable-icon"> | ||
| <i className={`fas fa-${child.icon || 'robot'}`} /> | ||
| </div> | ||
| <div className="observable-info"> | ||
| <div className="observable-name"> | ||
| {child.name} | ||
| <span className="observable-id">#{child.id}</span> | ||
| </div> | ||
| <ObservableSummary observable={child} /> | ||
| </div> | ||
| </div> | ||
| <div className="observable-actions"> | ||
| {!child.completion && <div className="spinner" />} | ||
| <i className={`fas fa-chevron-down observable-toggle ${expandedChildren.get(child.id) ? 'expanded' : ''}`} /> | ||
| </div> | ||
| </div> | ||
| {expandedChildren.get(child.id) && ( | ||
| <div className="observable-content"> | ||
| <CollapsibleRawSections container={child} /> | ||
| </div> | ||
| )} | ||
| </div> | ||
| ))} |
There was a problem hiding this comment.
The ObservableCard component manually renders nested observables inline rather than recursively calling itself. This creates code duplication and makes it harder to support deeper nesting levels. Consider refactoring to make ObservableCard truly recursive by having it render itself for children.
| ))} | ||
| </div> | ||
| <p className="status-section-description"> | ||
| Real-time summary of the agent's thoughts and actions |
There was a problem hiding this comment.
The HTML entity ''' is used here, but in JSX it's more common and clearer to use the apostrophe character directly ('s → 's) or use template literals. This improves code readability.
| Real-time summary of the agent's thoughts and actions | |
| Real-time summary of the agent's thoughts and actions |
| </h4> | ||
| {observable.children.map(child => ( | ||
| <div key={child.id} className="observable-card nested"> | ||
| <div className="observable-header" onClick={() => toggleChild(child.id)}> |
There was a problem hiding this comment.
The nested observable header also lacks keyboard accessibility. This should have role="button", tabIndex="0", and onKeyDown handlers to enable keyboard interaction, especially since it's a clickable element for expanding/collapsing content.
| import { useState } from 'react'; | ||
| import { Outlet, Link, useLocation } from 'react-router-dom'; | ||
|
|
||
| const Sidebar = ({ children }) => { | ||
| const [collapsed, setCollapsed] = useState(false); | ||
| const [mobileOpen, setMobileOpen] = useState(false); | ||
| const location = useLocation(); | ||
|
|
||
| const navItems = [ | ||
| { path: '/', icon: 'fas fa-home', label: 'Home' }, | ||
| { path: '/agents', icon: 'fas fa-users', label: 'Agents' }, | ||
| { path: '/actions-playground', icon: 'fas fa-bolt', label: 'Actions' }, | ||
| { path: '/group-create', icon: 'fas fa-users-cog', label: 'Groups' }, | ||
| ]; | ||
|
|
||
| const isActive = (path) => { | ||
| if (path === '/') return location.pathname === '/'; | ||
| return location.pathname.startsWith(path); | ||
| }; | ||
|
|
||
| const toggleMobile = () => setMobileOpen(!mobileOpen); | ||
| const closeMobile = () => setMobileOpen(false); | ||
|
|
||
| return ( | ||
| <div className={`app-layout ${collapsed ? 'sidebar-collapsed' : ''}`}> | ||
| {/* Mobile Overlay */} | ||
| {mobileOpen && <div className="sidebar-overlay" onClick={closeMobile} />} | ||
|
|
||
| {/* Sidebar */} | ||
| <aside className={`sidebar ${mobileOpen ? 'mobile-open' : ''}`}> | ||
| {/* Logo */} | ||
| <div className="sidebar-header"> | ||
| <Link to="/" className="sidebar-logo" onClick={closeMobile}> | ||
| <div className="logo-icon"> | ||
| <img src="/app/logo_1.png" alt="LocalAGI" /> | ||
| </div> | ||
| {!collapsed && <span className="logo-text">LocalAGI</span>} | ||
| </Link> | ||
| <button | ||
| className="sidebar-toggle desktop-only" | ||
| onClick={() => setCollapsed(!collapsed)} | ||
| title={collapsed ? 'Expand sidebar' : 'Collapse sidebar'} | ||
| > | ||
| <i className={`fas fa-chevron-${collapsed ? 'right' : 'left'}`} /> | ||
| </button> | ||
| </div> | ||
|
|
||
| {/* Navigation */} | ||
| <nav className="sidebar-nav"> | ||
| <ul className="nav-list"> | ||
| {navItems.map((item) => ( | ||
| <li key={item.path} className="nav-item"> | ||
| <Link | ||
| to={item.path} | ||
| className={`nav-link ${isActive(item.path) ? 'active' : ''}`} | ||
| onClick={closeMobile} | ||
| title={collapsed ? item.label : ''} | ||
| > | ||
| <i className={item.icon} /> | ||
| {!collapsed && <span className="nav-label">{item.label}</span>} | ||
| </Link> | ||
| </li> | ||
| ))} | ||
| </ul> | ||
| </nav> | ||
|
|
||
| {/* Status Footer */} | ||
| <div className="sidebar-footer"> | ||
| <div className="status-indicator-wrapper"> | ||
| <span className="status-dot" /> | ||
| {!collapsed && ( | ||
| <span className="status-label"> | ||
| System <strong>Active</strong> | ||
| </span> | ||
| )} | ||
| </div> | ||
| </div> | ||
| </aside> | ||
|
|
||
| {/* Main Area */} | ||
| <div className="main-area"> | ||
| {/* Mobile Header */} | ||
| <header className="mobile-header"> | ||
| <button className="mobile-menu-btn" onClick={toggleMobile}> | ||
| <i className="fas fa-bars" /> | ||
| </button> | ||
| <span className="mobile-title">LocalAGI</span> | ||
| </header> | ||
|
|
||
| {/* Page Content */} | ||
| <main className="content-area"> | ||
| {children} | ||
| </main> | ||
| </div> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| export default Sidebar; |
There was a problem hiding this comment.
The Sidebar component in this file appears to be unused and duplicates the sidebar implementation in App.jsx. This creates confusion and maintenance burden. Consider either removing this component or refactoring App.jsx to import and use this Sidebar component instead of implementing the sidebar inline.
| import { useState } from 'react'; | |
| import { Outlet, Link, useLocation } from 'react-router-dom'; | |
| const Sidebar = ({ children }) => { | |
| const [collapsed, setCollapsed] = useState(false); | |
| const [mobileOpen, setMobileOpen] = useState(false); | |
| const location = useLocation(); | |
| const navItems = [ | |
| { path: '/', icon: 'fas fa-home', label: 'Home' }, | |
| { path: '/agents', icon: 'fas fa-users', label: 'Agents' }, | |
| { path: '/actions-playground', icon: 'fas fa-bolt', label: 'Actions' }, | |
| { path: '/group-create', icon: 'fas fa-users-cog', label: 'Groups' }, | |
| ]; | |
| const isActive = (path) => { | |
| if (path === '/') return location.pathname === '/'; | |
| return location.pathname.startsWith(path); | |
| }; | |
| const toggleMobile = () => setMobileOpen(!mobileOpen); | |
| const closeMobile = () => setMobileOpen(false); | |
| return ( | |
| <div className={`app-layout ${collapsed ? 'sidebar-collapsed' : ''}`}> | |
| {/* Mobile Overlay */} | |
| {mobileOpen && <div className="sidebar-overlay" onClick={closeMobile} />} | |
| {/* Sidebar */} | |
| <aside className={`sidebar ${mobileOpen ? 'mobile-open' : ''}`}> | |
| {/* Logo */} | |
| <div className="sidebar-header"> | |
| <Link to="/" className="sidebar-logo" onClick={closeMobile}> | |
| <div className="logo-icon"> | |
| <img src="/app/logo_1.png" alt="LocalAGI" /> | |
| </div> | |
| {!collapsed && <span className="logo-text">LocalAGI</span>} | |
| </Link> | |
| <button | |
| className="sidebar-toggle desktop-only" | |
| onClick={() => setCollapsed(!collapsed)} | |
| title={collapsed ? 'Expand sidebar' : 'Collapse sidebar'} | |
| > | |
| <i className={`fas fa-chevron-${collapsed ? 'right' : 'left'}`} /> | |
| </button> | |
| </div> | |
| {/* Navigation */} | |
| <nav className="sidebar-nav"> | |
| <ul className="nav-list"> | |
| {navItems.map((item) => ( | |
| <li key={item.path} className="nav-item"> | |
| <Link | |
| to={item.path} | |
| className={`nav-link ${isActive(item.path) ? 'active' : ''}`} | |
| onClick={closeMobile} | |
| title={collapsed ? item.label : ''} | |
| > | |
| <i className={item.icon} /> | |
| {!collapsed && <span className="nav-label">{item.label}</span>} | |
| </Link> | |
| </li> | |
| ))} | |
| </ul> | |
| </nav> | |
| {/* Status Footer */} | |
| <div className="sidebar-footer"> | |
| <div className="status-indicator-wrapper"> | |
| <span className="status-dot" /> | |
| {!collapsed && ( | |
| <span className="status-label"> | |
| System <strong>Active</strong> | |
| </span> | |
| )} | |
| </div> | |
| </div> | |
| </aside> | |
| {/* Main Area */} | |
| <div className="main-area"> | |
| {/* Mobile Header */} | |
| <header className="mobile-header"> | |
| <button className="mobile-menu-btn" onClick={toggleMobile}> | |
| <i className="fas fa-bars" /> | |
| </button> | |
| <span className="mobile-title">LocalAGI</span> | |
| </header> | |
| {/* Page Content */} | |
| <main className="content-area"> | |
| {children} | |
| </main> | |
| </div> | |
| </div> | |
| ); | |
| }; | |
| export default Sidebar; | |
| // Sidebar component removed. | |
| // The sidebar layout is implemented directly in App.jsx, and this file | |
| // previously contained a duplicated, unused Sidebar component. | |
| // Keeping this file as a documented placeholder avoids confusion if it is | |
| // encountered in the codebase or by tooling. |
| * { | ||
| transition: background-color var(--duration-slow) var(--ease-default), | ||
| border-color var(--duration-slow) var(--ease-default), | ||
| color var(--duration-slow) var(--ease-default); | ||
| } |
There was a problem hiding this comment.
Using the universal selector (*) with transitions on multiple properties can cause performance issues, especially on complex pages. Consider being more selective by targeting specific element types or using a CSS class that components can opt into for theme transitions.
| // Don't render until theme is determined to prevent flash | ||
| if (!isReady) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Returning null while loading the theme will cause a flash of no content before the theme is ready. Consider rendering a minimal loading state or applying the theme attribute synchronously before the first render to avoid this flash. Alternatively, you could set the theme attribute via a script tag in index.html that runs before React mounts.
| @@ -264,17 +311,14 @@ function AgentStatus() { | |||
| return roots; | |||
| } | |||
There was a problem hiding this comment.
The buildObservableTree function is defined inside the useEffect but is called from event handlers that are set up once. This means the event handlers capture the initial version of the function in their closure. If this function ever needs to access updated dependencies, it could lead to stale closure bugs. Consider moving this function outside the useEffect or using useCallback to manage its dependencies.