UI: Apply workaround to TreeNode for iTwinUI-react Checkbox bug (backport #3149)

common/changes/@itwin/core-react/core-react-checkbox-workaround_2022-02-02-10-47.json

@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@itwin/core-react",
+      "comment": "`TreeNode`: Fix checkbox propagating click events to the parent element, resulting in unexpected TreeNodeProps.onClick invocations.",
+      "type": "none"
+    }
+  ],
+  "packageName": "@itwin/core-react"
+}

ui/core-react/src/core-react/tree/Node.tsx

@@ -85,6 +85,8 @@ export interface TreeNodeProps extends CommonProps {
  * @public
  */
 export class TreeNode extends React.Component<TreeNodeProps> {
+  private checkboxWrapperRef = React.createRef<HTMLDivElement>();
+
   constructor(props: TreeNodeProps) {
     super(props);
   }
@@ -115,9 +117,13 @@ export class TreeNode extends React.Component<TreeNodeProps> {
         checkbox = this.props.renderOverrides.renderCheckbox(props);
       } else {
         checkbox = (
-          <Checkbox {...props}
-            onChange={(e: React.ChangeEvent<HTMLInputElement>) => this._onCheckboxChange(e.target.checked)} data-testid={this.createSubComponentTestId("checkbox")}
-          />
+          <div ref={this.checkboxWrapperRef}>
+            <Checkbox
+              {...props}
+              onChange={(e: React.ChangeEvent<HTMLInputElement>) => this._onCheckboxChange(e.target.checked)}
+              data-testid={this.createSubComponentTestId("checkbox")}
+            />
+          </div>
         );
       }
     }
@@ -186,7 +192,23 @@ export class TreeNode extends React.Component<TreeNodeProps> {
 
   private _onClick = (e: React.MouseEvent<HTMLDivElement>) => {
     e.stopPropagation();
-    if (this.props.onClick)
-      this.props.onClick(e);
+
+    // ITwinUI-react has a bug that prevents us from stopping checkbox click event propagation to the parent element
+    if (!this.checkboxWrapperRef.current
+      || (e.target instanceof Element && !isDescendant(this.checkboxWrapperRef.current, e.target))) {
+      this.props.onClick?.(e);
+    }
   };
 }
+
+function isDescendant(ancestor: Element, descendant: Element): boolean {
+  while (descendant.parentElement !== null) {
+    if (descendant === ancestor) {
+      return true;
+    }
+
+    descendant = descendant.parentElement;
+  }
+
+  return false;
+}

ui/core-react/src/test/tree/Node.test.tsx

@@ -6,10 +6,11 @@ import { expect } from "chai";
 import { mount, shallow } from "enzyme";
 import * as React from "react";
 import * as sinon from "sinon";
+import { Checkbox } from "@itwin/itwinui-react";
+import { fireEvent, render } from "@testing-library/react";
+import { TreeNode as Node } from "../../core-react";
 import { CheckBoxState } from "../../core-react/enums/CheckBoxState";
 import { ExpansionToggle } from "../../core-react/tree/ExpansionToggle";
-import { TreeNode as Node } from "../../core-react";
-import { Checkbox } from "@itwin/itwinui-react";
 
 describe("<Node />", () => {
   it("should render", () => {
@@ -117,7 +118,7 @@ describe("<Node />", () => {
     expect(callback).to.not.be.called;
   });
 
-  it("should not call checkboxProps.onClick callback when checkbox is clicked", () => {
+  it("should not call checkboxProps.onClick callback when checkbox label is clicked", () => {
     const callback = sinon.spy();
     const wrapper = mount(<Node label="a" level={0} checkboxProps={{ onClick: callback }} />);
     const checkboxLabel = wrapper.find(Checkbox).find("label");
@@ -128,6 +129,20 @@ describe("<Node />", () => {
     expect(callback).to.not.be.called;
   });
 
+  it("does not call node onClick callback when checkbox is clicked", () => {
+    const handleOnClick = sinon.fake();
+    const { getByRole } = render(<Node label="a" level={0} onClick={handleOnClick} checkboxProps={{}} />);
+    fireEvent.click(getByRole("checkbox").parentElement!);
+    expect(handleOnClick).to.not.have.been.called;
+  });
+
+  it("calls node onClick callback when not checkbox is clicked", () => {
+    const handleOnClick = sinon.fake();
+    const { getByText } = render(<Node label="a" level={0} onClick={handleOnClick} checkboxProps={{}} />);
+    fireEvent.click(getByText("a"));
+    expect(handleOnClick).to.have.been.calledOnce;
+  });
+
   it("should safely handle click events with undefined handlers", () => {
     const wrapper = shallow(<Node label="a" level={0} />);
     wrapper.should.exist;
@@ -156,5 +171,4 @@ describe("<Node />", () => {
     content.simulate("contextmenu");
     expect(callback).to.be.calledOnce;
   });
-
 });