feet: チャッピー風デザインにする#5
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements a significant UI redesign introducing a persistent sidebar layout for better navigation and user experience. The redesign consolidates repository settings into a dedicated Settings page, removes inline repository configuration from the Dashboard, and adds a collapsible sidebar showing recent jobs.
Changes:
- Added a Layout component with collapsible sidebar for persistent navigation
- Created a Settings page for centralized repository configuration
- Refactored Dashboard to remove repository/branch inputs and job list display
- Updated styling to use darker theme colors and larger font sizes (14px → 16px)
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/components/Layout.tsx | New layout wrapper with sidebar state management and outlet for child routes |
| frontend/src/components/Sidebar.tsx | New sidebar component displaying job list, logo, and navigation |
| frontend/src/hooks/useLayoutContext.ts | New hook providing access to layout context (refreshJobs function) |
| frontend/src/pages/Settings.tsx | New settings page for repository URL and branch configuration |
| frontend/src/pages/Dashboard.tsx | Simplified to remove repo/branch inputs and job list; now fetches settings from API |
| frontend/src/pages/JobDetail.tsx | Removed back navigation links; updated font sizes from 14px to 16px |
| frontend/src/main.tsx | Updated routing structure to use nested routes under Layout component |
| frontend/src/index.css | Removed body centering styles; added full-height #root container |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <button | ||
| onClick={onToggle} | ||
| style={{ | ||
| background: 'none', | ||
| border: 'none', | ||
| cursor: 'pointer', | ||
| padding: '4px', | ||
| display: 'flex', | ||
| alignItems: 'center', | ||
| }} | ||
| > |
There was a problem hiding this comment.
The toggle button should have an aria-label to describe its purpose for screen reader users. Add aria-label="Close sidebar" or similar to improve accessibility.
| import { useNavigate, useLocation } from 'react-router-dom' | ||
| import { type Job } from '../services/api' | ||
| import StatusBadge from './StatusBadge' | ||
| import logoIcon from '../assets/claude_desgin.png' |
There was a problem hiding this comment.
The filename 'claude_desgin.png' appears to have a typo. It should likely be 'claude_design.png'. Note that if you rename the file, you'll also need to update the import statement accordingly.
| import logoIcon from '../assets/claude_desgin.png' | |
| import logoIcon from '../assets/claude_design.png' |
| > | ||
| {repoUrl ? ( | ||
| <> | ||
| {repoUrl.replace('https://github.com/', '')} |
There was a problem hiding this comment.
The string replace operation assumes the URL starts with 'https://github.com/'. If the URL doesn't match this pattern (e.g., 'http://github.com/', 'https://gitlab.com/'), it won't be replaced and will display the full URL. Consider using a more robust URL parsing approach or handle non-GitHub URLs more gracefully.
| {repoUrl.replace('https://github.com/', '')} | |
| {repoUrl.replace(/^https?:\/\/github\.com\//, '')} |
|
|
||
| return ( | ||
| <div style={{ display: 'flex', minHeight: '100vh' }}> | ||
| <Sidebar jobs={jobs} isOpen={sidebarOpen} onToggle={() => setSidebarOpen(false)} /> |
There was a problem hiding this comment.
When the sidebar is toggled, focus management should be considered for accessibility. When closing the sidebar, focus should move to the hamburger menu button. When opening, consider moving focus to a meaningful element within the sidebar or keeping it on the toggle button. This improves keyboard navigation experience.
| <div | ||
| onClick={() => navigate('/settings')} | ||
| style={{ | ||
| padding: '0 16px', | ||
| height: '63px', | ||
| borderTop: '1px solid #374151', | ||
| cursor: 'pointer', | ||
| display: 'flex', | ||
| alignItems: 'center', | ||
| gap: '8px', | ||
| fontSize: '16px', | ||
| color: '#9ca3af', | ||
| backgroundColor: location.pathname === '/settings' ? '#374151' : 'transparent', | ||
| }} | ||
| > |
There was a problem hiding this comment.
The settings item is a clickable div without proper semantic HTML or keyboard navigation. Consider using a button element or adding tabIndex and onKeyDown handlers to support keyboard navigation (Enter/Space keys) for better accessibility.
| textOverflow: 'ellipsis', | ||
| }} | ||
| > | ||
| {job.repo_url.replace('https://github.com/', '')} |
There was a problem hiding this comment.
The string replace operation assumes the URL starts with 'https://github.com/'. If the URL doesn't match this pattern (e.g., 'http://github.com/', 'https://gitlab.com/'), it won't be replaced and will display the full URL. Consider using a more robust URL parsing approach or handle non-GitHub URLs more gracefully.
|
|
||
| return ( | ||
| <div style={{ display: 'flex', minHeight: '100vh' }}> | ||
| <Sidebar jobs={jobs} isOpen={sidebarOpen} onToggle={() => setSidebarOpen(false)} /> |
There was a problem hiding this comment.
The onToggle callback always closes the sidebar (setSidebarOpen(false)) rather than toggling its state. While this may be intentional for this specific button, the naming 'onToggle' is misleading. Consider renaming to 'onClose' to better reflect the actual behavior.
| <Sidebar jobs={jobs} isOpen={sidebarOpen} onToggle={() => setSidebarOpen(false)} /> | |
| <Sidebar jobs={jobs} isOpen={sidebarOpen} onToggle={() => setSidebarOpen((prev) => !prev)} /> |
| borderRadius: '6px', | ||
| fontSize: '16px', | ||
| fontWeight: 600, | ||
| cursor: isSaving ? 'not-allowed' : 'pointer', |
There was a problem hiding this comment.
The cursor style should also account for the disabled state when repoUrl is empty. Currently it only checks isSaving, but the button can also be disabled when !repoUrl is true. The cursor should be 'not-allowed' in both cases to match the disabled state and provide consistent user experience.
| cursor: isSaving ? 'not-allowed' : 'pointer', | |
| cursor: isSaving || !repoUrl ? 'not-allowed' : 'pointer', |
| alignItems: 'center', | ||
| }} | ||
| > | ||
| <span style={{ fontSize: '30px', color: '#9ca3af' }}>☰</span> |
There was a problem hiding this comment.
Using emoji characters (☰) for interactive UI elements can cause accessibility issues. Screen readers may not announce them meaningfully. Consider using an SVG icon with proper aria-label or replace with actual text for better accessibility.
| backgroundColor: location.pathname === '/settings' ? '#374151' : 'transparent', | ||
| }} | ||
| > | ||
| <span style={{ fontSize: '30px' }}>⚙</span> |
There was a problem hiding this comment.
Using emoji characters (⚙) for interactive UI elements can cause accessibility issues. Screen readers may not announce them meaningfully. Consider using an SVG icon with proper aria-label or replace with actual text for better accessibility.
| <span style={{ fontSize: '30px' }}>⚙</span> |
No description provided.