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

Viewer improvements #2909

Merged
merged 16 commits into from
Jan 3, 2024
Merged
10 changes: 10 additions & 0 deletions packages/components/.storybook/main.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { StorybookConfig } from "@storybook/react-vite";
import { mergeConfig } from "vite";

const config: StorybookConfig = {
stories: [
Expand Down Expand Up @@ -37,6 +38,15 @@ const config: StorybookConfig = {
docs: {
autodocs: true,
},
async viteFinal(config) {
// Merge custom configuration into the default config
return mergeConfig(config, {
// https://github.com/storybookjs/storybook/issues/22223
build: {
target: "esnext",
},
});
},
};

export default config;
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { useReactiveExtension } from "./codemirrorHooks.js";

export function useSubmitExtension(
view: EditorView | undefined,
onSubmit: (() => void) | undefined
onSubmit?: () => void
) {
return useReactiveExtension(
view,
Expand Down
81 changes: 0 additions & 81 deletions packages/components/src/components/DynamicSquiggleViewer.tsx

This file was deleted.

50 changes: 37 additions & 13 deletions packages/components/src/components/SquiggleChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,22 @@ import { FC, memo } from "react";
import { SqValuePath } from "@quri/squiggle-lang";
import { RefreshIcon } from "@quri/ui";

import { SquiggleViewer } from "../index.js";
import { useRunnerState } from "../lib/hooks/useRunnerState.js";
import {
ProjectExecutionProps,
StandaloneExecutionProps,
useSquiggle,
} from "../lib/hooks/useSquiggle.js";
import { DynamicSquiggleViewer } from "./DynamicSquiggleViewer.js";
import { getResultValue, getResultVariables } from "../lib/utility.js";
import { MessageAlert } from "./Alert.js";
import { PartialPlaygroundSettings } from "./PlaygroundSettings.js";
import { SquiggleErrorAlert } from "./SquiggleErrorAlert.js";
import { SquiggleOutputViewer } from "./SquiggleOutputViewer/index.js";
import { useGetSubvalueByPath } from "./SquiggleViewer/utils.js";

export type SquiggleChartProps = {
code: string;
showHeader?: boolean;
rootPathOverride?: SqValuePath; // Note: This should be static. We don't support rootPathOverride to change once set.
} & (StandaloneExecutionProps | ProjectExecutionProps) &
// `environment` is passed through StandaloneExecutionProps; this way we guarantee that it's not compatible with `project` prop
Expand All @@ -23,7 +27,6 @@ export type SquiggleChartProps = {
export const SquiggleChart: FC<SquiggleChartProps> = memo(
function SquiggleChart({
code,
showHeader = false,
project,
continues,
environment,
Expand All @@ -41,19 +44,40 @@ export const SquiggleChart: FC<SquiggleChartProps> = memo(
...(project ? { project, continues } : { environment }),
});

// TODO - if `<ViewerProvider>` is not set up (which is very possible) then calculator paths won't be resolved.
const getSubvalueByPath = useGetSubvalueByPath();

if (!squiggleOutput) {
return <RefreshIcon className="animate-spin" />;
}

return (
<DynamicSquiggleViewer
rootPathOverride={rootPathOverride}
squiggleOutput={squiggleOutput}
isRunning={isRunning}
showHeader={showHeader}
environment={environment}
{...settings}
/>
);
if (rootPathOverride) {
const rootValue =
rootPathOverride.root === "result"
? getResultValue(squiggleOutput)
: getResultVariables(squiggleOutput);
if (!rootValue) {
return <MessageAlert heading="Value is not defined" />;
}
if (!rootValue.ok) {
return <SquiggleErrorAlert error={rootValue.value} />;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this SquiggleErrorAlert, but the others MessageAlert?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name is the same as it was before, but I think it's good:

  • this component takes SqError, so it's squiggle-specific
  • unlike <MessageAlert>, this component is public, and other components have Squiggle prefix

}

const value = getSubvalueByPath(rootValue.value, rootPathOverride);
if (!value) {
return <MessageAlert heading="Value is not defined" />;
}

return <SquiggleViewer value={value} />;
} else {
return (
<SquiggleOutputViewer
squiggleOutput={squiggleOutput}
isRunning={isRunning}
environment={environment}
{...settings}
/>
);
}
}
);
4 changes: 2 additions & 2 deletions packages/components/src/components/SquiggleEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import { useRunnerState } from "../lib/hooks/useRunnerState.js";
import { useSquiggle } from "../lib/hooks/useSquiggle.js";
import { getErrors } from "../lib/utility.js";
import { CodeEditor, CodeEditorHandle } from "./CodeEditor/index.js";
import { DynamicSquiggleViewer } from "./DynamicSquiggleViewer.js";
import { PartialPlaygroundSettings } from "./PlaygroundSettings.js";
import { SquiggleOutputViewer } from "./SquiggleOutputViewer/index.js";
import { SquiggleCodeProps } from "./types.js";

export type SquiggleEditorProps = SquiggleCodeProps & {
Expand Down Expand Up @@ -62,7 +62,7 @@ export const SquiggleEditor: FC<SquiggleEditorProps> = ({
/>
</div>
{hideViewer || !squiggleOutput?.code ? null : (
<DynamicSquiggleViewer
<SquiggleOutputViewer
squiggleOutput={squiggleOutput}
isRunning={isRunning}
editor={editorRef.current ?? undefined}
Expand Down
5 changes: 3 additions & 2 deletions packages/components/src/components/SquiggleErrorAlert.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { clsx } from "clsx";
import { FC, PropsWithChildren } from "react";

import {
Expand Down Expand Up @@ -25,8 +26,8 @@ const LocationLine: FC<{

return (
<span
className="cursor-pointer hover:underline text-blue-500"
onClick={findInEditor}
className={clsx(editor && "cursor-pointer hover:underline text-blue-500")}
onClick={editor ? findInEditor : undefined}
>
line {location.start.line}, column {location.start.column}
</span>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { FC, ReactNode } from "react";
berekuk marked this conversation as resolved.
Show resolved Hide resolved

export const Layout: FC<{
viewer: ReactNode;
menu: ReactNode;
indicator: ReactNode;
}> = ({ viewer, menu, indicator }) => {
return (
// `flex flex-col` helps to fit this in playground right panel and doesn't hurt otherwise
<div className="flex flex-col overflow-y-auto">
<div className="flex justify-between items-center px-2 h-8 mb-1">
{menu}
{indicator}
</div>
<div
className="flex-1 overflow-auto px-2 pb-1"
data-testid="dynamic-viewer-result"
>
{viewer}
</div>
</div>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { FC } from "react";

import { SquiggleOutput } from "../../lib/hooks/useSquiggle.js";

export const RenderingIndicator: FC<{
output: SquiggleOutput;
isRunning: boolean;
}> = ({ output, isRunning }) => {
const showTime = (executionTime: number) =>
executionTime > 1000
? `${(executionTime / 1000).toFixed(2)}s`
: `${executionTime}ms`;

return (
<div className="text-zinc-400 text-sm whitespace-nowrap">
{isRunning
? "rendering..."
: `render #${output.executionId} in ${showTime(output.executionTime)}`}
</div>
);
};