Fix ReactKey issues with multiple new annotation rows (#1240)

## Description

<!-- Please provide a brief description of the changes made in this pull request. Include any relevant context or reasoning for the changes. -->
Fixes a bug where React Keys for new annotation rows were unstable due to being based on the row `index` in the array. This would cause issues when, for example, a row was delete or submitted, as all other rows would shift upwards and inherit the info of the row above.

The solution is to assign each new row a unique id specifically for use by React.


I also did a brief bit of tidying up while I was there, by introducing a proper `NewRow` type.

## Related Issue and Pull requests

<!-- Link to any related issues using the format #<issue-number> -->
Closes Shopify/oasis-frontend#336

## Type of Change

<!-- Please delete options that are not relevant -->

- [x] Bug fix

## Checklist

<!-- Please ensure the following are completed before submitting the PR -->

- [ ] I have tested this does not break current pipelines / runs functionality
- [ ] I have tested the changes on staging

## Screenshots (if applicable)

<!-- Include any screenshots that might help explain the changes or provide visual context -->

## Test Instructions

<!-- Detail steps and prerequisites for testing the changes in this PR -->
- Confirm you can now edit/delete/navigate multiple new annotation rows without results being mixed up or erroneously deleted.

## Additional Comments

<!-- Add any additional context or information that reviewers might need to know regarding this PR -->

Author: camielvs

src/components/shared/ReactFlow/FlowCanvas/TaskNode/AnnotationsEditor/AnnotationsEditor.tsx

@@ -6,15 +6,18 @@ import type { AnnotationConfig, Annotations } from "@/types/annotations";
 
 import { AnnotationsInput } from "./AnnotationsInput";
 import { COMPUTE_RESOURCES } from "./ComputeResourcesEditor";
-import { NewAnnotationRow } from "./NewAnnotationRow";
+import {
+  NewAnnotationRow,
+  type NewAnnotationRowData,
+} from "./NewAnnotationRow";
 
 interface AnnotationsEditorProps {
   annotations: Annotations;
   onSave: (key: string, value: string) => void;
   onRemove: (key: string) => void;
-  newRows: Array<{ key: string; value: string }>;
-  onNewRowBlur: (idx: number, newRow: { key: string; value: string }) => void;
-  onRemoveNewRow: (idx: number) => void;
+  newRows: Array<NewAnnotationRowData>;
+  onNewRowBlur: (newRow: NewAnnotationRowData) => void;
+  onRemoveNewRow: (newRow: NewAnnotationRowData) => void;
   onAddNewRow: () => void;
 }
 
@@ -96,11 +99,11 @@ export const AnnotationsEditor = ({
 
       {newRows.map((row, idx) => (
         <NewAnnotationRow
-          key={row.key + idx}
+          key={row.id}
           row={row}
           autofocus={idx === newRows.length - 1}
-          onBlur={(newRow) => onNewRowBlur(idx, newRow)}
-          onRemove={() => onRemoveNewRow(idx)}
+          onBlur={onNewRowBlur}
+          onRemove={onRemoveNewRow}
         />
       ))}
     </div>

src/components/shared/ReactFlow/FlowCanvas/TaskNode/AnnotationsEditor/AnnotationsSection.tsx

@@ -8,6 +8,7 @@ import type { TaskSpec } from "@/utils/componentSpec";
 
 import { AnnotationsEditor } from "./AnnotationsEditor";
 import { ComputeResourcesEditor } from "./ComputeResourcesEditor";
+import type { NewAnnotationRowData } from "./NewAnnotationRow";
 
 interface AnnotationsSectionProps {
   taskSpec: TaskSpec;
@@ -27,43 +28,41 @@ export const AnnotationsSection = ({
   });
 
   // Track new rows separately until they have a key
-  const [newRows, setNewRows] = useState<Array<{ key: string; value: string }>>(
-    [],
-  );
+  const [newRows, setNewRows] = useState<Array<NewAnnotationRowData>>([]);
 
   const handleAddNewRow = useCallback(() => {
-    setNewRows((rows) => [...rows, { key: "", value: "" }]);
+    const newRow = { id: Date.now().toString(), key: "", value: "" };
+    setNewRows((rows) => [...rows, newRow]);
   }, []);
 
-  const handleRemoveNewRow = useCallback((idx: number) => {
-    setNewRows((rows) => rows.filter((_, i) => i !== idx));
+  const handleRemoveNewRow = useCallback((newRow: NewAnnotationRowData) => {
+    setNewRows((rows) => rows.filter((row) => row.id !== newRow.id));
   }, []);
 
   const handleNewRowBlur = useCallback(
-    (idx: number, newRow: { key: string; value: string }) => {
-      const updatedRows = [...newRows];
-      updatedRows[idx] = { ...updatedRows[idx], ...newRow };
-
-      const row = updatedRows[idx];
-
-      if (row.key.trim() && !(row.key in annotations)) {
+    (newRow: NewAnnotationRowData) => {
+      if (newRow.key.trim() && !(newRow.key in annotations)) {
         const newAnnotations = {
           ...annotations,
-          [row.key]: row.value,
+          [newRow.key]: newRow.value,
         };
         setAnnotations(newAnnotations);
         onApply(newAnnotations);
 
-        setNewRows((rows) => rows.filter((_, i) => i !== idx));
+        setNewRows((rows) => rows.filter((row) => row.id !== newRow.id));
       } else {
-        if (row.key.trim() && row.key in annotations) {
+        if (newRow.key.trim() && newRow.key in annotations) {
           notify("Annotation key already exists", "warning");
         }
 
-        setNewRows(updatedRows);
+        setNewRows((rows) =>
+          rows.map((row) =>
+            row.id === newRow.id ? { ...row, ...newRow } : row,
+          ),
+        );
       }
     },
-    [newRows, annotations, onApply],
+    [annotations, onApply, notify],
   );
 
   const handleRemove = useCallback(

src/components/shared/ReactFlow/FlowCanvas/TaskNode/AnnotationsEditor/NewAnnotationRow.tsx

@@ -5,11 +5,17 @@ import { Icon } from "@/components/ui/icon";
 import { Input } from "@/components/ui/input";
 import { InlineStack } from "@/components/ui/layout";
 
+export type NewAnnotationRowData = {
+  id: string;
+  key: string;
+  value: string;
+};
+
 interface NewAnnotationRowProps {
-  row: { key: string; value: string };
+  row: NewAnnotationRowData;
   autofocus: boolean;
-  onBlur: (row: { key: string; value: string }) => void;
-  onRemove: () => void;
+  onBlur: (row: NewAnnotationRowData) => void;
+  onRemove: (row: NewAnnotationRowData) => void;
 }
 
 export const NewAnnotationRow = ({
@@ -21,7 +27,7 @@ export const NewAnnotationRow = ({
   const [key, setKey] = useState(row.key);
   const [value, setValue] = useState(row.value);
 
-  const newRow = useMemo(() => ({ key, value }), [key, value]);
+  const newRow = useMemo(() => ({ ...row, key, value }), [row, key, value]);
 
   const handleRowBlur = useCallback(
     (e: FocusEvent<HTMLDivElement>) => {
@@ -32,6 +38,10 @@ export const NewAnnotationRow = ({
     [newRow, onBlur],
   );
 
+  const handleRowRemove = useCallback(() => {
+    onRemove(newRow);
+  }, [newRow, onRemove]);
+
   return (
     <div onBlur={handleRowBlur}>
       <InlineStack blockAlign="center" gap="2" className="mt-2">
@@ -48,7 +58,7 @@ export const NewAnnotationRow = ({
           onChange={(e) => setValue(e.target.value)}
           className="flex-1"
         />
-        <Button variant="ghost" size="icon" onClick={onRemove}>
+        <Button variant="ghost" size="icon" onClick={handleRowRemove}>
           <Icon name="Trash" className="text-destructive" />
         </Button>
       </InlineStack>