-
Notifications
You must be signed in to change notification settings - Fork 10
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
Add Local Banner #618
Add Local Banner #618
Conversation
WalkthroughThe changes involve adjusting the layout and components in an authenticated layout to improve the user experience. This includes adding a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AuthenticatedLayout
participant LocalBanner
participant AuthenticatedHeader
participant Sidebar
participant ServerInfo
User->>AuthenticatedLayout: Load page
AuthenticatedLayout->>ServerInfo: Fetch deployment type
ServerInfo-->>AuthenticatedLayout: Return deployment type
AuthenticatedLayout->>LocalBanner: Render (if local deployment)
AuthenticatedLayout->>AuthenticatedHeader: Render
AuthenticatedLayout->>Sidebar: Render with `isLocal` styling
AuthenticatedLayout-->>User: Display page
Poem
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
❌ Deploy Preview for zenmlapp failed.
|
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.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
src/layouts/AuthenticatedLayout/Sidebar.tsx (1)
Line range hint
1-1
: UseuseEffect
to handle window resize events.To ensure
isMinWidth
updates when the window is resized, useuseEffect
to add an event listener for window resize events.import { useEffect, useState } from 'react'; // ... export function AuthenticatedLayout() { const { data } = useCurrentUser(); const [isMinWidth, setIsMinWidth] = useState(window.innerWidth >= 1440); useEffect(() => { const handleResize = () => setIsMinWidth(window.innerWidth >= 1440); window.addEventListener('resize', handleResize); return () => window.removeEventListener('resize', handleResize); }, []); // ... }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- src/layouts/AuthenticatedLayout/AuthenticatedHeader.tsx (1 hunks)
- src/layouts/AuthenticatedLayout/LocalBanner.tsx (1 hunks)
- src/layouts/AuthenticatedLayout/Sidebar.tsx (1 hunks)
- src/layouts/AuthenticatedLayout/index.tsx (2 hunks)
Files skipped from review due to trivial changes (1)
- src/layouts/AuthenticatedLayout/AuthenticatedHeader.tsx
Additional comments not posted (7)
src/layouts/AuthenticatedLayout/LocalBanner.tsx (4)
3-3
: Import statement improvement.Consider importing only the necessary functions or components from
@zenml-io/react-component-library
to reduce the bundle size.- import { Button } from "@zenml-io/react-component-library"; + import Button from "@zenml-io/react-component-library/Button";
8-8
: Improve readability of conditional return.Use a more readable conditional return statement.
- if (serverInfo.data?.deployment_type !== "local") return null; + if (serverInfo.data?.deployment_type !== "local") { + return null; + }
11-11
: Consider using a more descriptive class name.The class name
bg-warning-400
may not be descriptive enough. Consider using a class name that reflects the purpose or context of the banner.- <aside className="flex h-9 items-center justify-between gap-2 bg-warning-400 px-4 py-3"> + <aside className="flex h-9 items-center justify-between gap-2 bg-local-warning px-4 py-3">
24-24
: Consider using a more descriptive class name.The class name
bg-theme-surface-primary
may not be descriptive enough. Consider using a class name that reflects the purpose or context of the button.- className="whitespace-nowrap bg-theme-surface-primary" + className="whitespace-nowrap bg-local-banner-button"src/layouts/AuthenticatedLayout/index.tsx (1)
10-10
: Ensure import order consistency.Ensure that the import order is consistent with the project's style guide.
- import { LocalBanner } from "./LocalBanner"; + import { Sidebar } from "./Sidebar"; import { LocalBanner } from "./LocalBanner";src/layouts/AuthenticatedLayout/Sidebar.tsx (2)
26-26
: Import statement improvement.Consider importing only the necessary functions or components from
@zenml-io/react-component-library
to reduce the bundle size.- import { useServerInfo } from "../../data/server/info-query"; + import useServerInfo from "../../data/server/info-query";
35-35
: Ensure consistent spacing in class names.Ensure consistent spacing in class names to improve readability.
- className={`sticky ${isLocal ? "top-[128px] h-[calc(100vh_-_64px_-_64px)]" : "top-9 h-[calc(100vh_-_64px)]"} overflow-y-auto overflow-x-clip`} + className={`sticky ${isLocal ? "top-[128px] h-[calc(100vh_-_64px_-_64px)]" : "top-9 h-[calc(100vh_-_64px)]"} overflow-y-auto overflow-x-clip`}
Summary by CodeRabbit
New Features
LocalBanner
component that displays a message for local deployments in ZenML.Changes
sticky
andtop-0
classes.Refactor
AuthenticatedLayout
.