feat: TET-858 review changes

src/components/BooleanPill/BooleanPill.props.ts

@@ -1,13 +1,17 @@
+import { HTMLAttributes } from 'react';
+
 import { BooleanPillConfig } from './BooleanPill.styles';
 import { AvatarAppearance } from '../Avatar/types';
 
 export type BooleanPillProps = {
   text: string;
-  state?: 'default' | 'disabled' | 'selected';
+  state?: 'default' | 'disabled';
+  isSelected?: boolean;
   isInverted?: boolean;
+  tabIndex?: number;
   custom?: BooleanPillConfig;
   avatar?:
     | { appearance?: 'image'; image: string }
     | { appearance: Exclude<AvatarAppearance, 'image'>; initials: string };
   onChange?: (state: boolean) => void;
-};
+} & Omit<HTMLAttributes<HTMLSpanElement>, 'color'>;

src/components/BooleanPill/BooleanPill.stories.tsx

@@ -53,13 +53,20 @@ export const Disabled: Story = {
 
 export const Selected: Story = {
   args: {
-    state: 'selected',
+    isSelected: true,
+  },
+};
+
+export const DisabledAndSelected: Story = {
+  args: {
+    isSelected: true,
+    state: 'disabled',
   },
 };
 
 export const SelectedWithAvatar: Story = {
   args: {
-    state: 'selected',
+    isSelected: true,
     avatar: { appearance: 'magenta', initials: 'M' },
   },
 };

src/components/BooleanPill/BooleanPill.styles.ts

@@ -3,6 +3,8 @@ import { BooleanPillState } from './BooleanPillState.type';
 import { BaseProps } from '@/types';
 
 export type BooleanPillConfig = {
+  isSelected: BaseProps;
+  hasAvatar: BaseProps;
   state?: Partial<
     Record<BooleanPillState, Record<'primary' | 'inverted', BaseProps>>
   >;
@@ -14,10 +16,8 @@ export const defaultConfig = {
   alignItems: 'center',
   textAlign: 'center',
   whiteSpace: 'nowrap',
-  minWidth: `62px`,
-  w: 'fit-content',
   h: `$size-small`,
-  padding: ` $space-component-padding-xSmall $space-component-padding-medium`,
+  padding: `$space-component-padding-xSmall $space-component-padding-medium`,
   gap: `$space-component-gap-small`,
   borderRadius: `$border-radius-large`,
   color: '$color-content-primary',
@@ -36,6 +36,18 @@ export const defaultConfig = {
     focus: '$border-width-focus',
   },
   outlineOffset: 1,
+  hasAvatar: {
+    pl: '$space-component-padding-xSmall',
+  },
+  isSelected: {
+    pl: '$space-component-padding-small',
+    backgroundColor: '$color-interaction-background-formField',
+    borderColor: {
+      _: '$color-interaction-border-neutral-normal',
+      hover: '$color-interaction-border-neutral-hover',
+      active: '$color-interaction-border-neutral-active',
+    },
+  },
   state: {
     default: {
       primary: {
@@ -67,24 +79,6 @@ export const defaultConfig = {
         pointerEvents: 'none',
       },
     },
-    selected: {
-      primary: {
-        backgroundColor: '$color-interaction-background-formField',
-        borderColor: {
-          _: '$color-interaction-border-neutral-normal',
-          hover: '$color-interaction-border-neutral-hover',
-          active: '$color-interaction-border-neutral-active',
-        },
-      },
-      inverted: {
-        backgroundColor: '$color-interaction-background-formField',
-        borderColor: {
-          _: '$color-interaction-border-neutral-normal',
-          hover: '$color-interaction-border-neutral-hover',
-          active: '$color-interaction-border-neutral-active',
-        },
-      },
-    },
   },
 } as const satisfies BooleanPillConfig;
 

src/components/BooleanPill/BooleanPill.test.tsx

@@ -5,7 +5,8 @@ import { BooleanPillState } from './BooleanPillState.type';
 import { render, screen, fireEvent } from '../../tests/render';
 
 describe('BooleanPill', () => {
-  const states: BooleanPillState[] = ['default', 'selected', 'disabled'];
+  const states: BooleanPillState[] = ['default', 'disabled'];
+  const selected = [false, true];
   const pillPointer = 'boolean-pill';
 
   it('should render the BooleanPill ', () => {
@@ -57,34 +58,49 @@ describe('BooleanPill', () => {
         expect(checkmark).toBeInTheDocument();
       });
 
-      it('should handle onChange properly when clicked', () => {
-        const onChangeMock = vi.fn();
-        render(
-          <BooleanPill text="Value" state={state} onChange={onChangeMock} />,
-        );
+      selected.forEach((isSelected) => {
+        describe(`isSelected ${isSelected}`, () => {
+          it('should handle onChange properly when clicked', () => {
+            const onChangeMock = vi.fn();
+            render(
+              <BooleanPill
+                text="Value"
+                state={state}
+                isSelected={isSelected}
+                onChange={onChangeMock}
+              />,
+            );
 
-        const pill = screen.getByTestId(pillPointer);
-        expect(pill).toBeInTheDocument();
-        fireEvent.click(pill);
-        if (state !== 'disabled') {
-          expect(onChangeMock).toHaveBeenCalled();
-          expect(onChangeMock).toBeCalledWith(state === 'default');
-        } else {
-          expect(onChangeMock).not.toHaveBeenCalled();
-        }
-      });
+            const pill = screen.getByTestId(pillPointer);
+            expect(pill).toBeInTheDocument();
+            fireEvent.click(pill);
+            if (state !== 'disabled') {
+              expect(onChangeMock).toHaveBeenCalled();
+              expect(onChangeMock).toBeCalledWith(!isSelected);
+            } else {
+              expect(onChangeMock).not.toHaveBeenCalled();
+            }
+          });
 
-      it('should correctly render the checkmark depending on the passed state', () => {
-        render(<BooleanPill text="Value" state={state} />);
-        const pill = screen.getByTestId(pillPointer);
-        const checkmark = screen.queryByTestId('boolean-pill-checkmark');
-        expect(pill).toBeInTheDocument();
+          it('should correctly render the checkmark', () => {
+            render(
+              <BooleanPill
+                text="Value"
+                state={state}
+                isSelected={isSelected}
+              />,
+            );
+            const pill = screen.getByTestId(pillPointer);
+            const checkmark = screen.queryByTestId('boolean-pill-checkmark');
+            expect(pill).toBeInTheDocument();
 
-        if (state !== 'selected') {
-          expect(checkmark).not.toBeInTheDocument();
-        } else {
-          expect(checkmark).toBeInTheDocument();
-        }
+            if (isSelected) {
+              expect(checkmark).toBeInTheDocument();
+            } else {
+              expect(checkmark).not.toBeInTheDocument();
+            }
+          });
+        });
       });
     });
   });

src/components/BooleanPill/BooleanPill.tsx

@@ -9,16 +9,25 @@ import { tet } from '@/tetrisly';
 
 export const BooleanPill: FC<BooleanPillProps> = ({
   state = 'default',
+  isSelected = false,
   isInverted = false,
+  tabIndex = 0,
   avatar,
   text,
   custom,
   onChange,
   ...rest
 }) => {
   const styles = useMemo(
-    () => stylesBuilder(state, isInverted, custom),
-    [custom, isInverted, state],
+    () =>
+      stylesBuilder({
+        state,
+        custom,
+        isSelected,
+        isInverted,
+        hasAvatar: !!avatar,
+      }),
+    [custom, isInverted, state, avatar, isSelected],
   );
 
   const avatarProps = useMemo(
@@ -39,19 +48,20 @@ export const BooleanPill: FC<BooleanPillProps> = ({
 
   const handleOnClick: MouseEventHandler<HTMLSpanElement> = useCallback(() => {
     if (state !== 'disabled') {
-      onChange?.(state === 'default');
+      onChange?.(!isSelected);
     }
-  }, [onChange, state]);
+  }, [onChange, state, isSelected]);
 
   return (
     <tet.span
+      tabIndex={tabIndex}
       data-state={state}
       onClick={handleOnClick}
       data-testid="boolean-pill"
       {...styles.container}
       {...rest}
     >
-      {state === 'selected' && (
+      {isSelected && (
         <Icon data-testid="boolean-pill-checkmark" name="20-check-large" />
       )}
       {!!avatarProps && (

src/components/BooleanPill/BooleanPillState.type.ts

@@ -1 +1 @@
-export type BooleanPillState = 'default' | 'disabled' | 'selected';
+export type BooleanPillState = 'default' | 'disabled';

src/components/BooleanPill/stylesBuilder.ts

@@ -8,11 +8,21 @@ type BooleanPillStyleBuilder = {
   container: BaseProps;
 };
 
-export const stylesBuilder = (
-  state: BooleanPillState,
-  isInverted: boolean,
-  custom?: BooleanPillConfig,
-): BooleanPillStyleBuilder => {
+type BooleanPillStyleBuilderInput = {
+  state: BooleanPillState;
+  isInverted: boolean;
+  isSelected: boolean;
+  hasAvatar: boolean;
+  custom?: BooleanPillConfig;
+};
+
+export const stylesBuilder = ({
+  state,
+  isInverted,
+  isSelected,
+  hasAvatar,
+  custom,
+}: BooleanPillStyleBuilderInput): BooleanPillStyleBuilder => {
   const { state: containerState, ...container } = mergeConfigWithCustom({
     defaultConfig,
     custom,
@@ -21,10 +31,15 @@ export const stylesBuilder = (
     ? containerState[state].inverted
     : containerState[state].primary;
 
+  const withAvatarStyles = hasAvatar ? container.hasAvatar : {};
+  const withSelectedStyles = isSelected ? container.isSelected : {};
+
   return {
     container: {
       ...container,
       ...containerStyles,
+      ...withAvatarStyles,
+      ...withSelectedStyles,
     },
   };
 };

src/docs-components/BooleanPillDocs.tsx

@@ -6,12 +6,17 @@ import { SectionHeader } from './common/SectionHeader';
 import { BooleanPill, BooleanPillProps } from '@/components/BooleanPill';
 import { tet } from '@/tetrisly';
 
-const states = ['default', 'disabled', 'selected'] as const;
+const states = ['default', 'disabled'] as const;
 const appearances = [false, true] as const;
+const selected = [false, true] as const;
 
 const props = [
   { text: 'Value', onChange: () => {} } as const,
-  { text: 'Value', onChange: () => {}, avatar: { initials: 'M' } } as const,
+  {
+    text: 'Value',
+    onChange: () => {},
+    avatar: { initials: 'M' },
+  } as const,
   {
     text: 'Value',
     onChange: () => {},
@@ -48,27 +53,34 @@ export const BooleanPillDocs: FC = () => (
               {appearance ? 'Inverted' : 'Primary'}
             </SectionHeader>
 
-            <tet.div px="$dimension-1000" pb="$dimension-500">
-              <tet.div
-                display="flex"
-                flexBasis="180px"
-                flexGrow="1"
-                flexShrink="0"
-                alignItems="flex-start"
-                justifyContent="flex-start"
-                flexDirection="row"
-                gap="$dimension-300"
-                py="$dimension-500"
-              >
-                {props.map((prop) => (
-                  <BooleanPill
-                    state={state}
-                    isInverted={appearance}
-                    {...prop}
-                  />
-                ))}
+            {selected.map((select) => (
+              <tet.div px="$dimension-1000" pb="$dimension-500">
+                <SectionHeader variant="H3" as="h4" pt="$dimension-500">
+                  Selected: {String(select)}
+                </SectionHeader>
+
+                <tet.div
+                  display="flex"
+                  flexBasis="180px"
+                  flexGrow="1"
+                  flexShrink="0"
+                  alignItems="flex-start"
+                  justifyContent="flex-start"
+                  flexDirection="row"
+                  gap="$dimension-300"
+                  py="$dimension-500"
+                >
+                  {props.map((prop) => (
+                    <BooleanPill
+                      state={state}
+                      isInverted={appearance}
+                      isSelected={select}
+                      {...prop}
+                    />
+                  ))}
+                </tet.div>
               </tet.div>
-            </tet.div>
+            ))}
           </tet.div>
         ))}
       </tet.section>