-
Notifications
You must be signed in to change notification settings - Fork 5
feat: Improve visual contrast between Pipelines and Runs #1610
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,195 @@ | ||
| import { useNavigate, useParams } from "@tanstack/react-router"; | ||
| import { Check, ChevronRight, Copy, Network } from "lucide-react"; | ||
| import { type MouseEvent, useCallback, useEffect, useState } from "react"; | ||
|
|
||
| import { | ||
| Breadcrumb, | ||
| BreadcrumbItem, | ||
| BreadcrumbLink, | ||
| BreadcrumbList, | ||
| BreadcrumbPage, | ||
| BreadcrumbSeparator, | ||
| } from "@/components/ui/breadcrumb"; | ||
| import { Button } from "@/components/ui/button"; | ||
| import { cn } from "@/lib/utils"; | ||
| import { useComponentSpec } from "@/providers/ComponentSpecProvider"; | ||
| import { useExecutionDataOptional } from "@/providers/ExecutionDataProvider"; | ||
| import { loadPipelineByName } from "@/services/pipelineService"; | ||
| import { copyToClipboard } from "@/utils/string"; | ||
|
|
||
| interface PipelineRunBreadcrumbsProps { | ||
| variant?: "overlay" | "topbar"; | ||
| } | ||
|
|
||
| export const PipelineRunBreadcrumbs = ({ | ||
| variant = "overlay", | ||
| }: PipelineRunBreadcrumbsProps) => { | ||
| const navigate = useNavigate(); | ||
| const params = useParams({ strict: false }); | ||
| const { componentSpec } = useComponentSpec(); | ||
| const executionData = useExecutionDataOptional(); | ||
|
|
||
| // Get run ID from execution data OR from URL params (fallback for when provider isn't loaded yet) | ||
| const runIdFromParams = | ||
| "id" in params && typeof params.id === "string" ? params.id : undefined; | ||
| const runId = executionData?.runId || runIdFromParams; | ||
| const metadata = executionData?.metadata; | ||
| const [isCopied, setIsCopied] = useState(false); | ||
| const [isHovered, setIsHovered] = useState(false); | ||
| const [pipelineExistsLocally, setPipelineExistsLocally] = useState< | ||
| boolean | null | ||
| >(null); | ||
|
|
||
| const pipelineName = componentSpec?.name || metadata?.pipeline_name; | ||
|
|
||
| // Check if the pipeline exists in local storage | ||
| useEffect(() => { | ||
| const checkPipelineExists = async () => { | ||
| if (!pipelineName) { | ||
| setPipelineExistsLocally(null); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| const result = await loadPipelineByName(pipelineName); | ||
| setPipelineExistsLocally(!!result.experiment); | ||
| } catch (error) { | ||
| console.error("Error checking pipeline existence:", error); | ||
| setPipelineExistsLocally(false); | ||
| } | ||
| }; | ||
|
|
||
| checkPipelineExists(); | ||
| }, [pipelineName]); | ||
|
|
||
| const handleNavigateToPipeline = useCallback(() => { | ||
| if (pipelineName && pipelineExistsLocally) { | ||
| navigate({ to: `/editor/${encodeURIComponent(pipelineName)}` }); | ||
| } | ||
| }, [pipelineName, pipelineExistsLocally, navigate]); | ||
|
|
||
| const handleCopyRunId = useCallback( | ||
| (e: MouseEvent) => { | ||
| e.stopPropagation(); | ||
| if (runId) { | ||
| copyToClipboard(runId); | ||
| setIsCopied(true); | ||
| } | ||
| }, | ||
| [runId], | ||
| ); | ||
|
|
||
| useEffect(() => { | ||
| if (isCopied) { | ||
| const timer = setTimeout(() => { | ||
| setIsCopied(false); | ||
| }, 1500); | ||
| return () => clearTimeout(timer); | ||
| } | ||
| }, [isCopied]); | ||
|
|
||
| if (!pipelineName) { | ||
| return null; | ||
| } | ||
|
|
||
| // Styles for topbar variant (white text on dark background) | ||
| const isTopbar = variant === "topbar"; | ||
| const textColorClass = isTopbar ? "text-white" : "text-foreground"; | ||
| const mutedTextClass = isTopbar ? "text-gray-300" : "text-muted-foreground"; | ||
| const buttonVariantClass = isTopbar | ||
| ? "h-7 px-2 gap-1.5 font-medium text-gray-300 hover:text-white hover:bg-stone-800" | ||
| : "h-7 px-2 gap-1.5 text-muted-foreground hover:text-foreground font-medium"; | ||
|
|
||
| // Container for overlay variant | ||
| const containerClass = isTopbar | ||
| ? "flex items-center gap-1" | ||
| : "absolute top-0 left-0 z-10 bg-white/95 backdrop-blur-sm shadow-md rounded-br-xl border-b border-r border-gray-200"; | ||
|
|
||
| return ( | ||
| <div className={containerClass}> | ||
| <div className={isTopbar ? "" : "px-4 py-2.5"}> | ||
| <Breadcrumb> | ||
| <BreadcrumbList> | ||
| <BreadcrumbItem> | ||
| {pipelineExistsLocally ? ( | ||
| <BreadcrumbLink asChild> | ||
| <Button | ||
| variant="ghost" | ||
| size="sm" | ||
| onClick={handleNavigateToPipeline} | ||
| title="Navigate to pipeline in editor" | ||
| className={buttonVariantClass} | ||
| > | ||
| <Network className="w-4 h-4 rotate-270" /> | ||
| {pipelineName} | ||
| </Button> | ||
| </BreadcrumbLink> | ||
| ) : ( | ||
| <BreadcrumbPage | ||
| title={ | ||
| pipelineExistsLocally === null | ||
| ? "Checking if pipeline exists locally..." | ||
| : "Pipeline not saved locally. Use 'Clone Pipeline' to edit." | ||
| } | ||
| className={cn( | ||
| "flex items-center gap-1.5 h-7 px-2", | ||
| mutedTextClass, | ||
| )} | ||
| > | ||
| <Network className="w-4 h-4 rotate-270" /> | ||
| {pipelineName} | ||
| </BreadcrumbPage> | ||
| )} | ||
| </BreadcrumbItem> | ||
| <BreadcrumbSeparator> | ||
| <ChevronRight className={cn("w-4 h-4", mutedTextClass)} /> | ||
| </BreadcrumbSeparator> | ||
| <BreadcrumbItem> | ||
| {runId ? ( | ||
| <div | ||
| className="group flex items-center gap-1 cursor-pointer" | ||
| onClick={handleCopyRunId} | ||
| onMouseEnter={() => setIsHovered(true)} | ||
| onMouseLeave={() => setIsHovered(false)} | ||
| title={`Click to copy: ${runId}`} | ||
| > | ||
| <span | ||
| className={cn( | ||
| "text-sm font-medium transition-colors duration-150", | ||
| isCopied ? "text-emerald-500" : textColorClass, | ||
| )} | ||
| > | ||
| Run {runId} | ||
| </span> | ||
| <span className="relative h-3.5 w-3.5"> | ||
| <Check | ||
| className={cn( | ||
| "absolute inset-0 h-3.5 w-3.5 text-emerald-500 transition-all duration-200", | ||
| isCopied | ||
| ? "rotate-0 scale-100 opacity-100" | ||
| : "-rotate-90 scale-0 opacity-0", | ||
| )} | ||
| /> | ||
| <Copy | ||
| className={cn( | ||
| "absolute inset-0 h-3.5 w-3.5 transition-all duration-200", | ||
| mutedTextClass, | ||
| isHovered && !isCopied | ||
| ? "rotate-0 scale-100 opacity-100" | ||
| : "rotate-90 scale-0 opacity-0", | ||
| )} | ||
| /> | ||
| </span> | ||
| </div> | ||
| ) : ( | ||
| <span className={cn("text-sm font-medium", textColorClass)}> | ||
| Run | ||
| </span> | ||
| )} | ||
| </BreadcrumbItem> | ||
|
Comment on lines
+144
to
+189
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code could be simplified to this (and remove the copy logic above) |
||
| </BreadcrumbList> | ||
| </Breadcrumb> | ||
| </div> | ||
| </div> | ||
| ); | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,9 @@ | ||
| import { useLocation } from "@tanstack/react-router"; | ||
| import { Menu } from "lucide-react"; | ||
| import { useState } from "react"; | ||
|
|
||
| import logo from "/Tangle_white.png"; | ||
| import { PipelineRunBreadcrumbs } from "@/components/PipelineRun/components/PipelineRunBreadcrumbs"; | ||
| import { isAuthorizationRequired } from "@/components/shared/Authentication/helpers"; | ||
| import { TopBarAuthentication } from "@/components/shared/Authentication/TopBarAuthentication"; | ||
| import { CopyText } from "@/components/shared/CopyText/CopyText"; | ||
|
|
@@ -17,6 +19,7 @@ import { | |
| SheetTrigger, | ||
| } from "@/components/ui/sheet"; | ||
| import { useComponentSpec } from "@/providers/ComponentSpecProvider"; | ||
| import { RUNS_BASE_PATH } from "@/routes/router"; | ||
| import { TOP_NAV_HEIGHT } from "@/utils/constants"; | ||
|
|
||
| import BackendStatus from "../shared/BackendStatus"; | ||
|
|
@@ -27,9 +30,13 @@ import { PersonalPreferences } from "../shared/Settings/PersonalPreferences"; | |
| const AppMenu = () => { | ||
| const requiresAuthorization = isAuthorizationRequired(); | ||
| const { componentSpec } = useComponentSpec(); | ||
| const location = useLocation(); | ||
| const title = componentSpec?.name; | ||
| const [mobileMenuOpen, setMobileMenuOpen] = useState(false); | ||
|
|
||
| // Check if we're on a run page | ||
| const isRunPage = location.pathname.includes(RUNS_BASE_PATH); | ||
|
|
||
| return ( | ||
| <div | ||
| className="w-full bg-stone-900 px-3 py-2.5 md:px-4" | ||
|
|
@@ -45,10 +52,15 @@ const AppMenu = () => { | |
| /> | ||
| </Link> | ||
|
|
||
| {title && ( | ||
| <CopyText className="text-white text-md font-bold truncate max-w-32 sm:max-w-48 md:max-w-64 lg:max-w-md"> | ||
| {title} | ||
| </CopyText> | ||
| {/* Show breadcrumbs on run pages, otherwise show simple title */} | ||
| {isRunPage ? ( | ||
| <PipelineRunBreadcrumbs variant="topbar" /> | ||
| ) : ( | ||
| title && ( | ||
| <CopyText className="text-white text-md font-bold truncate max-w-32 sm:max-w-48 md:max-w-64 lg:max-w-md"> | ||
| {title} | ||
| </CopyText> | ||
| ) | ||
|
Comment on lines
+55
to
+63
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we just remove this logic and use seems to work and the logic is all in one place. Thoughts? |
||
| )} | ||
| </InlineStack> | ||
|
|
||
|
|
||
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.
We have a component
CopyText.tsxthat should be able to work for this .