Skip to content

Commit

Permalink
🐛(react) fix checkbox double onChange
Browse files Browse the repository at this point in the history
When clicking on the checkmark it was trigerring two onChange on parent
elements, thus causing double toggling ( which means revert to the initial
value ) in some controlled way approaches ( see the added test ).

Fixes #175
  • Loading branch information
NathanVss committed Oct 2, 2023
1 parent e15586d commit a1e8f46
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 1 deletion.
5 changes: 5 additions & 0 deletions .changeset/proud-readers-yell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@openfun/cunningham-react": patch
---

fix checkbox double onChange
39 changes: 38 additions & 1 deletion packages/react/src/components/Forms/Checkbox/index.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import userEvent from "@testing-library/user-event";
import React from "react";
import React, { useState } from "react";
import { render, screen } from "@testing-library/react";
import {
Checkbox,
Expand Down Expand Up @@ -134,4 +134,41 @@ describe("<Checkbox/>", () => {
render(<Checkbox {...propsInput} />);
expect(spyError).not.toHaveBeenCalled();
});

/**
* From this issue: https://github.com/openfun/cunningham/issues/175
* The bug was that when clicking on the checkmark (svg) it was firing two onClick event to
* <section>.
*/
it("can be unchecked when being controlled from a bigger parent", async () => {
const Wrapper = () => {
const [checked, setChecked] = useState(true);
return (
// eslint-disable-next-line jsx-a11y/no-static-element-interactions
<section
className="center"
onClick={() => {
setChecked(!checked);
}}
style={{ width: "30%", background: "gainsboro", cursor: "pointer" }}
>
<header>Full Card clickable</header>
<div>Checked = {checked ? "true" : "false"}|</div>
<Checkbox checked={checked} />
</section>
);
};

const user = userEvent.setup();
render(<Wrapper />);
const input: HTMLInputElement = screen.getByRole("checkbox");
expect(input.checked).toEqual(true);
screen.getByText("Checked = true|");

// Uncheck by clicking on the SVG.
const checkmark = document.querySelector("svg.checkmark")!;
await user.click(checkmark);
expect(input.checked).toEqual(false);
screen.getByText("Checked = false|");
});
});
2 changes: 2 additions & 0 deletions packages/react/src/components/Forms/Checkbox/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ const Checkmark = () => (
viewBox="0 0 24 24"
fill="none"
xmlns="http://www.w3.org/2000/svg"
onClick={(e) => e.stopPropagation()}
>
<path
d="M17.5685 6.34318L9.8751 14.0366L7.07184 11.2333C6.84127 11.0106 6.53245 10.8874 6.21191 10.8902C5.89137 10.893 5.58474 11.0215 5.35807 11.2482C5.1314 11.4749 5.00283 11.7815 5.00005 12.102C4.99726 12.4226 5.12048 12.7314 5.34318 12.962L9.01077 16.6296C9.24003 16.8587 9.55093 16.9875 9.8751 16.9875C10.1993 16.9875 10.5102 16.8587 10.7394 16.6296L19.2972 8.07184C19.5198 7.84127 19.6431 7.53245 19.6403 7.21191C19.6375 6.89136 19.5089 6.58474 19.2823 6.35807C19.0556 6.1314 18.749 6.00283 18.4284 6.00005C18.1079 5.99726 17.7991 6.12048 17.5685 6.34318Z"
Expand All @@ -118,6 +119,7 @@ const Indeterminate = () => (
viewBox="0 0 24 24"
fill="none"
xmlns="http://www.w3.org/2000/svg"
onClick={(e) => e.stopPropagation()}
>
<rect x="5" y="10" width="14" height="3" rx="1.5" fill="currentColor" />
</svg>
Expand Down

0 comments on commit a1e8f46

Please sign in to comment.