Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the frontend layout to use a left-hand sidebar navigation and introduces a set of responsive layout/style adjustments across multiple pages to improve usability on smaller screens.
Changes:
- Replaced the top TabList navigation with a collapsible left sidebar navigation persisted in
localStorage. - Added responsive padding/wrapping/scroll affordances to several feature pages (Workbench, Settings, CSV tickets, Agent chat).
- Improved horizontal overflow handling for wide UI elements (workflow canvas, tabs, tables, dialogs).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/App.jsx | Implements the left sidebar navigation shell, collapse behavior, and responsive app/header/content layout. |
| frontend/src/features/workflow/WorkflowPage.jsx | Adds responsive padding and horizontal scrolling for the workflow canvas; adjusts canvas sizing logic. |
| frontend/src/features/workbench/WorkbenchPage.jsx | Makes header/tabs/layout responsive and adds a horizontal scroller for the TabList. |
| frontend/src/features/workbench/RunsSidePanel.jsx | Adds responsive styles for the runs panel (full-width on smaller screens). |
| frontend/src/features/settings/SettingsPage.jsx | Improves responsiveness for the settings layout and icon picker grid. |
| frontend/src/features/csvtickets/CSVTicketTable.jsx | Makes filters/table/pagination/dialog responsive and adds horizontal scrolling for wide tables. |
| frontend/src/features/agent/AgentChat.jsx | Adds responsive layout adjustments for chat card/header/messages/input area. |
| className={`${styles.navButton} ${isNavCollapsed ? styles.navButtonCollapsed : ''} ${isActive ? styles.navButtonActive : ''}`} | ||
| onClick={() => navigate(tab.path)} | ||
| data-testid={tab.testId} | ||
| aria-label={tab.label} |
There was a problem hiding this comment.
Navigation items no longer expose an active-state attribute. Our Playwright e2e tests (and assistive tech) previously relied on tab semantics (e.g., aria-selected="true" on the active item). Consider restoring equivalent semantics by marking the active button with aria-selected (and adding appropriate role/container role like tab/tablist), or otherwise ensuring there is a stable, testable active-state attribute such as aria-current="page" (and updating tests accordingly).
| aria-label={tab.label} | |
| aria-label={tab.label} | |
| aria-current={isActive ? 'page' : undefined} |
| import { | ||
| Button, | ||
| makeStyles, | ||
| Subtitle2, | ||
| Subtitle1, | ||
| Tab, | ||
| TabList, | ||
| Text, | ||
| tokens, | ||
| } from '@fluentui/react-components' |
There was a problem hiding this comment.
The import blocks have inconsistent indentation (mix of 2 and 4 spaces) which hurts readability and is likely accidental. Please run the repo formatter (or manually normalize indentation) so the import list is consistently aligned.
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
Fixed in f72dd03 — normalized all import indentation in App.jsx to consistent 2-space style (also alphabetically sorted Subtitle1/Subtitle2 while at it).
| const resize = useCallback(() => { | ||
| const c = canvasRef.current, w = wrapRef.current; if (!c || !w) return | ||
| const dpr = window.devicePixelRatio || 1, ww = w.clientWidth | ||
| c.width = ww * dpr; c.height = 460 * dpr | ||
| c.style.width = ww + 'px'; c.style.height = '460px' | ||
| const dpr = window.devicePixelRatio || 1 | ||
| const contentWidth = nodes.reduce((maxX, node) => Math.max(maxX, node.x), 0) + 160 | ||
| const ww = Math.max(w.clientWidth, contentWidth) | ||
| c.width = ww * dpr; c.height = canvasHeight * dpr | ||
| c.style.width = ww + 'px'; c.style.height = `${canvasHeight}px` | ||
| setCanvasReady((v) => v + 1) | ||
| }, []) | ||
| }, [nodes]) | ||
|
|
||
| useEffect(() => { resize(); window.addEventListener('resize', resize); return () => window.removeEventListener('resize', resize) }, [resize]) |
There was a problem hiding this comment.
resize is recreated on every nodes change, which causes the [resize] effect to remove and re-add the window resize event listener each time the workflow updates. Consider keeping a stable window-resize handler (e.g., compute contentWidth separately via useMemo/refs, and only (re)size the canvas on nodes changes without re-registering the global listener) to avoid unnecessary listener churn.
Agent-Logs-Url: https://github.com/abossard/python-quart-vite-react/sessions/7389d2d3-93ec-4ab8-ae29-3d4f3a0cb093 Co-authored-by: btesterc <116953132+btesterc@users.noreply.github.com>
The navigation bar has been positioned on the left-hand side of the page for ease of use. Efforts have been made to ensure the site functions correctly across different screen sizes by making it responsive.