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

[editor] Run Prompt Button UI #639

Merged
merged 2 commits into from
Dec 27, 2023
Merged

[editor] Run Prompt Button UI #639

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
54d7f13
[editor] Prompt Output Bar
Dec 27, 2023
e2d893f
[editor] Run Prompt Button UI
Dec 27, 2023
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
87 changes: 50 additions & 37 deletions python/src/aiconfig/editor/client/src/components/prompt/PromptActionBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { ActionIcon, Container, Flex, Tabs } from "@mantine/core";
import { IconClearAll } from "@tabler/icons-react";
import { memo, useState } from "react";
import ParametersRenderer, { ParametersArray } from "../ParametersRenderer";
import RunPromptButton from "./RunPromptButton";

type Props = {
prompt: ClientPrompt;
Expand Down Expand Up @@ -44,48 +45,60 @@ export default memo(function PromptActionBar({
const promptMetadataSchema = promptSchema?.prompt_metadata;

return (
<Flex direction="column" justify="space-between">
<Flex direction="column" justify="space-between" h="100%">
{isExpanded ? (
<Container miw="400px">
<ActionIcon size="sm" onClick={() => setIsExpanded(false)}>
<IconClearAll />
</ActionIcon>
<Tabs defaultValue="settings">
<Tabs.List>
<Tabs.Tab value="settings">Settings</Tabs.Tab>
{checkParametersSupported(prompt) && (
<Tabs.Tab value="parameters">
Local Variables (Parameters)
</Tabs.Tab>
)}
</Tabs.List>
<>
<Container miw="400px">
<ActionIcon
size="sm"
onClick={() => setIsExpanded(false)}
mt="0.5em"
>
Comment on lines +52 to +56
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think having an "x" button would be more intuitive. Also having it inline horizontally with the tabs instead of ontop would look cleaner. Again not a blocker for MVP

<IconClearAll />
</ActionIcon>
<Tabs defaultValue="settings" mb="1em">
<Tabs.List>
<Tabs.Tab value="settings">Settings</Tabs.Tab>
{checkParametersSupported(prompt) && (
<Tabs.Tab value="parameters">
Local Variables (Parameters)
</Tabs.Tab>
)}
</Tabs.List>

<Tabs.Panel value="settings">
<ModelSettingsRenderer
settings={getModelSettings(prompt)}
schema={modelSettingsSchema}
onUpdateModelSettings={onUpdateModelSettings}
/>
<PromptMetadataRenderer
prompt={prompt}
schema={promptMetadataSchema}
/>
</Tabs.Panel>

{checkParametersSupported(prompt) && (
<Tabs.Panel value="parameters">
<ParametersRenderer
initialValue={getPromptParameters(prompt)}
onUpdateParameters={onUpdateParameters}
<Tabs.Panel value="settings">
<ModelSettingsRenderer
settings={getModelSettings(prompt)}
schema={modelSettingsSchema}
onUpdateModelSettings={onUpdateModelSettings}
/>
<PromptMetadataRenderer
prompt={prompt}
schema={promptMetadataSchema}
/>
</Tabs.Panel>
)}
</Tabs>{" "}
</Container>

{checkParametersSupported(prompt) && (
<Tabs.Panel value="parameters">
<ParametersRenderer
initialValue={getPromptParameters(prompt)}
onUpdateParameters={onUpdateParameters}
/>
</Tabs.Panel>
)}
</Tabs>
</Container>
<RunPromptButton prompt={prompt} size="full" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I'd prefer to have this in the top right of the prompt-input area (not bottom of the settings), similar to what we have in the web app. Not a blocker for MVP, just commenting

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ya, the current placement is a bit far from the input especially for large inputs/outputs. I was thinking it might be better to have it directly across from bottom-right of the input when the action bar is collapsed, otherwise top right of the settings. In any case, this is currently matching designs in https://www.figma.com/file/7YVa6KpEDgptj8DiAc5BaU/AiConfig-Editor?type=design&node-id=264-10830&mode=design&t=vIs81NxFIjhv8BbK-0 so we'll want to handle design/style updates after other things are more complete

cc @zakariassoul for visibility here as well

</>
) : (
<ActionIcon size="sm" onClick={() => setIsExpanded(true)}>
<IconClearAll />
</ActionIcon>
<Flex direction="column" justify="space-between" h="100%">
<Flex direction="row" justify="center" mt="0.5em">
<ActionIcon size="sm" onClick={() => setIsExpanded(true)}>
<IconClearAll />
Copy link
Contributor

@rossdanlm rossdanlm Dec 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry noob question, can you show screenshot and point which button is this from the UI? It's not clear to me right away?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screenshot 2023-12-27 at 11 59 15 AM

It's the stacked lines to open/close the drawer

</ActionIcon>
</Flex>
<RunPromptButton prompt={prompt} size="compact" />
</Flex>
)}
</Flex>
);
Expand Down
8 changes: 5 additions & 3 deletions python/src/aiconfig/editor/client/src/components/prompt/PromptContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ const useStyles = createStyles((theme) => ({
borderBottomRightRadius: 0,
borderTopRightRadius: 0,
},
actionBarCard: {
actionBar: {
border: `1px solid ${theme.colors.gray[3]}`,
borderRadius: "0.25em",
borderBottomLeftRadius: 0,
borderTopLeftRadius: 0,
},
Expand Down Expand Up @@ -93,14 +95,14 @@ export default memo(function PromptContainer({
{prompt.outputs && <PromptOutputsRenderer outputs={prompt.outputs} />}
</Flex>
</Card>
<Card withBorder className={classes.actionBarCard}>
<div className={classes.actionBar}>
<PromptActionBar
prompt={prompt}
promptSchema={promptSchema}
onUpdateModelSettings={updateModelSettings}
onUpdateParameters={updateParameters}
/>
</Card>
</div>
</Flex>
);
});
33 changes: 33 additions & 0 deletions python/src/aiconfig/editor/client/src/components/prompt/RunPromptButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { ClientPrompt } from "../../shared/types";
import { Button, createStyles, Flex, Text } from "@mantine/core";
import { IconPlayerPlayFilled } from "@tabler/icons-react";
import { memo } from "react";

type Props = {
prompt: ClientPrompt;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this used this diff?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not yet, but will be used in subsequent diff

size: "compact" | "full";
};

const useStyles = createStyles(() => ({
executeButton: {
borderBottomLeftRadius: 0,
borderTopLeftRadius: 0,
borderTopRightRadius: 0,
Comment on lines +13 to +15
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we only defining 3 of the corners? Also is 0 the default and not needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We want the bottom right to have radius to match the card border. Default is 0.25em, which already matches the prompt card border radius

},
}));

export default memo(function RunPromptButton({ prompt, size }: Props) {
const { classes } = useStyles();
return (
<Button
onClick={() => {}}
Copy link
Contributor

@rossdanlm rossdanlm Dec 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Oh ok, we'll connect next diff, cool. Just a heads up right now I dont think the backend endpoint supports params argument yet. I have diff for that but need to do some rebasing first: #613

p="xs"
size="xs"
fullWidth={size === "full"}
className={classes.executeButton}
>
<IconPlayerPlayFilled size="16" />
{size === "full" && <Text ml="0.5em">Run</Text>}
</Button>
);
});
15 changes: 12 additions & 3 deletions ...src/aiconfig/editor/client/src/components/prompt/model_settings/ModelSettingsRenderer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import ModelSettingsConfigRenderer from "./ModelSettingsConfigRenderer";
import ModelSettingsSchemaRenderer from "./ModelSettingsSchemaRenderer";
import { GenericPropertiesSchema } from "../../../utils/promptUtils";
import { Flex, Text } from "@mantine/core";
import { Flex, createStyles } from "@mantine/core";
import { JSONObject } from "aiconfig";
import { memo } from "react";

Expand All @@ -11,11 +11,21 @@ type Props = {
onUpdateModelSettings: (settings: Record<string, unknown>) => void;
};

const useStyles = createStyles(() => ({
settingsContainer: {
maxHeight: "400px",
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should we use something else other than px? Ex: %

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Potentially, but just doing this for now and we can update later with our UI/style pass through

overflow: "auto",
paddingTop: "0.5em",
width: "100%",
},
}));

export default memo(function ModelSettingsRenderer({
settings,
schema,
onUpdateModelSettings,
}: Props) {
const { classes } = useStyles();
let settingsComponent;

if (schema) {
Expand All @@ -31,8 +41,7 @@ export default memo(function ModelSettingsRenderer({
}

return (
<Flex direction="column">
<Text>Model Settings</Text>
<Flex direction="column" className={classes.settingsContainer}>
{settingsComponent}
</Flex>
);
Expand Down