Update with feedback

ClickHouse · Apr 10, 2024 · 2f25c06 · 2f25c06
1 parent d5f778a
commit 2f25c06
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 10 deletions.
diff --git a/src/components/Select/CheckboxMultiSelect.stories.tsx b/src/components/Select/CheckboxMultiSelect.stories.tsx
@@ -20,7 +20,7 @@ const CheckboxMultiSelectExample = ({ childrenType, value, ...props }: Props) =>
         value={selectedValues}
         options={selectOptions}
         onSelect={value => setSelectedValues(value)}
-        selectLabel="Columns"
+        selectLabel="Columns;"
         {...props}
       />
     );
@@ -73,6 +73,7 @@ export default {
     showCheck: { control: "boolean" },
     orientation: { control: "inline-radio", options: ["horizontal", "vertical"] },
     dir: { control: "inline-radio", options: ["start", "end"] },
+    selectLabel: { control: "text" },
   },
 };
 

diff --git a/src/components/Select/CheckboxMultiSelect.test.tsx b/src/components/Select/CheckboxMultiSelect.test.tsx
@@ -100,17 +100,21 @@ describe("CheckboxCheckboxMultiSelect", () => {
     const selectTrigger = getByTestId("select-trigger");
     selectTrigger && fireEvent.click(selectTrigger);
 
-    expect(screen.getByTestId("multi-select-checkbox-content0")).toHaveAttribute(
+    expect(
+      screen
+        .getByTestId("multi-select-checkbox-content0")
+        .querySelector("[role='checkbox']")
+    ).toHaveAttribute("data-state", "checked");
+
+    const content3 = screen.getByTestId("multi-select-checkbox-content3");
+    expect(content3.querySelector("[role='checkbox']")).toHaveAttribute(
       "data-state",
-      "checked"
+      "unchecked"
     );
 
-    const content2 = screen.getByTestId("multi-select-checkbox-content2");
-    expect(content2).toHaveAttribute("data-state", "unchecked");
-
-    const checkbox = content2.querySelector("[data-testid='multi-select-checkbox']");
+    const checkbox = content3.querySelector("[role='checkbox']");
     checkbox && fireEvent.click(checkbox);
-    expect(screen.getByTestId("multi-select-checkbox-content2")).toHaveAttribute(
+    expect(await content3.querySelector("[role='checkbox']")).toHaveAttribute(
       "data-state",
       "checked"
     );

diff --git a/src/components/Select/common/InternalSelect.tsx b/src/components/Select/common/InternalSelect.tsx
@@ -336,7 +336,7 @@ export const InternalSelect = ({
                 ) : (
                   <SingleSelectValue
                     valueNode={
-                      checkbox
+                      checkbox && selectLabel
                         ? { label: selectLabel as string, value: selectLabel as string }
                         : valueNode.current.get(selectedValues[0])
                     }
@@ -644,7 +644,6 @@ export const MultiSelectCheckboxItem = forwardRef<HTMLDivElement, SelectItemProp
           onClick={onSelectValue}
           onMouseOver={onMouseOver}
           ref={forwardedRef}
-          data-state={isChecked ? "checked" : "unchecked"}
           data-disabled={disabled ? true : undefined}
           data-highlighted={highlighted == value ? "true" : undefined}
           data-testid={`multi-select-checkbox-${value}`}