Fallback for key (#353)

deephaven · Mar 19, 2024 · e13e6ba · e13e6ba
1 parent 48a8bba
commit e13e6ba
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 46 deletions.
diff --git a/plugins/ui/src/js/src/elements/ElementUtils.test.tsx b/plugins/ui/src/js/src/elements/ElementUtils.test.tsx
@@ -75,50 +75,57 @@ describe('wrapElementChildren', () => {
     }
   );
 
-  describe.each(['Some text value', undefined])(
-    'Item element `textValue`: `%s`',
-    textValue => {
-      it.each([
-        ['String', <Text key="String">String</Text>],
-        [999, <Text key="999">999</Text>],
-        [true, <Text key="true">true</Text>],
-        [false, <Text key="false">false</Text>],
+  describe.each([
+    ['Some text value', undefined],
+    [undefined, 'itemKey'],
+  ])('Item element `textValue`:`%s`, `key`:`%s`', (textValue, itemKey) => {
+    it.each([
+      ['String', <Text key="String">String</Text>],
+      [999, <Text key="999">999</Text>],
+      [true, <Text key="true">true</Text>],
+      [false, <Text key="false">false</Text>],
+      [
+        ['String', 999, true, false],
         [
-          ['String', 999, true, false],
-          [
-            <Text key="String">String</Text>,
-            <Text key="999">999</Text>,
-            <Text key="true">true</Text>,
-            <Text key="false">false</Text>,
-          ],
+          <Text key="String">String</Text>,
+          <Text key="999">999</Text>,
+          <Text key="true">true</Text>,
+          <Text key="false">false</Text>,
         ],
-      ])(
-        'should wrap primitive item element children in Text elements: %s, %s',
-        (children, expectedChildren) => {
-          const givenProps =
-            textValue == null ? { children } : { textValue, children };
+      ],
+    ])(
+      'should wrap primitive item element children in Text elements: %s, %s',
+      (children, expectedChildren) => {
+        const givenProps: Record<string, unknown> = { children };
+        if (textValue != null) {
+          givenProps.textValue = textValue;
+        }
 
-          const expectedTextValue =
-            textValue == null && isPrimitive(children)
-              ? String(children)
-              : textValue;
+        if (itemKey != null) {
+          givenProps.key = itemKey;
+        }
 
-          const expected = {
-            [ELEMENT_KEY]: ITEM_ELEMENT_NAME,
-            props: {
-              textValue: expectedTextValue,
-              children: expectedChildren,
-            },
-          };
+        const expectedTextValue =
+          textValue == null && isPrimitive(children)
+            ? String(children)
+            : textValue;
 
-          const actual = wrapElementChildren({
-            [ELEMENT_KEY]: ITEM_ELEMENT_NAME,
-            props: givenProps,
-          });
+        const expected = {
+          [ELEMENT_KEY]: ITEM_ELEMENT_NAME,
+          props: {
+            key: itemKey ?? textValue,
+            textValue: expectedTextValue,
+            children: expectedChildren,
+          },
+        };
 
-          expect(actual).toEqual(expected);
-        }
-      );
-    }
-  );
+        const actual = wrapElementChildren({
+          [ELEMENT_KEY]: ITEM_ELEMENT_NAME,
+          props: givenProps,
+        });
+
+        expect(actual).toEqual(expected);
+      }
+    );
+  });
 });
diff --git a/plugins/ui/src/js/src/elements/ElementUtils.tsx b/plugins/ui/src/js/src/elements/ElementUtils.tsx
@@ -149,12 +149,14 @@ export function wrapElementChildren(element: ElementNode): ElementNode {
   // if they don't contain exactly 1 `string` child, this will trigger a11y
   // warnings. We can set a default `textValue` prop in cases where the
   // original had a single primitive child.
-  if (
-    isItemElement &&
-    !('textValue' in newProps) &&
-    isPrimitive(newProps.children)
-  ) {
-    newProps.textValue = String(newProps.children);
+  if (isItemElement) {
+    if (!('textValue' in newProps) && isPrimitive(newProps.children)) {
+      newProps.textValue = String(newProps.children);
+    }
+
+    if (!('key' in newProps)) {
+      newProps.key = newProps.textValue;
+    }
   }
 
   // Derive child keys based on type + index of the occurrence of the type