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

feature: adding test run outputs UX/UI improvements #1896

Merged
merged 4 commits into from
Jan 26, 2023
Merged

Conversation

xoscar
Copy link
Collaborator

@xoscar xoscar commented Jan 26, 2023

This PR updates the outputs UX/UI to be more interactive as now you can select spans based on outputs and vice-versa.

Changes

  • Output cards are now clickable
  • SpanNodes now have the outputs mark which opens and selects the outputs coming from that span
  • AttributeRow now has the outputs mark which opens and selects the outputs coming from that attribute
  • The autocomplete feature now adds the ability to include outputs as part of the options

Fixes

Checklist

  • tested locally
  • added new dependencies
  • updated the docs
  • added a test

https://www.loom.com/share/cbc55581cbfc45108f4b3b9cd0a1a820

res.Add(output.Name, model.RunOutput{
Value: output.Value,
Name: output.Name,
SpanID: output.SpanId,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Adding span id as part of the output object

@@ -166,6 +166,7 @@ const RunDetailTest = ({run, testId}: IProps) => {
{!isTestSpecFormOpen && !isTestOutputFormOpen && (
<S.TabsContainer>
<Tabs
activeKey={query.get('tab') || TABS.SPECS}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Allowing update based on the url change

@@ -67,7 +69,7 @@ const SpanDetail = ({onCreateTestSpec = noop, searchText, span}: IProps) => {
value: `attr:${key}`,
});

onNavigateAndOpen(output);
onNavigateAndOpen({...output, spanId: span!.id});
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Adding the spanId

$isDeleted={isDeleted}
data-cy="output-item-container"
$isSelected={isSelected}
onClick={handleOutputClick}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Including click event

}

const TestOutputMark = ({className, outputs}: IProps) => {
const {onSelectedOutputs} = useTestOutput();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updates the selected outputs on click

const isDraftMode = useAppSelector(selectIsPending);

useEffect(() => {
dispatch(outputsInitiated(testOutputs));

return () => {
dispatch(outputsReseted());
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Resets the outputs so we don't show them in following tests or other contexts

selectSelectedEnvironmentValues,
selectOutputs,
withOutputsSelector,
(environmentValues, outputs, withOutputs) => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Including outputs into the mix

@xoscar xoscar marked this pull request as ready for review January 26, 2023 17:17
Copy link
Contributor

@jorgeepc jorgeepc left a comment

Choose a reason for hiding this comment

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

Great job @xoscar

@@ -48,6 +48,7 @@ const TestOutputForm = ({isEditing = false, isLoading = false, onCancel, onSubmi
onFinish={values => onSubmit(values, spanIdList[0])}
onValuesChange={onValidate}
>
<Form.Item name="spanId" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to send the spanID from the client?

@xoscar xoscar merged commit 6ac94a3 into main Jan 26, 2023
@xoscar xoscar deleted the outputs-update branch January 26, 2023 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants