feat: main layout revamp #865
Conversation
WalkthroughRefactors multiple UI components to use MUI Box-based flex layouts, adjusts spacing and widths (sidebar, API key display, menu), moves the informational Alert into ProxyForm's right panel, tweaks text in locales, and simplifies spacing across robot pages; no exported API or signature changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Browser
participant PageWrapper
participant NavBar
participant Routes
participant MainPage
participant MainMenu
participant DisplayContent
User->>Browser: navigate
Browser->>PageWrapper: render
alt not /recording
PageWrapper->>NavBar: render (sticky)
end
PageWrapper->>Routes: render selected route
Routes->>MainPage: render MainPage route
MainPage->>MainMenu: render (sticky, 230px)
MainPage->>DisplayContent: render content (scrollable)
sequenceDiagram
autonumber
actor User
participant ProxyForm
participant LeftForm as "Form Column"
participant RightPanel as "Info/Alert Panel"
User->>ProxyForm: open form
ProxyForm->>LeftForm: render form controls (flex column)
ProxyForm->>RightPanel: render informational Alert (fixed height)
User->>LeftForm: interact (submit/remove)
LeftForm-->>ProxyForm: update state
ProxyForm-->>RightPanel: reflect state changes if needed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/pages/MainPage.tsx (2)
322-332: Consider extracting magic numbers to constants.The values
64(navbar height) and250(sidebar width) are hardcoded in multiple places throughout this file and others in the PR (e.g., PageWrapper.tsx line 107, MainMenu.tsx line 66). If the navbar height or sidebar width changes, all occurrences need to be updated manually.Consider extracting these to shared constants:
// In a shared constants file (e.g., src/constants/layout.ts) export const LAYOUT_CONSTANTS = { NAVBAR_HEIGHT: 64, SIDEBAR_WIDTH: 250, } as const;Then use throughout:
+import { LAYOUT_CONSTANTS } from '../constants/layout'; + return ( - <Box sx={{ display: 'flex', minHeight: 'calc(100vh - 64px)', width: '100%' }}> + <Box sx={{ display: 'flex', minHeight: `calc(100vh - ${LAYOUT_CONSTANTS.NAVBAR_HEIGHT}px)`, width: '100%' }}> <Box sx={{ - width: 250, + width: LAYOUT_CONSTANTS.SIDEBAR_WIDTH, flexShrink: 0, position: 'sticky', - top: 64, + top: LAYOUT_CONSTANTS.NAVBAR_HEIGHT, - height: 'calc(100vh - 64px)', + height: `calc(100vh - ${LAYOUT_CONSTANTS.NAVBAR_HEIGHT}px)`,
337-343: Redundant width calculation in scrollable content area.The
width: 'calc(100% - 250px)'on line 342 is redundant sinceflex: 1on line 338 already makes the content area fill the remaining space. The explicit width calculation adds maintenance overhead (another hardcoded250pxvalue) without functional benefit.Consider removing the redundant width property:
<Box sx={{ flex: 1, minWidth: 0, overflow: 'auto', minHeight: 'calc(100vh - 64px)', - width: 'calc(100% - 250px)' }}>src/components/proxy/ProxyForm.tsx (1)
159-294: Consider adding responsive behavior for smaller screens.The two-column layout uses
display: 'flex'but lacksflexWrapor responsive breakpoints. On smaller screens, both columns (each withmaxWidth: 600) may cause horizontal scrolling rather than stacking vertically.Consider adding responsive behavior:
<Box sx={{ display: 'flex', + flexDirection: { xs: 'column', md: 'row' }, gap: 4, p: 5, width: '100%', maxWidth: '100%', boxSizing: 'border-box' }}>This will stack columns vertically on extra-small screens and display them side-by-side on medium+ screens.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/api/ApiKey.tsx(3 hunks)src/components/proxy/ProxyForm.tsx(6 hunks)src/components/robot/pages/RobotConfigPage.tsx(3 hunks)src/pages/MainPage.tsx(2 hunks)src/pages/PageWrapper.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/pages/PageWrapper.tsx (5)
src/components/dashboard/NavBar.tsx (1)
NavBar(49-597)src/pages/MainPage.tsx (1)
MainPage(36-348)src/context/browserDimensions.tsx (1)
BrowserDimensionsProvider(19-54)src/pages/RecordingPage.tsx (1)
RecordingPage(29-180)src/components/dashboard/NotFound.tsx (1)
NotFoundPage(3-11)
src/pages/MainPage.tsx (1)
src/components/dashboard/MainMenu.tsx (1)
MainMenu(16-186)
🔇 Additional comments (2)
src/components/robot/pages/RobotConfigPage.tsx (1)
62-71: Good responsive container improvements.The changes to make the container responsive are well-executed:
width: '100%'allows flexibilitymaxWidth: 1000maintains readability on large screensheight: 'auto'withminHeightprovides flexible sizingboxSizing: 'border-box'ensures predictable dimension calculationsThese changes align well with the PR's objective of creating responsive Box-based layouts.
src/components/proxy/ProxyForm.tsx (1)
272-293: Good separation of form and instructions.Moving the instructions to a dedicated right column improves the layout clarity and user experience. The use of a fixed
minHeightensures the alert maintains visual weight even with varying content.
src/pages/PageWrapper.tsx
Outdated
| {/* Sticky NavBar - only show on non-recording pages */} | ||
| {location.pathname !== '/recording' && ( | ||
| <Box sx={{ | ||
| position: 'sticky', | ||
| top: 0, | ||
| zIndex: 1100, | ||
| backgroundColor: 'background.paper' | ||
| }}> | ||
| <NavBar recordingName={recordingName} isRecording={false} /> | ||
| </Box> | ||
| )} |
There was a problem hiding this comment.
Missing useLocation hook call - this will cause a runtime error.
The code references location.pathname on line 95, but the useLocation hook is never called to obtain the location object. Although useLocation is imported on line 13, it must be invoked to get the location object.
Add the hook call at the beginning of the PageWrapper component:
export const PageWrapper = () => {
const [open, setOpen] = useState(false);
const [isRecordingMode, setIsRecordingMode] = useState(false);
+ const location = useLocation();
const navigate = useNavigate();Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/pages/PageWrapper.tsx around lines 94 to 104, the JSX uses
location.pathname but the useLocation hook is never invoked; call const location
= useLocation() at the top of the PageWrapper component (after imports and
inside the component body) so location is defined before rendering the
conditional NavBar, ensuring the existing import is used and no runtime error
occurs.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/robot/pages/RobotConfigPage.tsx (1)
142-166: Consider consistent styling for secondary actions.The "Back to Selection" button (lines 148-150) uses explicit pink styling, while the Cancel button (line 163) now uses neutral
inheritstyling. For better visual consistency, secondary actions should typically share the same styling treatment, with only the primary action (Save) being prominent.Consider applying neutral styling to the "Back to Selection" button as well:
<Button variant="outlined" onClick={onBackToSelection} disabled={isLoading} - sx={{ - color: '#ff00c3 !important', - borderColor: '#ff00c3 !important', - backgroundColor: 'white !important', - }} > + > {backToSelectionText || t("buttons.back_arrow")} </Button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
public/locales/en.json(1 hunks)src/components/api/ApiKey.tsx(4 hunks)src/components/dashboard/MainMenu.tsx(1 hunks)src/components/robot/pages/RobotConfigPage.tsx(4 hunks)src/components/robot/pages/ScheduleSettingsPage.tsx(9 hunks)src/pages/MainPage.tsx(2 hunks)src/pages/PageWrapper.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/pages/MainPage.tsx
- src/components/api/ApiKey.tsx
- src/pages/PageWrapper.tsx
🔇 Additional comments (5)
src/components/dashboard/MainMenu.tsx (1)
75-75: LGTM! Width adjustment aligns with the layout revamp.The width reduction from 250px to 230px complements the sticky sidebar layout introduced in the broader PR changes.
public/locales/en.json (1)
583-583: Good improvement to user-facing text."Extracting data..." is more specific and descriptive than "Loading data...", providing clearer feedback to users about what's happening.
src/components/robot/pages/ScheduleSettingsPage.tsx (2)
190-191: LGTM! Cleaner spacing approach.Using
gap: 3instead of individual margins provides consistent spacing and is more maintainable. The explicitwidth: "100%"ensures proper container sizing.
233-346: Excellent consistency improvements.The standardized label styling (
width: "200px", flexShrink: 0) across all form rows ensures consistent visual alignment. The use of nested Box components withgapfor grouping related inputs (like the time range fields) is a clean, modern approach that improves readability and maintainability.src/components/robot/pages/RobotConfigPage.tsx (1)
67-69: LGTM! Layout changes support flexible, responsive behavior.The addition of
width: '100%',height: 'auto', andboxSizing: 'border-box'appropriately supports the flexible layout needed for the sticky navigation implementation.
| mt: 1.8, | ||
| mb: 5, |
There was a problem hiding this comment.
Consider using standard MUI spacing values.
The mt: 1.8 value is unusual and deviates from typical Material-UI spacing patterns (which use whole numbers or standard fractions like 0.5, 1.5, 2, etc.). Consider using mt: 2 or mt: 1.5 for consistency with MUI design tokens.
- mt: 1.8,
+ mt: 2,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mt: 1.8, | |
| mb: 5, | |
| mt: 2, | |
| mb: 5, |
🤖 Prompt for AI Agents
In src/components/robot/pages/RobotConfigPage.tsx around lines 124 to 125, the
styling uses a nonstandard mt: 1.8 which deviates from Material-UI spacing
tokens; change it to a standard MUI spacing value (for example mt: 1.5 or mt: 2)
to align with design tokens and consistency across the app.
This change makes the navigation bar and side menu sticky, so that they are persistent on the screen, and only the internal content scrolls.
Summary by CodeRabbit
Style & UI Improvements
Localization