-
Notifications
You must be signed in to change notification settings - Fork 292
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
feat(playground): example run slideover #5361
base: main
Are you sure you want to change the base?
Conversation
<DialogContainer type="slideOver" isDismissable onDismiss={() => {}}> | ||
<Dialog size="XL" title={`Experiment Run for Example: ${exampleId}`}> | ||
<Flex direction="row" justifyContent="center"> | ||
<View |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need overflow so that the view can scroll when there is large amounts of content inside.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah would copy from the SelectedExampleDiaog in the experiment compare table here since you are building the same thing, doesn't look like it probably makes sense to reuse that component but copying the structure and styles makes sense
https://github.com/Arize-ai/phoenix/blob/main/app/src/pages/experiment/ExperimentCompareTable.tsx#L712
}, [data]); | ||
const { exampleId, input, referenceOutput, runOutput } = run; | ||
return ( | ||
<DialogContainer type="slideOver" isDismissable onDismiss={() => {}}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clicking anywhere within the dialog closes it, including when trying to collapse cards or select text
wonky.sidebar.mov
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(this also shows that you cannot scroll with large outputs)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be happening because the onDismiss function is just that emptyfunction? although that doesn't seem quite right. In either case would recommend passing in an onClose to this that gets passed as the onDismiss prop that allows the parent to control it's closing
<Tooltip>View Trace</Tooltip> | ||
</TooltipTrigger> | ||
)} | ||
{hasSpan && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can put these both in the same conditional just wrap in a fragment
Adds simple slideover for experiment run details in playground
experiment_run_details.mp4
resolves #5286