fix(CheckboxMultiSelect): properly handle option clicks (#612)

Author: ariser

src/components/IconWrapper/IconWrapper.tsx

@@ -1,26 +1,11 @@
 import { ReactNode } from "react";
+
 import { HorizontalDirection, IconName } from "@/components";
-import { styled } from "styled-components";
-import { EllipsisContent } from "../EllipsisContent/EllipsisContent";
+import { Container, GapOptions } from "@/components/Container/Container";
+import { EllipsisContent } from "@/components/EllipsisContent/EllipsisContent";
 import { Icon } from "@/components/Icon/Icon";
 import { IconSize } from "@/components/Icon/types";
 
-const LabelContainer = styled.div<{ $hasIcon: boolean; $iconDir: HorizontalDirection }>`
-  display: grid;
-  grid-template-columns: ${({ $hasIcon, $iconDir }) =>
-    `${$hasIcon && $iconDir === "start" ? "auto " : ""}1fr${
-      $hasIcon && $iconDir === "end" ? " auto" : ""
-    }`};
-  align-items: center;
-  justify-content: flex-start;
-
-  width: 100%;
-  width: -webkit-fill-available;
-  width: fill-available;
-  width: stretch;
-  gap: ${({ theme }) => theme.click.sidebar.navigation.item.default.space.gap};
-`;
-
 interface IconWrapperProps {
   icon?: IconName;
   iconDir?: HorizontalDirection;
@@ -29,6 +14,7 @@ interface IconWrapperProps {
   height?: number | string;
   children: ReactNode;
   ellipsisContent?: boolean;
+  gap?: GapOptions;
 }
 
 const IconWrapper = ({
@@ -39,13 +25,14 @@ const IconWrapper = ({
   height,
   children,
   ellipsisContent = true,
+  gap = "sm",
   ...props
 }: IconWrapperProps) => {
   const TextWrapper = ellipsisContent ? EllipsisContent : "div";
   return (
-    <LabelContainer
-      $hasIcon={typeof icon === "string"}
-      $iconDir={iconDir}
+    <Container
+      orientation="horizontal"
+      gap={gap}
       {...props}
     >
       {icon && iconDir === "start" && (
@@ -69,7 +56,7 @@ const IconWrapper = ({
           height={height}
         />
       )}
-    </LabelContainer>
+    </Container>
   );
 };
 export default IconWrapper;

src/components/Select/SingleSelectValue.tsx

@@ -31,6 +31,7 @@ const SingleSelectValue = ({
       <IconWrapper
         icon={icon}
         iconDir={iconDir}
+        gap="xxs"
       >
         {label ?? children ?? value}
       </IconWrapper>

src/components/Select/common/InternalSelect.tsx

@@ -25,6 +25,7 @@ import { Portal } from "@radix-ui/react-popover";
 import {
   Checkbox,
   CheckboxVariants,
+  Container,
   Icon,
   IconButton,
   Label,
@@ -585,6 +586,7 @@ export const SelectItem = forwardRef<HTMLDivElement, SelectItemProps>(
           <IconWrapper
             icon={icon}
             iconDir={iconDir}
+            gap="xxs"
           >
             {label ?? children}
           </IconWrapper>
@@ -630,7 +632,7 @@ export const MultiSelectCheckboxItem = forwardRef<
     const { highlighted, updateHighlighted, isHidden, selectedValues, onSelect } =
       useOption();
 
-    const handleSelect = (evt: MouseEvent<HTMLElement>) => {
+    const handleMenuItemClick = (evt: MouseEvent<HTMLElement>) => {
       if (!disabled) {
         onSelect(value, undefined, evt);
 
@@ -640,21 +642,6 @@ export const MultiSelectCheckboxItem = forwardRef<
       }
     };
 
-    const handleMenuItemClick = (evt: MouseEvent<HTMLElement>) => {
-      // Clicking checkbox label fires another click event on the checkbox input.
-      // They cancel each other out, so we handle checkbox clicks separately,
-      // and this covers the outside area.
-      if (evt.target !== evt.currentTarget) {
-        return;
-      }
-
-      handleSelect(evt);
-    };
-
-    const handleCheckboxClick = (evt: MouseEvent<HTMLButtonElement>): void => {
-      handleSelect(evt);
-    };
-
     const handleMenuItemMouseOver = (e: MouseEvent<HTMLDivElement>) => {
       if (onMouseOverProp) {
         onMouseOverProp(e);
@@ -683,59 +670,24 @@ export const MultiSelectCheckboxItem = forwardRef<
           data-testid={`multi-select-checkbox-${value}`}
           cui-select-item=""
         >
-          {icon && iconDir === "start" && (
+          <Container
+            orientation="horizontal"
+            gap="xs"
+          >
             <Checkbox
               checked={isChecked}
               data-testid="multi-select-checkbox"
               disabled={disabled}
-              label={
-                label ? (
-                  <div style={{ display: "flex", justifyContent: "center" }}>
-                    <Icon
-                      name={icon}
-                      size="sm"
-                    />
-                    {label}
-                  </div>
-                ) : (
-                  <div style={{ display: "flex", justifyContent: "center" }}>
-                    <Icon
-                      name={icon}
-                      size="sm"
-                    />
-                    {children}
-                  </div>
-                )
-              }
-              onClick={handleCheckboxClick}
               variant={variant ?? "default"}
             />
-          )}
-          {icon && iconDir === "end" && (
             <IconWrapper
               icon={icon}
-              iconDir="end"
+              iconDir={iconDir}
+              gap="xxs"
             >
-              <Checkbox
-                checked={isChecked}
-                data-testid="multi-select-checkbox"
-                disabled={disabled}
-                label={label ?? children}
-                onClick={handleCheckboxClick}
-                variant={variant ?? "default"}
-              />
+              {label ?? children}
             </IconWrapper>
-          )}
-          {!icon && (
-            <Checkbox
-              checked={isChecked}
-              data-testid="multi-select-checkbox"
-              disabled={disabled}
-              label={label ?? children}
-              onClick={handleCheckboxClick}
-              variant={variant ?? "default"}
-            />
-          )}
+          </Container>
         </GenericMenuItem>
         {separator && <Separator size="sm" />}
       </>