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

Plot title additions #2337

Merged
merged 10 commits into from
Oct 17, 2023
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as React from "react";
import { FC, PropsWithChildren } from "react";
import * as d3 from "d3";

import { XIcon } from "@heroicons/react/solid/esm/index.js";
import {
Expand Down Expand Up @@ -30,6 +31,7 @@ type SummaryTableRowProps = {
name: string;
showName: boolean;
environment: Env;
tickFormat: string | undefined;
};

const percentiles = [0.05, 0.1, 0.25, 0.5, 0.75, 0.9, 0.95];
Expand All @@ -39,6 +41,7 @@ const SummaryTableRow: FC<SummaryTableRowProps> = ({
name,
showName,
environment,
tickFormat,
}) => {
const mean = distribution.mean(environment);
const stdev = distribution.stdev(environment);
Expand All @@ -47,11 +50,19 @@ const SummaryTableRow: FC<SummaryTableRowProps> = ({
distribution.inv(environment, percentile)
);

const formatNumber = (number: number) => {
if (tickFormat) {
return d3.format(tickFormat)(number);
} else {
return <NumberShower number={number} precision={3} />;
}
};

const unwrapResult = (
x: result<number, SqDistributionError>
): React.ReactNode => {
if (x.ok) {
return <NumberShower number={x.value} precision={3} />;
return formatNumber(x.value);
} else {
return (
<TextTooltip text={x.value.toString()}>
Expand All @@ -64,9 +75,7 @@ const SummaryTableRow: FC<SummaryTableRowProps> = ({
return (
<tr>
{showName && <Cell>{name}</Cell>}
<Cell>
<NumberShower number={mean} />
</Cell>
<Cell>{formatNumber(mean)}</Cell>
<Cell>{unwrapResult(stdev)}</Cell>
{percentileValues.map((value, i) => (
<Cell key={i}>{unwrapResult(value)}</Cell>
Expand All @@ -82,6 +91,7 @@ type SummaryTableProps = {

export const SummaryTable: FC<SummaryTableProps> = ({ plot, environment }) => {
const showNames = plot.distributions.some((d) => d.name);
const tickFormat = plot.xScale?.tickFormat;
return (
<table className="table border border-collapse border-slate-400">
<thead className="bg-slate-50">
Expand All @@ -102,6 +112,7 @@ export const SummaryTable: FC<SummaryTableProps> = ({ plot, environment }) => {
name={dist.name ?? dist.distribution.toString()}
showName={showNames}
environment={environment}
tickFormat={tickFormat}
/>
))}
</tbody>
Expand Down
23 changes: 6 additions & 17 deletions packages/components/src/components/DistributionsChart/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import { Point } from "../../lib/draw/types.js";
import { DrawContext } from "../../lib/hooks/useCanvas.js";
import { sqScaleToD3 } from "../../lib/d3/index.js";
import { adjustPdfHeightToScale } from "./utils.js";
import { PlotTitle } from "../../lib/plotTitle.js";

export type DistributionsChartProps = {
plot: SqDistributionsPlot;
Expand Down Expand Up @@ -73,14 +74,11 @@ const InnerDistributionsChart: FC<{
const legendItemHeight = 16;
const sampleBarHeight = 5;

const showTitle = !!plot.title;
const titleHeight = showTitle ? 20 : 4;
const legendHeight = isMulti ? legendItemHeight * shapes.length : 0;
const _showSamplesBar = showSamplesBar && samples.length;
const samplesFooterHeight = _showSamplesBar ? 10 : 0;

const height =
innerHeight + legendHeight + titleHeight + samplesFooterHeight + 30;
const height = innerHeight + legendHeight + samplesFooterHeight + 34;

const { xScale, yScale } = useMemo(() => {
const xScale = sqScaleToD3(plot.xScale);
Expand Down Expand Up @@ -117,31 +115,22 @@ const InnerDistributionsChart: FC<{
suggestedPadding: {
left: 10,
right: 10,
top: 10 + legendHeight + titleHeight,
top: 10 + legendHeight,
Copy link
Collaborator

Choose a reason for hiding this comment

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

There's a slight discrepancy (4px) with the old behavior.

Before: https://www.squiggle-language.com/playground?v=dev#code=eNqrVkpJTUsszSlxzk9JVbJSqlCwVQiuzNUrzctMyy%2FK1TDSMTTQVKoFAAjgDIc%3D

After: https://squiggle-website-git-plot-additions-quantified-uncertainty.vercel.app/playground?v=dev#code=eNqrVkpJTUsszSlxzk9JVbJSqlCwVQiuzNUrzctMyy%2FK1TDSMTTQVKoFAAjgDIc%3D

In both versions, canvas is 84px tall:

  • 20px bottom padding
  • 14px top padding previously (10 + empty titleHeight); 10px now

So previously chart height itself was exactly as was requested, 50px, but it's 54px now.

Of course this doesn't matter much, but to avoid future confusion about what's right, I'd suggest:

  • move suggestedPadding outside of draw function
  • then, calculate const height based on it, const height = innerHeight + suggestedPadding.top + suggestedPadding.bottom, then we'd be confident that they match and that innerHeight is correct

Copy link
Contributor Author

@OAGr OAGr Oct 16, 2023

Choose a reason for hiding this comment

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

This is a messy area, thanks for explaining!

For one thing, I imagine we'll later just want the labels to be in HTML, which would change the constraints here.

We'd also want it underneath the samples toolbar, as discussed in my PR description.

bottom: 20 + samplesFooterHeight,
},
xScale,
yScale,
hideYAxis: true,
drawTicks: true,
xTickFormat: plot.xScale.tickFormat,
xAxisTitle: plot.xScale.title,
});

if (plot.title) {
context.save();
context.textAlign = "center";
context.textBaseline = "top";
context.fillStyle = "black";
context.font = "bold 12px sans-serif";
context.fillText(plot.title, width / 2, 4);
context.restore();
}

if (isMulti) {
const radius = 5;
for (let i = 0; i < shapes.length; i++) {
context.save();
context.translate(padding.left, titleHeight + legendItemHeight * i);
context.translate(padding.left, legendItemHeight * i);
context.fillStyle = getColor(i);
drawCircle({
context,
Expand Down Expand Up @@ -266,7 +255,6 @@ const InnerDistributionsChart: FC<{
[
height,
legendHeight,
titleHeight,
samplesFooterHeight,
shapes,
samples,
Expand Down Expand Up @@ -407,6 +395,7 @@ export const DistributionsChart: FC<DistributionsChartProps> = ({

return (
<div className="flex flex-col items-stretch">
{plot.title && <PlotTitle title={plot.title} />}
{plot.xScale.tag === "log" && shapes.value.some(hasMassBelowZero) ? (
<ErrorAlert heading="Log Domain Error">
Cannot graph distribution with negative values on logarithmic scale.
Expand Down
19 changes: 11 additions & 8 deletions packages/components/src/components/DynamicSquiggleViewer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { getResultVariables, getResultValue } from "../lib/utility.js";
import { CodeEditorHandle } from "./CodeEditor.js";
import { PartialPlaygroundSettings } from "./PlaygroundSettings.js";
import { SquiggleViewerHandle } from "./SquiggleViewer/index.js";
import { ErrorBoundary } from "./ErrorBoundary.js";

type Props = {
squiggleOutput: SquiggleOutput | undefined;
Expand Down Expand Up @@ -34,14 +35,16 @@ export const DynamicSquiggleViewer = forwardRef<SquiggleViewerHandle, Props>(
// `opacity-0 squiggle-semi-appear` would be better, but won't work reliably until we move Squiggle evaluation to Web Workers
<div className="absolute z-10 inset-0 bg-white opacity-50" />
)}
<SquiggleViewer
{...settings}
ref={viewerRef}
localSettingsEnabled={localSettingsEnabled}
resultVariables={getResultVariables(squiggleOutput)}
resultItem={getResultValue(squiggleOutput)}
editor={editor}
/>
<ErrorBoundary>
<SquiggleViewer
{...settings}
ref={viewerRef}
localSettingsEnabled={localSettingsEnabled}
resultVariables={getResultVariables(squiggleOutput)}
resultItem={getResultValue(squiggleOutput)}
editor={editor}
/>
</ErrorBoundary>
</div>
) : null;

Expand Down
32 changes: 32 additions & 0 deletions packages/components/src/components/ErrorBoundary.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
"use client";

import { Component, PropsWithChildren } from "react";

type State = {
error?: Error;
};

export class ErrorBoundary extends Component<PropsWithChildren, State> {
public state: State = {};

public static getDerivedStateFromError(error: Error): State {
return { error };
}

componentDidCatch() {}

public render() {
if (this.state.error) {
return (
<div className="m-2 p-4 bg-red-300 rounded">
<header className="mb-2 font-semibold">Fatal Error</header>
<div className="mb-2">{this.state.error.message}</div>
<div className="mb-2">Try reloading the browser.</div>
<pre className="text-xs overflow-auto">{this.state.error.stack}</pre>
</div>
);
}

return this.props.children;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { DistributionsChart } from "../DistributionsChart/index.js";
import { ImageErrors } from "./ImageErrors.js";
import { getFunctionImage } from "./utils.js";
import { TailwindContext } from "@quri/ui";
import { PlotTitle } from "../../lib/plotTitle.js";

type FunctionChart1DistProps = {
plot: SqDistFnPlot;
Expand Down Expand Up @@ -150,6 +151,9 @@ function useDrawDistFunctionChart({
height,
context,
xTickFormat: plot.xScale.tickFormat,
yTickFormat: plot.yScale.tickFormat,
xAxisTitle: plot.xScale.title,
yAxisTitle: plot.yScale.title,
});
d3ref.current = { frame, xScale };

Expand Down Expand Up @@ -303,6 +307,7 @@ export const DistFunctionChart: FC<FunctionChart1DistProps> = ({

return (
<div className="flex flex-col items-stretch">
{plot.title && <PlotTitle title={plot.title} />}
<div ref={refs.setReference}>
<canvas ref={canvasRef} className={canvasClasses}>
Chart for {plot.toString()}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
import { canvasClasses } from "../../lib/utility.js";
import { ImageErrors } from "./ImageErrors.js";
import { getFunctionImage } from "./utils.js";
import { PlotTitle } from "../../lib/plotTitle.js";

type Props = {
plot: SqNumericFnPlot;
Expand Down Expand Up @@ -60,6 +61,8 @@ export const NumericFunctionChart: FC<Props> = ({
yScale,
xTickFormat: plot.xScale.tickFormat,
yTickFormat: plot.yScale.tickFormat,
xAxisTitle: plot.xScale.title,
yAxisTitle: plot.yScale.title,
});

if (
Expand Down Expand Up @@ -119,6 +122,7 @@ export const NumericFunctionChart: FC<Props> = ({

return (
<div className="flex flex-col items-stretch">
{plot.title && <PlotTitle title={plot.title} />}
<canvas ref={ref} className={canvasClasses}>
Chart for {plot.toString()}
</canvas>
Expand Down
13 changes: 8 additions & 5 deletions packages/components/src/components/FunctionChart/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
import { SquiggleErrorAlert } from "../SquiggleErrorAlert.js";
import { DistFunctionChart } from "./DistFunctionChart.js";
import { NumericFunctionChart } from "./NumericFunctionChart.js";
import { ErrorBoundary } from "../ErrorBoundary.js";

type FunctionChartProps = {
fn: SqLambda;
Expand Down Expand Up @@ -113,11 +114,13 @@ export const FunctionChart: FC<FunctionChartProps> = ({
});

return (
<NumericFunctionChart
plot={plot}
environment={environment}
height={height}
/>
<ErrorBoundary>
<NumericFunctionChart
plot={plot}
environment={environment}
height={height}
/>
</ErrorBoundary>
);
}
default:
Expand Down
4 changes: 4 additions & 0 deletions packages/components/src/components/ScatterChart/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
import { canvasClasses } from "../../lib/utility.js";
import { ErrorAlert } from "../Alert.js";
import { sqScaleToD3 } from "../../lib/d3/index.js";
import { PlotTitle } from "../../lib/plotTitle.js";

type Props = {
plot: SqScatterPlot;
Expand Down Expand Up @@ -70,6 +71,8 @@ export const ScatterChart: FC<Props> = ({ plot, height, environment }) => {
yScale,
xTickFormat: plot.xScale?.tickFormat,
yTickFormat: plot.yScale?.tickFormat,
xAxisTitle: plot.xScale?.title,
yAxisTitle: plot.yScale?.title,
drawTicks: true,
});

Expand Down Expand Up @@ -106,6 +109,7 @@ export const ScatterChart: FC<Props> = ({ plot, height, environment }) => {

return (
<div className="flex flex-col items-stretch">
{plot.title && <PlotTitle title={plot.title} />}
<canvas ref={ref} className={canvasClasses}>
Chart for {plot.toString()}
</canvas>
Expand Down
71 changes: 37 additions & 34 deletions packages/components/src/components/SquiggleViewer/VariableBox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
pathToShortName,
} from "./utils.js";
import { useEffectRef } from "../../lib/hooks/useEffectRef.js";
import { ErrorBoundary } from "../ErrorBoundary.js";

type SettingsMenuParams = {
// Used to notify VariableBox that settings have changed, so that VariableBox could re-render itself.
Expand Down Expand Up @@ -234,40 +235,42 @@ export const VariableBox: FC<VariableBoxProps> = ({
);

return (
<div ref={saveRef}>
{(name !== undefined || isRoot) && (
<header
className={clsx(
"flex justify-between group",
isFocused ? "mb-2" : "hover:bg-stone-100 rounded-md"
)}
>
<div className="inline-flex items-center">
{!isFocused && triangleToggle()}
{headerName}
{!isFocused && headerPreview()}
{!isFocused && !isOpen && commentIcon()}
{!isRoot && editor && headerFindInEditorButton()}
</div>
<div className="inline-flex space-x-1">
{isOpen && headerString()}
{isOpen && headerSettingsButton()}
</div>
</header>
)}
{isOpen && (
<div className="flex w-full pt-1">
{!isFocused && isDictOrList && leftCollapseBorder()}
{!isFocused && !isDictOrList && !isRoot && (
<div className="flex w-4 min-w-[1rem]" /> // min-w-1rem = w-4
)}
<div className="grow">
{commentPosition === "top" && hasComment && showComment()}
{children(getAdjustedMergedSettings(path))}
{commentPosition === "bottom" && hasComment && showComment()}
<ErrorBoundary>
<div ref={saveRef}>
{(name !== undefined || isRoot) && (
<header
className={clsx(
"flex justify-between group",
isFocused ? "mb-2" : "hover:bg-stone-100 rounded-md"
)}
>
<div className="inline-flex items-center">
{!isFocused && triangleToggle()}
{headerName}
{!isFocused && headerPreview()}
{!isFocused && !isOpen && commentIcon()}
{!isRoot && editor && headerFindInEditorButton()}
</div>
<div className="inline-flex space-x-1">
{isOpen && headerString()}
{isOpen && headerSettingsButton()}
</div>
</header>
)}
{isOpen && (
<div className="flex w-full pt-1">
{!isFocused && isDictOrList && leftCollapseBorder()}
{!isFocused && !isDictOrList && !isRoot && (
<div className="flex w-4 min-w-[1rem]" /> // min-w-1rem = w-4
)}
<div className="grow">
{commentPosition === "top" && hasComment && showComment()}
{children(getAdjustedMergedSettings(path))}
{commentPosition === "bottom" && hasComment && showComment()}
</div>
</div>
</div>
)}
</div>
)}
</div>
</ErrorBoundary>
);
};