-
Notifications
You must be signed in to change notification settings - Fork 59
feat(webui/header): move tabs to right & add breadcrumb navigation #314
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## unstable #314 +/- ##
============================================
+ Coverage 43.38% 46.84% +3.45%
============================================
Files 37 45 +8
Lines 2971 4391 +1420
============================================
+ Hits 1289 2057 +768
- Misses 1544 2128 +584
- Partials 138 206 +68
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR redesigns the header by moving the tabs to the right and adding breadcrumb navigation for improved clarity and alignment. Key changes include:
- Updating sidebar components by removing the shadow effect.
- Enhancing the NavLinks button styling with a new scrolled prop.
- Adding a new Breadcrumb component along with banner and layout updates.
- Minor adjustments in footer image styling and global CSS transitions.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
webui/src/app/ui/sidebar.tsx | Removed shadow classes from Paper components in sidebar |
webui/src/app/ui/nav-links.tsx | Revised inline styling for button with a new scrolled prop |
webui/src/app/ui/footer.tsx | Updated footer image style for improved appearance |
webui/src/app/ui/breadcrumb.tsx | Added new breadcrumb component to show navigation context |
webui/src/app/ui/banner.tsx | Integrated scroll-based adjustments for header behavior |
webui/src/app/page.tsx | Modified Image component props for header logo rendering |
webui/src/app/layout.tsx | Integrated Breadcrumb component into the main layout |
webui/src/app/globals.css | Adjusted transition properties and dark mode styles |
sx={{ | ||
textTransform: "none", | ||
borderRadius: "20px", | ||
paddingLeft: scrolled ? 2 : 2.5, | ||
paddingRight: scrolled ? 2 : 2.5, | ||
paddingTop: scrolled ? 0.6 : 0.8, | ||
paddingBottom: scrolled ? 0.6 : 0.8, | ||
fontSize: scrolled ? "0.875rem" : "0.9rem", | ||
letterSpacing: "0.01em", | ||
fontWeight: 500, | ||
marginX: 0.5, | ||
transition: "all 0.3s ease", | ||
backgroundColor: isActive | ||
? (theme) => | ||
theme.palette.mode === "dark" | ||
? "rgba(255, 255, 255, 0.15)" | ||
: "rgba(25, 118, 210, 0.08)" | ||
: "transparent", | ||
color: (theme) => { | ||
if (theme.palette.mode === "dark") { | ||
return isActive ? "#fff" : "rgba(255, 255, 255, 0.9)"; | ||
} | ||
return isActive ? "#1976d2" : "rgba(0, 0, 0, 0.7)"; | ||
}, | ||
"&:hover": { | ||
backgroundColor: (theme) => | ||
theme.palette.mode === "dark" | ||
? "rgba(255, 255, 255, 0.2)" | ||
: "rgba(25, 118, 210, 0.12)", | ||
boxShadow: isActive | ||
? (theme) => | ||
theme.palette.mode === "dark" | ||
? "0 2px 8px rgba(0, 0, 0, 0.3)" | ||
: "0 2px 8px rgba(25, 118, 210, 0.2)" | ||
: "none", | ||
}, | ||
boxShadow: isActive | ||
? (theme) => | ||
theme.palette.mode === "dark" | ||
? "0 2px 5px rgba(0, 0, 0, 0.2)" | ||
: "0 2px 5px rgba(25, 118, 210, 0.15)" | ||
: "none", | ||
}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The inline style object for the Button is quite complex; consider extracting it into a separate variable or styled component to improve readability and maintainability.
sx={{ | |
textTransform: "none", | |
borderRadius: "20px", | |
paddingLeft: scrolled ? 2 : 2.5, | |
paddingRight: scrolled ? 2 : 2.5, | |
paddingTop: scrolled ? 0.6 : 0.8, | |
paddingBottom: scrolled ? 0.6 : 0.8, | |
fontSize: scrolled ? "0.875rem" : "0.9rem", | |
letterSpacing: "0.01em", | |
fontWeight: 500, | |
marginX: 0.5, | |
transition: "all 0.3s ease", | |
backgroundColor: isActive | |
? (theme) => | |
theme.palette.mode === "dark" | |
? "rgba(255, 255, 255, 0.15)" | |
: "rgba(25, 118, 210, 0.08)" | |
: "transparent", | |
color: (theme) => { | |
if (theme.palette.mode === "dark") { | |
return isActive ? "#fff" : "rgba(255, 255, 255, 0.9)"; | |
} | |
return isActive ? "#1976d2" : "rgba(0, 0, 0, 0.7)"; | |
}, | |
"&:hover": { | |
backgroundColor: (theme) => | |
theme.palette.mode === "dark" | |
? "rgba(255, 255, 255, 0.2)" | |
: "rgba(25, 118, 210, 0.12)", | |
boxShadow: isActive | |
? (theme) => | |
theme.palette.mode === "dark" | |
? "0 2px 8px rgba(0, 0, 0, 0.3)" | |
: "0 2px 8px rgba(25, 118, 210, 0.2)" | |
: "none", | |
}, | |
boxShadow: isActive | |
? (theme) => | |
theme.palette.mode === "dark" | |
? "0 2px 5px rgba(0, 0, 0, 0.2)" | |
: "0 2px 5px rgba(25, 118, 210, 0.15)" | |
: "none", | |
}} | |
sx={getButtonStyles(scrolled, isActive)} |
Copilot uses AI. Check for mistakes.
{breadcrumbItems.map((item, index) => ( | ||
<Box key={index} className="flex items-center"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using index as a key in a list render may lead to issues with component identity; consider using a unique identifier if available.
{breadcrumbItems.map((item, index) => ( | |
<Box key={index} className="flex items-center"> | |
{breadcrumbItems.map((item) => ( | |
<Box key={item.name} className="flex items-center"> |
Copilot uses AI. Check for mistakes.
@@ -65,80 +67,179 @@ export default function Banner() { | |||
document.getElementById("navbar")?.classList.add("navbar-dark-mode"); | |||
} | |||
|
|||
const handleScroll = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider debouncing the scroll event handler to reduce the frequency of state updates during fast scrolling, which could improve performance.
Copilot uses AI. Check for mistakes.
It closes #313.
I have redesigned the header-
Moved the tabs to the right side for better alignment and cleaner layout.
Added a breadcrumb below the header to display the current navigation path clearly and improve contextual awareness.
before

after
Monosnap.screencast.2025-06-02.15-29-26.mp4