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.

Sign up for GitHub

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

Jump to bottom

Components: Hide small strings, other short stories #2547

Merged
merged 6 commits into from
Nov 17, 2023
Merged

Components: Hide small strings, other short stories #2547

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5fd53c9
Components should hide small strings
OAGr Nov 17, 2023
2b361b3
Export should show error if error, not just variable-undefined
OAGr Nov 17, 2023
bf3c7a8
Deleting a model should return user to owner page
OAGr Nov 17, 2023
cda0359
First parts of CR
OAGr Nov 17, 2023
a412058
Added fragment to DeleteModelAction
OAGr Nov 17, 2023
b62d351
Fix off-by-one length check
berekuk Nov 17, 2023
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
22 changes: 12 additions & 10 deletions packages/components/src/components/Calculator/index.tsx
View file
Open in desktop
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
35 changes: 19 additions & 16 deletions packages/components/src/components/SquiggleViewer/ValueWithContextViewer.tsx
View file
Open in desktop
Expand Up @@ -114,6 +114,23 @@ const ValueViewerBody: FC<Props> = ({ value }) => {
);
};

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

const shouldBeginCollapsed = (
isRoot: boolean,
v: SqValueWithContext
): boolean => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This makes me want to write a long document on why inlining is better, again, but I'll do it some other day...
(still hope to persuade you, and the more I think about it the more convinced I become)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved it much closer. It definitely seems nicer to me to be a bit separate.

if (isRoot) {
return getChildrenValues(v).length > 30;
} else {
return (
getChildrenValues(v).length > 5 ||
tagsDefaultCollapsed.has(v.tag) ||
(v.tag === "String" && v.value.length < 25)
OAGr marked this conversation as resolved.
Show resolved Hide resolved
);
}
};

export const ValueWithContextViewer: FC<Props> = ({ value }) => {
const { tag } = value;
const { path } = value.context;
Expand All @@ -136,24 +153,10 @@ 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 = () => {
if (isRoot) {
return childrenCount > 30;
} else {
return childrenCount > 5 || tagsDefaultCollapsed.has(tag);
}
};

if (shouldCollapseChildren) {
if (getChildrenValues(value).length > 10) {
collapseChildren(value);
}
if (shouldCollapseElement()) {
if (shouldBeginCollapsed(isRoot, value)) {
setCollapsed(path, true);
}
});
Expand Down
6 changes: 3 additions & 3 deletions packages/components/src/components/SquiggleViewer/index.tsx
View file
Open in desktop
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
7 changes: 4 additions & 3 deletions packages/hub/src/app/models/[owner]/[slug]/DeleteModelAction.tsx
View file
Open in desktop
Expand Up @@ -5,6 +5,7 @@ import { useMutation } from "react-relay";
import { graphql } from "relay-runtime";

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

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

type Props = {
owner: string;
owner: { slug: string; __typename: string };
slug: string;
close(): void;
};
Expand All @@ -33,14 +34,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: owner.slug, 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(owner));
}
},
onError(e) {
Expand Down
3 changes: 2 additions & 1 deletion packages/hub/src/app/models/[owner]/[slug]/ModelSettingsButton.tsx
View file
Open in desktop
Expand Up @@ -17,6 +17,7 @@ export const ModelSettingsButton: FC<{
slug
...MoveModelAction
owner {
__typename
OAGr marked this conversation as resolved.
Show resolved Hide resolved
slug
}
}
Expand All @@ -35,7 +36,7 @@ export const ModelSettingsButton: FC<{
/>
<MoveModelAction model={model} close={close} />
<DeleteModelAction
owner={model.owner.slug}
owner={model.owner}
slug={model.slug}
close={close}
/>
Expand Down