Skip to content
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

enh: make viz responsive, remove extra renders, handle err in main window #6629

Merged
merged 6 commits into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions front/components/assistant/conversation/AgentMessage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,12 @@ export function AgentMessage({
...event.message,
};
});
// Mark the last viz as complete if it is not already.
setVisualizations((v) =>
v.map((item, index) =>
index === v.length - 1 ? { ...item, complete: true } : item
)
);
break;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Spinner } from "@dust-tt/sparkle";
import { Button, Spinner } from "@dust-tt/sparkle";
import type {
CommandResultMap,
VisualizationRPCCommand,
Expand Down Expand Up @@ -38,14 +38,14 @@ const sendResponseToIframe = <T extends VisualizationRPCCommand>(
// Custom hook to encapsulate the logic for handling visualization messages.
function useVisualizationDataHandler({
visualization,
onRetry,
setContentHeight,
setIsErrored,
vizIframeRef,
workspaceId,
}: {
visualization: Visualization;
onRetry: () => void;
setContentHeight: (v: SetStateAction<number>) => void;
setIsErrored: (v: SetStateAction<boolean>) => void;
vizIframeRef: React.MutableRefObject<HTMLIFrameElement | null>;
workspaceId: string;
}) {
Expand Down Expand Up @@ -98,14 +98,14 @@ function useVisualizationDataHandler({

break;

case "retry":
onRetry();
break;

case "setContentHeight":
setContentHeight(data.params.height);
break;

case "setErrored":
setIsErrored(true);
break;

default:
assertNever(data);
}
Expand All @@ -117,8 +117,8 @@ function useVisualizationDataHandler({
visualization.identifier,
code,
getFileBlob,
onRetry,
setContentHeight,
setIsErrored,
vizIframeRef,
]);
}
Expand All @@ -130,45 +130,41 @@ export function VisualizationActionIframe({
}: {
owner: WorkspaceType;
visualization: Visualization;

onRetry: () => void;
}) {
const [contentHeight, setContentHeight] = useState(0);
const [iframeLoaded, setIframeLoaded] = useState(false);
const [showSpinner, setShowSpinner] = useState(true);
const [contentHeight, setContentHeight] = useState<number>(0);
const [isErrored, setIsErrored] = useState(false);
const [activeIndex, setActiveIndex] = useState(1);

const vizIframeRef = useRef<HTMLIFrameElement>(null);
const containerRef = useRef<HTMLDivElement>(null);
const codeRef = useRef<HTMLDivElement>(null);
const errorRef = useRef<HTMLDivElement>(null);

const workspaceId = owner.sId;

useVisualizationDataHandler({
visualization,
workspaceId,
onRetry,
setContentHeight,
setIsErrored,
vizIframeRef,
});

const { code, complete: codeFullyGenerated } = visualization;

const iframeLoaded = contentHeight > 0;
const showSpinner =
((!codeFullyGenerated && !code) || (codeFullyGenerated && !iframeLoaded)) &&
!isErrored;

useEffect(() => {
if (!codeFullyGenerated) {
// Display spinner over the code block while waiting for code generation.
setShowSpinner(!code);
setActiveIndex(0);
} else if (iframeLoaded) {
// Display iframe if code is generated and iframe has loaded.
setShowSpinner(false);
setActiveIndex(1);
} else {
// Show spinner while iframe is loading.
setShowSpinner(true);
setActiveIndex(1);
}
}, [codeFullyGenerated, code, iframeLoaded]);
}, [codeFullyGenerated, code]);

useEffect(() => {
if (!containerRef.current) {
Expand All @@ -182,9 +178,13 @@ export function VisualizationActionIframe({
? `${codeRef.current?.scrollHeight}px`
: "100%";
} else if (activeIndex === 1) {
containerRef.current.style.height = `${contentHeight}px`;
if (isErrored && errorRef.current) {
containerRef.current.style.height = `${errorRef.current.scrollHeight}px`;
} else if (!isErrored) {
containerRef.current.style.height = `${contentHeight}px`;
}
}
}, [activeIndex, contentHeight, codeFullyGenerated]);
}, [activeIndex, contentHeight, codeFullyGenerated, isErrored]);

return (
<div className="relative flex flex-col">
Expand All @@ -202,7 +202,9 @@ export function VisualizationActionIframe({
<div
className={classNames(
"transition-height relative w-full overflow-hidden duration-500 ease-in-out",
codeFullyGenerated ? "min-h-96" : ""
codeFullyGenerated && !isErrored ? "min-h-96" : "",
isErrored ? "h-full" : "",
activeIndex === 1 ? "max-h-[60vh]" : ""
)}
ref={containerRef}
>
Expand All @@ -219,21 +221,38 @@ export function VisualizationActionIframe({
/>
</div>
<div className="relative flex h-full w-full shrink-0 items-center justify-center">
{codeFullyGenerated && (
{codeFullyGenerated && !isErrored && (
<div
style={{ height: `${contentHeight}px` }}
style={{
height: !isErrored ? `${contentHeight}px` : "100%",
minHeight: !isErrored ? "96" : undefined,
}}
className={classNames("max-h-[60vh] w-full")}
>
<iframe
ref={vizIframeRef}
// Set a min height so iframe can display error.
className="h-full min-h-96 w-full"
className={classNames(
"h-full w-full",
!isErrored ? "min-h-96" : ""
)}
src={`${process.env.NEXT_PUBLIC_VIZ_URL}/content?identifier=${visualization.identifier}`}
sandbox="allow-scripts"
onLoad={() => setIframeLoaded(true)}
/>
</div>
)}
{isErrored && (
<div
className="flex h-full w-full flex-col items-center gap-4 py-8"
ref={errorRef}
>
<div className="text-sm text-element-800">
An error occured while rendering the visualization.
</div>
<div>
<Button label="Retry" onClick={onRetry} size="sm" />
</div>
</div>
)}
</div>
</div>
</div>
Expand Down
12 changes: 8 additions & 4 deletions front/lib/api/assistant/visualization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,15 @@ The generated component should not have any required props / parameters.



### Outermost div height and width
### Responsiveness

The component's outermost JSX tag should have a fixed height and width in pixels, set using the \`style\` prop, e.g. \`<div style={{ height: '600px', width: '600px' }}>...</div>\`.
The content should be responsive and should not have fixed widths or heights. The component should be able to adapt to different screen sizes.
The content should never overflow the viewport and should never have horizontal or vertical scrollbars.

The height and width should be set to a fixed value, not a percentage. This style should not use tailwind CSS or any type of custom class. There should be a few pixels of horizontal padding to ensure the content is fully visible by the user.
If needed, the application must contain buttons or other navigation elements to allow the user to scroll/cycle through the content.
Never use tailwind's specific values like \`h-[600px]\`.

Always add padding to the content (both horizontal and vertical) to make it look better and make sure the labels are fully visible.



Expand Down Expand Up @@ -173,7 +177,7 @@ if (file) {

- Base React is available to be imported. In order to use hooks, they have to be imported at the top of the script, e.g. \`import { useState } from "react"\`

- The recharts charting library is available to be imported, e.g. \`import { LineChart, XAxis, ... } from "recharts"\` & \`<LineChart ...><XAxis dataKey="name"> ...\`. Support for defaultProps will be removed from function components in a future major release. JavaScript default parameters should be used instead.
- The recharts charting library is available to be imported, e.g. \`import { LineChart, XAxis, ... } from "recharts"\` & \`<LineChart ...><XAxis dataKey="name"> ...\`.

- The papaparse library is available to be imported, e.g. \`import Papa from "papaparse"\` & \`const parsed = Papa.parse(fileContent, {header:true, skipEmptyLines: "greedy"});\`. The \`skipEmptyLines:"greedy"\` configuration should always be used.

Expand Down
39 changes: 15 additions & 24 deletions types/src/front/assistant/visualization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,6 @@ interface GetFileParams {
fileId: string;
}

interface RetryParams {
errorMessage: string;
}

interface SetContentHeightParams {
height: number;
}
Expand All @@ -24,8 +20,8 @@ interface SetContentHeightParams {
export type VisualizationRPCRequestMap = {
getFile: GetFileParams;
getCodeToExecute: null;
retry: RetryParams;
setContentHeight: SetContentHeightParams;
setErrored: void;
};

// Derive the command type from the keys of the request map
Expand All @@ -42,17 +38,17 @@ export type VisualizationRPCRequest = {
export const validCommands: VisualizationRPCCommand[] = [
"getFile",
"getCodeToExecute",
"retry",
"setContentHeight",
"setErrored",
];

// Command results.

export interface CommandResultMap {
getFile: { fileBlob: Blob | null };
getCodeToExecute: { code: string };
retry: void;
setContentHeight: void;
setErrored: void;
}

// TODO(@fontanierh): refactor all these guards to use io-ts instead of manual checks.
Expand Down Expand Up @@ -100,12 +96,12 @@ export function isGetCodeToExecuteRequest(
);
}

// Type guard for retry.
export function isRetryRequest(
// Type guard for setContentHeight.
export function isSetContentHeightRequest(
value: unknown
): value is VisualizationRPCRequest & {
command: "retry";
params: RetryParams;
command: "setContentHeight";
params: SetContentHeightParams;
} {
if (typeof value !== "object" || value === null) {
return false;
Expand All @@ -114,21 +110,19 @@ export function isRetryRequest(
const v = value as Partial<VisualizationRPCRequest>;

return (
v.command === "retry" &&
v.command === "setContentHeight" &&
typeof v.identifier === "string" &&
typeof v.messageUniqueId === "string" &&
typeof v.params === "object" &&
v.params !== null &&
typeof (v.params as RetryParams).errorMessage === "string"
typeof (v.params as SetContentHeightParams).height === "number"
);
}

// Type guard for setContentHeight.
export function isSetContentHeightRequest(
export function isSetErroredRequest(
value: unknown
): value is VisualizationRPCRequest & {
command: "setContentHeight";
params: SetContentHeightParams;
command: "setErrored";
} {
if (typeof value !== "object" || value === null) {
return false;
Expand All @@ -137,12 +131,9 @@ export function isSetContentHeightRequest(
const v = value as Partial<VisualizationRPCRequest>;

return (
v.command === "setContentHeight" &&
v.command === "setErrored" &&
typeof v.identifier === "string" &&
typeof v.messageUniqueId === "string" &&
typeof v.params === "object" &&
v.params !== null &&
typeof (v.params as SetContentHeightParams).height === "number"
typeof v.messageUniqueId === "string"
);
}

Expand All @@ -156,7 +147,7 @@ export function isVisualizationRPCRequest(
return (
isGetCodeToExecuteRequest(value) ||
isGetFileRequest(value) ||
isRetryRequest(value) ||
isSetContentHeightRequest(value)
isSetContentHeightRequest(value) ||
isSetErroredRequest(value)
);
}
49 changes: 0 additions & 49 deletions viz/app/components/Components.tsx
Original file line number Diff line number Diff line change
@@ -1,54 +1,5 @@
"use client";

// We can't use Sparkle components in the viz app,
// because of client-side rendering issue.
// So we define the components here.

export const Button = ({
label,
onClick,
}: {
label: string;
onClick: () => void;
}) => {
return (
<button
onClick={onClick}
className="px-4 py-2 text-sm font-medium text-white bg-blue-500 rounded hover:bg-blue-600"
>
{label}
</button>
);
};

export const ErrorMessage = ({
children,
title,
}: {
children: React.ReactNode;
title: string;
}) => {
return (
<div className="bg-pink-100 border-l-4 border-pink-500 rounded-lg p-4 max-w-md">
<div className="flex items-center mb-2">
<svg
className="w-6 h-6 text-pink-500 mr-2"
fill="currentColor"
viewBox="0 0 20 20"
>
<path
fillRule="evenodd"
d="M18 10a8 8 0 11-16 0 8 8 0 0116 0zm-7-4a1 1 0 11-2 0 1 1 0 012 0zM9 9a1 1 0 000 2v3a1 1 0 001 1h1a1 1 0 100-2v-3a1 1 0 00-1-1H9z"
clipRule="evenodd"
/>
</svg>
<h3 className="text-lg font-semibold text-pink-800">{title}</h3>
</div>
<div className="text-pink-700">{children}</div>
</div>
);
};

export const Spinner = () => {
return (
<div className="flex items-center justify-center">
Expand Down
Loading
Loading