-
Notifications
You must be signed in to change notification settings - Fork 285
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(annotations): add ability to edit human span annotations #4111
Conversation
@@ -178,11 +178,6 @@ export function SpanAnnotationForm(props: SpanAnnotationFormProps) { | |||
type="submit" | |||
size="compact" | |||
isDisabled={!isValid || !isDirty || isSubmitting} | |||
onClick={() => { | |||
// TODO: This is a bit of a hack as the form is not working in a dialog for some reason |
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.
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.
hmm, yeah this form submission in a dialog had me scratching my head. I can pull down tomorrow to investigate. I needed this on my machine but maybe you are right to remove.
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.
It was double submitting with, the safest thing if we're not sure if it double submits is to do the opposite of what i did and only do the onClick
@@ -72,7 +72,7 @@ async def patch_span_annotations( | |||
( | |||
models.SpanAnnotation.annotator_kind, | |||
annotation.annotator_kind.value | |||
if annotation.annotator_kind is not None | |||
if annotation.annotator_kind is not None and annotation.annotator_kind is not UNSET |
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.
when annotator kind was not passed in (cannot be updated), we were getting an issue reading value
off of UNSET type
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.
@anticorrelator can you fix up this part?
@@ -226,7 +238,7 @@ function SpanAnnotationsList(props: { | |||
`} | |||
> | |||
{annotations.map((annotation, idx) => ( | |||
<li key={idx}> | |||
<li key={`${idx}_${annotation.name}`}> |
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 key was not unique enough and was causing issues on delete
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.
Hey @Parker-Stafford sorry for the churn. Can we have these annotations be in edit mode from the get go? We need to eliminate the number of clicks as much as possible for these.
if (onEdit) { | ||
items.unshift( | ||
<Item key={AnnotationAction.EDIT}> | ||
<Flex | ||
direction="row" | ||
gap="size-75" | ||
justifyContent="start" | ||
alignItems="center" | ||
> | ||
<Icon svg={<Icons.Edit2Outline />} /> | ||
<Text>Edit</Text> | ||
</Flex> | ||
</Item> | ||
); | ||
} | ||
return items; | ||
}, [onEdit]); |
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.
Can we make it so that annotations are always in edit mode for human? They already clicked on "annotate"
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.
yes makes sense will fix that up!
@@ -72,7 +72,7 @@ async def patch_span_annotations( | |||
( | |||
models.SpanAnnotation.annotator_kind, | |||
annotation.annotator_kind.value | |||
if annotation.annotator_kind is not None | |||
if annotation.annotator_kind is not None and annotation.annotator_kind is not UNSET |
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.
@anticorrelator can you fix up this part?
query { | ||
node(id: $spanId) { | ||
... on Span { | ||
...EditSpanAnnotationsDialog_spanAnnotations |
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.
Can you do the span details fragment here too? Or not necessary
fix: update key for list of span annotations for deleting and adding fix double submission on create remove unused switch to view mode when mutation is completed make human annotations always editable disable save after submission switch to form remove use effect fix ruff clear feature flag simplify package json
bc6bfd4
to
18ca63a
Compare
resolves #4076
Screen.Recording.2024-08-01.at.4.57.01.PM.mov