Lucas/t 353 checkbox should change state when clicking on their whole (#167)

* Added on click on Checkbox component

* - Added test in story
- Added sleep util
- Fixed click target collision (thanks to test !)

* Use a new CheckboxCell to wrap Checkbox

* Fixed lint

* Refactored CSS after comment

* Fixed tests

front/src/components/form/Checkbox.tsx

@@ -6,13 +6,17 @@ type OwnProps = {
   id: string;
   checked?: boolean;
   indeterminate?: boolean;
-  onChange?: (event: React.ChangeEvent<HTMLInputElement>) => void;
+  onChange?: (newCheckedValue: boolean) => void;
 };
 
-const StyledContainer = styled.span`
+const StyledContainer = styled.div`
+  display: flex;
+  justify-content: center;
+  align-items: center;
+
   input[type='checkbox'] {
     accent-color: ${(props) => props.theme.blue};
-    margin: 8px;
+    margin: 2px;
     height: 14px;
     width: 14px;
     cursor: pointer;
@@ -37,8 +41,15 @@ const StyledContainer = styled.span`
   }
 `;
 
-function Checkbox({ name, id, checked, onChange, indeterminate }: OwnProps) {
+export function Checkbox({
+  name,
+  id,
+  checked,
+  onChange,
+  indeterminate,
+}: OwnProps) {
   const ref = React.useRef<HTMLInputElement>(null);
+
   React.useEffect(() => {
     if (ref.current === null) return;
     if (typeof indeterminate === 'boolean') {
@@ -46,6 +57,12 @@ function Checkbox({ name, id, checked, onChange, indeterminate }: OwnProps) {
     }
   }, [ref, indeterminate, checked]);
 
+  function handleInputChange(event: React.ChangeEvent<HTMLInputElement>) {
+    if (onChange) {
+      onChange(event.target.checked);
+    }
+  }
+
   return (
     <StyledContainer>
       <input
@@ -55,10 +72,8 @@ function Checkbox({ name, id, checked, onChange, indeterminate }: OwnProps) {
         id={id}
         name={name}
         checked={checked}
-        onChange={onChange}
+        onChange={handleInputChange}
       />
     </StyledContainer>
   );
 }
-
-export default Checkbox;

front/src/components/table/CheckboxCell.tsx

@@ -0,0 +1,65 @@
+import * as React from 'react';
+import styled from '@emotion/styled';
+import { Checkbox } from '../form/Checkbox';
+
+type OwnProps = {
+  name: string;
+  id: string;
+  checked?: boolean;
+  indeterminate?: boolean;
+  onChange?: (newCheckedValue: boolean) => void;
+};
+
+const StyledContainer = styled.div`
+  width: 32px;
+  height: 32px;
+  margin-left: -${(props) => props.theme.table.horizontalCellMargin};
+  padding-left: ${(props) => props.theme.table.horizontalCellMargin};
+
+  display: flex;
+  align-items: center;
+  justify-content: center;
+
+  cursor: pointer;
+`;
+
+export function CheckboxCell({
+  name,
+  id,
+  checked,
+  onChange,
+  indeterminate,
+}: OwnProps) {
+  const [internalChecked, setInternalChecked] = React.useState(checked);
+
+  function handleContainerClick() {
+    handleCheckboxChange(!internalChecked);
+  }
+
+  React.useEffect(() => {
+    setInternalChecked(checked);
+  }, [checked]);
+
+  function handleCheckboxChange(newCheckedValue: boolean) {
+    setInternalChecked(newCheckedValue);
+
+    if (onChange) {
+      onChange(newCheckedValue);
+    }
+  }
+
+  return (
+    <StyledContainer
+      onClick={handleContainerClick}
+      data-testid="input-checkbox-cell-container"
+    >
+      <Checkbox
+        id={id}
+        name={name}
+        checked={internalChecked}
+        onChange={handleCheckboxChange}
+        indeterminate={indeterminate}
+      />
+    </StyledContainer>
+  );
+}

front/src/components/table/EntityTable.tsx

@@ -34,12 +34,12 @@ type OwnProps<
 
 const StyledTable = styled.table`
   min-width: 1000px;
-  width: calc(100% - ${(props) => props.theme.spacing(4)});
+  width: calc(100% - 2 * ${(props) => props.theme.table.horizontalCellMargin});
   border-radius: 4px;
   border-spacing: 0;
   border-collapse: collapse;
-  margin-left: ${(props) => props.theme.spacing(2)};
+  margin-left: ${(props) => props.theme.table.horizontalCellMargin};
-  margin-right: ${(props) => props.theme.spacing(2)};
+  margin-right: ${(props) => props.theme.table.horizontalCellMargin};
   table-layout: fixed;
 
   th {

front/src/components/table/SelectAllCheckbox.tsx

@@ -1,14 +1,14 @@
-import Checkbox from '../form/Checkbox';
+import { CheckboxCell } from './CheckboxCell';
 
 export const SelectAllCheckbox = ({
   indeterminate,
   onChange,
 }: {
   indeterminate?: boolean;
-  onChange?: any;
+  onChange?: (newCheckedValue: boolean) => void;
 } & React.HTMLProps<HTMLInputElement>) => {
   return (
-    <Checkbox
+    <CheckboxCell
       name="select-all-checkbox"
       id="select-all-checkbox"
       indeterminate={indeterminate}

front/src/layout/styles/themes.ts

@@ -16,6 +16,10 @@ const commonTheme = {
   fontFamily: 'Inter, sans-serif',
 
   spacing: (multiplicator: number) => `${multiplicator * 4}px`,
+
+  table: {
+    horizontalCellMargin: '8px',
+  },
 };
 
 const lightThemeSpecific = {

front/src/pages/companies/companies-columns.tsx

@@ -9,7 +9,6 @@ import { updateCompany } from '../../services/api/companies';
 import { User, mapToUser } from '../../interfaces/entities/user.interface';
 
 import ColumnHead from '../../components/table/ColumnHead';
-import Checkbox from '../../components/form/Checkbox';
 import { SelectAllCheckbox } from '../../components/table/SelectAllCheckbox';
 import EditableDate from '../../components/editable-cell/EditableDate';
 import EditableRelation from '../../components/editable-cell/EditableRelation';
@@ -29,6 +28,7 @@ import {
 } from 'react-icons/tb';
 import { QueryMode } from '../../generated/graphql';
 import { getLogoUrlFromDomainName } from '../../services/utils';
+import { CheckboxCell } from '../../components/table/CheckboxCell';
 
 const columnHelper = createColumnHelper<Company>();
 
@@ -41,15 +41,15 @@ export const useCompaniesColumns = () => {
           <SelectAllCheckbox
             checked={table.getIsAllRowsSelected()}
             indeterminate={table.getIsSomeRowsSelected()}
-            onChange={table.getToggleAllRowsSelectedHandler()}
+            onChange={(newValue) => table.toggleAllRowsSelected(newValue)}
           />
         ),
         cell: (props: CellContext<Company, string>) => (
-          <Checkbox
+          <CheckboxCell
             id={`company-selected-${props.row.original.id}`}
             name={`company-selected-${props.row.original.id}`}
             checked={props.row.getIsSelected()}
-            onChange={props.row.getToggleSelectedHandler()}
+            onChange={(newValue) => props.row.toggleSelected(newValue)}
           />
         ),
         size: 25,

front/src/pages/people/__stories__/People.inputs.stories.tsx

@@ -6,6 +6,7 @@ import People from '../People';
 import { Story } from './People.stories';
 import { mocks, render } from './shared';
 import { mockedPeopleData } from '../../../testing/mock-data/people';
+import { sleep } from '../../../testing/sleep';
 
 const meta: Meta<typeof People> = {
   title: 'Pages/People',
@@ -39,7 +40,7 @@ export const ChangeEmail: Story = {
 
     await userEvent.click(secondRowEmailCell);
 
-    await new Promise((resolve) => setTimeout(resolve, 25));
+    await sleep(25);
 
     expect(
       canvas.queryByTestId('editable-cell-edit-mode-container'),
@@ -47,7 +48,7 @@ export const ChangeEmail: Story = {
 
     await userEvent.click(secondRowEmailCell);
 
-    await new Promise((resolve) => setTimeout(resolve, 25));
+    await sleep(25);
 
     expect(
       canvas.queryByTestId('editable-cell-edit-mode-container'),
@@ -57,3 +58,34 @@ export const ChangeEmail: Story = {
     msw: mocks,
   },
 };
+
+export const Checkbox: Story = {
+  render,
+  play: async ({ canvasElement }) => {
+    const canvas = within(canvasElement);
+
+    await sleep(500);
+
+    const inputCheckboxContainers = await canvas.findAllByTestId(
+      'input-checkbox-cell-container',
+    );
+
+    const inputCheckboxes = await canvas.findAllByTestId('input-checkbox');
+
+    const secondCheckboxContainer = inputCheckboxContainers[1];
+    const secondCheckbox = inputCheckboxes[1] as HTMLInputElement;
+
+    expect(secondCheckboxContainer).toBeDefined();
+
+    await userEvent.click(secondCheckboxContainer);
+
+    expect(secondCheckbox.checked).toBe(true);
+
+    await userEvent.click(secondCheckbox);
+
+    expect(secondCheckbox.checked).toBe(false);
+  },
+  parameters: {
+    msw: mocks,
+  },
+};

front/src/pages/people/people-columns.tsx

@@ -4,7 +4,6 @@ import { Person } from '../../interfaces/entities/person.interface';
 import { updatePerson } from '../../services/api/people';
 
 import ColumnHead from '../../components/table/ColumnHead';
-import Checkbox from '../../components/form/Checkbox';
 import { SelectAllCheckbox } from '../../components/table/SelectAllCheckbox';
 import EditablePhone from '../../components/editable-cell/EditablePhone';
 import { EditablePeopleFullName } from '../../components/people/EditablePeopleFullName';
@@ -19,6 +18,7 @@ import {
   TbUser,
 } from 'react-icons/tb';
 import { PeopleCompanyCell } from '../../components/people/PeopleCompanyCell';
+import { CheckboxCell } from '../../components/table/CheckboxCell';
 
 const columnHelper = createColumnHelper<Person>();
 
@@ -31,15 +31,15 @@ export const usePeopleColumns = () => {
           <SelectAllCheckbox
             checked={table.getIsAllRowsSelected()}
             indeterminate={table.getIsSomeRowsSelected()}
-            onChange={table.getToggleAllRowsSelectedHandler()}
+            onChange={(newValue) => table.toggleAllRowsSelected(newValue)}
           />
         ),
         cell: (props: CellContext<Person, string>) => (
-          <Checkbox
+          <CheckboxCell
             id={`person-selected-${props.row.original.id}`}
             name={`person-selected-${props.row.original.id}`}
             checked={props.row.getIsSelected()}
-            onChange={props.row.getToggleSelectedHandler()}
+            onChange={(newValue) => props.row.toggleSelected(newValue)}
           />
         ),
         size: 25,

front/src/testing/sleep.ts

@@ -0,0 +1,5 @@
+export async function sleep(ms: number) {
+  return new Promise((resolve) => {
+    setTimeout(resolve, ms);
+  });
+}