-
Notifications
You must be signed in to change notification settings - Fork 88
feat(web): replace library page stubs with real implementations #31277
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
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 | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,13 +1,112 @@ | ||||||||||||||||||||||||||||||||||||||||||
| import { useParams } from "react-router"; | ||||||||||||||||||||||||||||||||||||||||||
| import { Loader2 } from "lucide-react"; | ||||||||||||||||||||||||||||||||||||||||||
| import { useCallback, useEffect, useRef, useState } from "react"; | ||||||||||||||||||||||||||||||||||||||||||
| import { useNavigate, useParams } from "react-router"; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| import { toast } from "@vellum/design-library"; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| import { useAssistantContext } from "@/domains/chat/assistant-context.js"; | ||||||||||||||||||||||||||||||||||||||||||
| import { openApp, shareApp } from "@/domains/chat/lib/apps.js"; | ||||||||||||||||||||||||||||||||||||||||||
| import { AppViewerContainer } from "@/domains/intelligence/components/apps/app-viewer-container.js"; | ||||||||||||||||||||||||||||||||||||||||||
| import { routes } from "@/utils/routes.js"; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| interface LoadedApp { | ||||||||||||||||||||||||||||||||||||||||||
| appId: string; | ||||||||||||||||||||||||||||||||||||||||||
| dirName?: string; | ||||||||||||||||||||||||||||||||||||||||||
| name: string; | ||||||||||||||||||||||||||||||||||||||||||
| html: string; | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| export function LibraryDetailPage() { | ||||||||||||||||||||||||||||||||||||||||||
| const { appId } = useParams<{ appId: string }>(); | ||||||||||||||||||||||||||||||||||||||||||
| const { assistantId } = useAssistantContext(); | ||||||||||||||||||||||||||||||||||||||||||
| const navigate = useNavigate(); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| const [app, setApp] = useState<LoadedApp | null>(null); | ||||||||||||||||||||||||||||||||||||||||||
| const [error, setError] = useState<string | null>(null); | ||||||||||||||||||||||||||||||||||||||||||
| const [isSharing, setIsSharing] = useState(false); | ||||||||||||||||||||||||||||||||||||||||||
| const requestRef = useRef<string | null>(null); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||||||||||||||||||||||||
| if (!assistantId || !appId) return; | ||||||||||||||||||||||||||||||||||||||||||
| requestRef.current = appId; | ||||||||||||||||||||||||||||||||||||||||||
| setApp(null); | ||||||||||||||||||||||||||||||||||||||||||
| setError(null); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| openApp(assistantId, appId) | ||||||||||||||||||||||||||||||||||||||||||
| .then((result) => { | ||||||||||||||||||||||||||||||||||||||||||
| if (requestRef.current !== appId) return; | ||||||||||||||||||||||||||||||||||||||||||
| setApp({ | ||||||||||||||||||||||||||||||||||||||||||
| appId: result.appId, | ||||||||||||||||||||||||||||||||||||||||||
| dirName: result.dirName, | ||||||||||||||||||||||||||||||||||||||||||
| name: result.name, | ||||||||||||||||||||||||||||||||||||||||||
| html: result.html, | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||
| .catch((err) => { | ||||||||||||||||||||||||||||||||||||||||||
| if (requestRef.current !== appId) return; | ||||||||||||||||||||||||||||||||||||||||||
| setError(err instanceof Error ? err.message : "Failed to open app"); | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| return () => { | ||||||||||||||||||||||||||||||||||||||||||
| requestRef.current = null; | ||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||
| }, [assistantId, appId]); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| const handleClose = useCallback(() => { | ||||||||||||||||||||||||||||||||||||||||||
| void navigate(routes.library.root); | ||||||||||||||||||||||||||||||||||||||||||
| }, [navigate]); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| const handleShare = useCallback(async () => { | ||||||||||||||||||||||||||||||||||||||||||
| if (!assistantId || !app || isSharing) return; | ||||||||||||||||||||||||||||||||||||||||||
| setIsSharing(true); | ||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||
| await shareApp(assistantId, app.appId, app.name); | ||||||||||||||||||||||||||||||||||||||||||
| toast.success("App exported", { description: `${app.name}.vellum` }); | ||||||||||||||||||||||||||||||||||||||||||
| } catch (err) { | ||||||||||||||||||||||||||||||||||||||||||
| toast.error("Failed to share app", { | ||||||||||||||||||||||||||||||||||||||||||
| description: err instanceof Error ? err.message : undefined, | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
| } finally { | ||||||||||||||||||||||||||||||||||||||||||
| setIsSharing(false); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| }, [assistantId, app, isSharing]); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| if (!assistantId || !appId) return null; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| if (error) { | ||||||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||||||
| <div className="flex flex-1 flex-col items-center justify-center gap-4"> | ||||||||||||||||||||||||||||||||||||||||||
| <p className="text-body-medium-lighter text-[var(--content-tertiary)]"> | ||||||||||||||||||||||||||||||||||||||||||
| {error} | ||||||||||||||||||||||||||||||||||||||||||
| </p> | ||||||||||||||||||||||||||||||||||||||||||
| <button | ||||||||||||||||||||||||||||||||||||||||||
| type="button" | ||||||||||||||||||||||||||||||||||||||||||
| onClick={handleClose} | ||||||||||||||||||||||||||||||||||||||||||
| className="text-body-medium-default text-[var(--primary-base)] underline" | ||||||||||||||||||||||||||||||||||||||||||
| > | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+78
to
+86
Contributor
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. 🔴 Missing The Comparison with existing pattern in LibraryViewThe analogous handler in catch (err) {
toast.error("Failed to share app", {
description: err instanceof Error ? err.message : undefined,
});
}It also shows a success toast on the happy path. The
Suggested change
Was this helpful? React with 👍 or 👎 to provide feedback.
Contributor
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. Fixed in 552e005 — added |
||||||||||||||||||||||||||||||||||||||||||
| Back to Library | ||||||||||||||||||||||||||||||||||||||||||
| </button> | ||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| if (!app) { | ||||||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||||||
| <div className="flex flex-1 items-center justify-center"> | ||||||||||||||||||||||||||||||||||||||||||
| <Loader2 className="h-6 w-6 animate-spin text-[var(--content-tertiary)]" /> | ||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||||||
| <section> | ||||||||||||||||||||||||||||||||||||||||||
| <h2>Library item</h2> | ||||||||||||||||||||||||||||||||||||||||||
| <p> | ||||||||||||||||||||||||||||||||||||||||||
| Placeholder for library item <code>{appId}</code>. | ||||||||||||||||||||||||||||||||||||||||||
| </p> | ||||||||||||||||||||||||||||||||||||||||||
| </section> | ||||||||||||||||||||||||||||||||||||||||||
| <AppViewerContainer | ||||||||||||||||||||||||||||||||||||||||||
| appId={app.appId} | ||||||||||||||||||||||||||||||||||||||||||
| appName={app.name} | ||||||||||||||||||||||||||||||||||||||||||
| html={app.html} | ||||||||||||||||||||||||||||||||||||||||||
| assistantId={assistantId} | ||||||||||||||||||||||||||||||||||||||||||
| onClose={handleClose} | ||||||||||||||||||||||||||||||||||||||||||
| onShare={handleShare} | ||||||||||||||||||||||||||||||||||||||||||
| isSharing={isSharing} | ||||||||||||||||||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,35 @@ | ||
| import { useCallback } from "react"; | ||
| import { useNavigate } from "react-router"; | ||
|
|
||
| import { useAssistantContext } from "@/domains/chat/assistant-context.js"; | ||
| import { LibraryView } from "@/domains/intelligence/components/apps/library-view.js"; | ||
| import { routes } from "@/utils/routes.js"; | ||
|
|
||
| export function LibraryPage() { | ||
| const { assistantId } = useAssistantContext(); | ||
| const navigate = useNavigate(); | ||
|
|
||
| const handleNewConversation = useCallback( | ||
| (_initialMessage?: string) => { | ||
| // TODO: initialMessage seeding requires cross-route state coordination | ||
| // (e.g. a Zustand store or sessionStorage handoff). The platform passes | ||
| // initialMessage directly via startNewConversation() in the same React | ||
| // tree, but here the library is a separate route. For now we just | ||
| // navigate to chat; the deploy-flow prompt handoff will come with the | ||
| // broader cross-route state work. | ||
| void navigate(routes.assistant); | ||
| }, | ||
| [navigate], | ||
| ); | ||
|
|
||
| if (!assistantId) return null; | ||
|
|
||
| return ( | ||
| <section> | ||
| <h2>Library</h2> | ||
| <p>Placeholder route. Library listing lands with the platform/web code port.</p> | ||
| </section> | ||
| <div className="flex min-h-0 flex-1 flex-col overflow-hidden rounded-lg border border-[var(--border-base)] bg-[var(--surface-overlay)]"> | ||
| <LibraryView | ||
| assistantId={assistantId} | ||
| onNewConversation={handleNewConversation} | ||
| /> | ||
| </div> | ||
| ); | ||
| } |
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.
The share callback awaits
shareApp(...)without acatch, so any network/API error rejects out of the click handler. In this case users get no explicit error toast and the app can emit unhandled promise rejection noise in production logs; this is a regression from the existingLibraryViewshare flow, which catches and reports failures.Useful? React with 👍 / 👎.
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.
Fixed in 552e005 — added
catch+toast.error/toast.successto matchLibraryView's share pattern. Thetry/finallywithoutcatchwas a real bug (unhandled rejection + silent failure).