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

Components: Hide small strings, other short stories #2547

Merged
merged 6 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fifty-schools-applaud.md
@@ -0,0 +1,5 @@
---
"@quri/squiggle-components": patch
---

Hide calculator top when empty, auto-close widgets with strings of <25 chars, expose errors when items are focused.
22 changes: 12 additions & 10 deletions packages/components/src/components/Calculator/index.tsx
Expand Up @@ -253,16 +253,18 @@ export const Calculator: FC<Props> = ({
</div>
)}

<div className="py-3 px-5 border-b border-slate-200 bg-gray-50 space-y-3">
{calculator.inputs.map((row) => (
<CalculatorInput
key={row.name}
input={row}
result={inputResults[row.name]}
settings={inputResultSettings}
/>
))}
</div>
{Boolean(calculator.inputs.length) && (
<div className="py-3 px-5 border-b border-slate-200 bg-gray-50 space-y-3">
{calculator.inputs.map((row) => (
<CalculatorInput
key={row.name}
input={row}
result={inputResults[row.name]}
settings={inputResultSettings}
/>
))}
</div>
)}

<CalculatorResult
valueWithContext={valueWithContext}
Expand Down
Expand Up @@ -23,6 +23,7 @@ import {
} from "./ViewerProvider.js";
import { getSqValueWidget } from "./getSqValueWidget.js";
import { getChildrenValues, pathToShortName } from "./utils.js";
import { SHORT_STRING_LENGTH } from "../../lib/constants.js";

export type SettingsMenuParams = {
// Used to notify this component that settings have changed, so that it could re-render itself.
Expand Down Expand Up @@ -114,6 +115,8 @@ const ValueViewerBody: FC<Props> = ({ value }) => {
);
};

const tagsDefaultCollapsed = new Set(["Bool", "Number", "Void", "Input"]);

export const ValueWithContextViewer: FC<Props> = ({ value }) => {
const { tag } = value;
const { path } = value.context;
Expand All @@ -136,24 +139,25 @@ export const ValueWithContextViewer: FC<Props> = ({ value }) => {
// Collapse children and element if desired. Uses crude heuristics.
// TODO - this code has side effects, it'd be better if we ran it somewhere else, e.g. traverse values recursively when `ViewerProvider` is initialized.
useState(() => {
const tagsDefaultCollapsed = new Set(["Bool", "Number", "Void", "Input"]);
// TODO - value.size() could be faster.
const childrenCount = getChildrenValues(value).length;

const shouldCollapseChildren = childrenCount > 10;

const shouldCollapseElement = () => {
const shouldBeginCollapsed = (
isRoot: boolean,
v: SqValueWithContext
): boolean => {
if (isRoot) {
return childrenCount > 30;
return getChildrenValues(v).length > 30;
} else {
return childrenCount > 5 || tagsDefaultCollapsed.has(tag);
return (
getChildrenValues(v).length > 5 ||
tagsDefaultCollapsed.has(v.tag) ||
(v.tag === "String" && v.value.length < SHORT_STRING_LENGTH)
berekuk marked this conversation as resolved.
Show resolved Hide resolved
);
}
};

if (shouldCollapseChildren) {
if (getChildrenValues(value).length > 10) {
collapseChildren(value);
}
if (shouldCollapseElement()) {
if (shouldBeginCollapsed(isRoot, value)) {
setCollapsed(path, true);
}
});
Expand Down
Expand Up @@ -33,6 +33,7 @@ import { ItemSettingsMenu } from "./ItemSettingsMenu.js";
import { ValueViewer } from "./ValueViewer.js";
import { SettingsMenuParams } from "./ValueWithContextViewer.js";
import { getChildrenValues } from "./utils.js";
import { SHORT_STRING_LENGTH } from "../../lib/constants.js";

// Distributions should be smaller than the other charts.
// Note that for distributions, this only applies to the internals, there's also extra margin and details.
Expand Down Expand Up @@ -120,7 +121,7 @@ export function getSqValueWidget(value: SqValueWithContext): ValueWidget {
return {
renderPreview: () => (
<div className="overflow-ellipsis overflow-hidden">
{truncateStr(value.value, 20)}
{truncateStr(value.value, SHORT_STRING_LENGTH)}
</div>
),
render: () => (
Expand Down
6 changes: 3 additions & 3 deletions packages/components/src/components/SquiggleViewer/index.tsx
Expand Up @@ -112,14 +112,14 @@ const SquiggleViewerOuter = forwardRef<
}

const body = () => {
if (focused) {
if (!resultVariables.ok) {
return <SquiggleErrorAlert error={resultVariables.value} />;
} else if (focused) {
if (focusedItem) {
return <ValueViewer value={focusedItem} />;
} else {
return <MessageAlert heading="Focused variable is not defined" />;
}
} else if (!resultVariables.ok) {
return <SquiggleErrorAlert error={resultVariables.value} />;
} else {
return (
<div className="space-y-2">
Expand Down
1 change: 1 addition & 0 deletions packages/components/src/lib/constants.ts
@@ -1,2 +1,3 @@
export const SAMPLE_COUNT_MAX = 1000000;
export const SAMPLE_COUNT_MIN = 10;
export const SHORT_STRING_LENGTH = 25;
@@ -1,10 +1,12 @@
import { DropdownMenuAsyncActionItem, TrashIcon, useToast } from "@quri/ui";
import { useRouter } from "next/navigation";
import { FC, useCallback } from "react";
import { useMutation } from "react-relay";
import { useFragment, useMutation } from "react-relay";
import { graphql } from "relay-runtime";

import { DeleteModelActionMutation } from "@/__generated__/DeleteModelActionMutation.graphql";
import { DeleteModelAction$key } from "@/__generated__/DeleteModelAction.graphql";
import { ownerRoute } from "@/routes";

const Mutation = graphql`
mutation DeleteModelActionMutation($input: MutationDeleteModelInput!) {
Expand All @@ -18,12 +20,25 @@ const Mutation = graphql`
`;

type Props = {
owner: string;
slug: string;
model: DeleteModelAction$key;
close(): void;
};

export const DeleteModelAction: FC<Props> = ({ owner, slug, close }) => {
export const DeleteModelAction: FC<Props> = ({ model: modelKey, close }) => {
const model = useFragment(
graphql`
fragment DeleteModelAction on Model {
slug
owner {
__typename
id
slug
}
}
`,
modelKey
);

const router = useRouter();

const [mutation] = useMutation<DeleteModelActionMutation>(Mutation);
Expand All @@ -33,14 +48,14 @@ export const DeleteModelAction: FC<Props> = ({ owner, slug, close }) => {
const onClick = useCallback((): Promise<void> => {
return new Promise((resolve) => {
mutation({
variables: { input: { owner, slug } },
variables: { input: { owner: model.owner.slug, slug: model.slug } },
onCompleted(response) {
if (response.deleteModel.__typename === "BaseError") {
toast(response.deleteModel.message, "error");
resolve();
} else {
// TODO - this is risky, what if we add more error types to GraphQL schema?
router.push("/");
router.push(ownerRoute(model.owner));
}
},
onError(e) {
Expand All @@ -49,7 +64,7 @@ export const DeleteModelAction: FC<Props> = ({ owner, slug, close }) => {
},
});
});
}, [mutation, owner, slug, router, toast]);
}, [mutation, model.owner, model.slug, router, toast]);

return (
<DropdownMenuAsyncActionItem
Expand Down
Expand Up @@ -16,6 +16,7 @@ export const ModelSettingsButton: FC<{
fragment ModelSettingsButton on Model {
slug
...MoveModelAction
...DeleteModelAction
owner {
slug
}
Expand All @@ -34,11 +35,7 @@ export const ModelSettingsButton: FC<{
close={close}
/>
<MoveModelAction model={model} close={close} />
<DeleteModelAction
owner={model.owner.slug}
slug={model.slug}
close={close}
/>
<DeleteModelAction model={model} close={close} />
</DropdownMenu>
)}
>
Expand Down
Expand Up @@ -10,7 +10,6 @@ import { SelectOwner, SelectOwnerOption } from "@/components/SelectOwner";
import { MutationModalAction } from "@/components/ui/MutationModalAction";
import { modelRoute } from "@/routes";
import { MoveModelAction$key } from "@/__generated__/MoveModelAction.graphql";
import { DefaultValues } from "react-hook-form";

const Mutation = graphql`
mutation MoveModelActionMutation($input: MutationMoveModelInput!) {
Expand Down