Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clicking on a label around an input causes the input to be clicked twice #1410

Open
illandril opened this issue Apr 17, 2024 · 7 comments
Open
Labels
bug Something isn't working

Comments

@illandril
Copy link

Describe the bug
Using @testing-library/react, when clicking on a label that includes an input, the input gets clicked twice instead of once.

To Reproduce
codesandbox

Test case that fails:

import React, { useCallback, useState } from "react";
import { render, screen } from "@testing-library/react";
import { userEvent } from "@testing-library/user-event";

const TestForm = () => {
  const [count, setCount] = useState(0);
  const increment = useCallback(() => {
    setCount((v) => v + 1);
  }, [setCount]);

  return (
    <div>
      <label data-testid="checkbox-label">
        <input type="checkbox" onClick={increment} data-testid="checkbox-input" />
        <span data-testid="checkbox-span">Label</span>
      </label>
      <div data-testid="clicks">{count}</div>
    </div>
  );
};

it("should click only once", async () => {
  const user = userEvent.setup();

  render(<TestForm />);

  expect(screen.getByTestId("clicks").textContent).toBe("0");

  await user.click(screen.getByTestId("checkbox-input"));

  expect(screen.getByTestId("clicks").textContent).toBe("1");

  await user.click(screen.getByTestId("checkbox-label"));

  expect(screen.getByTestId("clicks").textContent).toBe("2");

  await user.click(screen.getByTestId("checkbox-span"));

  expect(screen.getByTestId("clicks").textContent).toBe("3");
});

With testEnvironment: "@happy-dom/jest-environment", the test fails (the input click only causes one click, but both the label and span clicks cause two input clicks each).

Expected behavior
Clicking the input, the label, or the span all cause a single click of the input element.

Screenshots
If applicable, add screenshots to help explain your problem.

Device:

  • OS: [e.g. iOS]
  • Browser [e.g. chrome, safari]
  • Version [e.g. 22]

Additional context
The exact same test, but using testEnvironment: "jsdom", passes. The behavior in all browsers I've used also matches what the expected test behavior.

@illandril illandril added the bug Something isn't working label Apr 17, 2024
@illandril
Copy link
Author

After some additional testing, the issue seems to also happen when clicking on a label linked to an input via for="...", not just when the label wraps the input.

@illandril
Copy link
Author

Also, this is not limited to just checkbox inputs, it just is most disruptive for checkbox inputs (since it will check and then immediately uncheck the checkbox).

@Lanny
Copy link

Lanny commented Apr 17, 2024

FWIW the issue seems to affect happy-dom when used by RTL but not jquery, here's a branch adding two tests around this issue as a demonstration:

Lanny@81b88b8

@Lanny
Copy link

Lanny commented Apr 17, 2024

I think the issue is that the "label click induces input click" behavior is something that's been implemented both by @testing-library/user-event here: https://github.com/testing-library/user-event/blob/main/src/event/behavior/click.ts#L13 and by happy-dom here: https://github.com/capricorn86/happy-dom/blob/c29f36c5644eacd0546b5c302ecd6bd7feacf50f/packages/happy-dom/src/nodes/html-label-element/HTMLLabelElement.ts although presumably not by jsdom given it doesn't have the same issue. I'm not sure philosophically which project should own that though

@Lanny
Copy link

Lanny commented Apr 17, 2024

For what it's worth the WHATWG html spec has this to say:

The label element's exact default presentation and behavior, in particular what its activation behavior might be, if anything, should match the platform's label behavior.

which I guess is a case for the difference between happydom and jsdom being acceptable and that user-event shouldn't try and make this behavior a thing. Practically speaking though every major browser vendor (and all the minor ones I know about) implement the label-click-activates-input thing so something's gotta give here or in user-event

@Lanny
Copy link

Lanny commented Apr 17, 2024

Did some more digging, it looks like jsdom does in fact handle the click-on-label behavior, however user-event does this weird thing in its own implementation where it calls .preventDefault() on the event in this scenario but then overrides the defaultPrevented attribute here, which happens to do more or less the right thing due to an implementation detail of jsdom but IMO is the root cause of the issue.

There's an MR with what I take to be a fix on the user-event side above, but apparently nothing's been merged in that repo for several months now so it may be more expedient to try and apply a bandaid to the issue here (maybe a flag to HappyDOMEnvironment to disable the label -> input click propagation?).

@fjpedrosa
Copy link

I am observing the same behavior:, in a component like this:

<div>
  <input
            ref={inputRef}
            id={id || inputId}
            data-cy={id}
            type="checkbox"
            onChange={handleChange}
            defaultChecked={defaultState}
            disabled={disabled}
          />
  <label
          htmlFor={id || inputId}
          className={classNames(styles[size], classes?.label, {
            [styles.isDisabled]: disabled,
          })}
        >
          {label}
        </label>
</div>

this tests:

it('is triggering a click on the input when the label is clicked', async () => {
      const { getByRole, getByText } = render(
        <Toggle label="Toggle test" onChange={onChangeMock} />
      );

      const toggle = getByRole('checkbox') as HTMLInputElement;
      const label = getByText('Toggle test');
      await userEvent.click(label);
      await waitFor(
        () => {
          expect(onChangeMock).toHaveBeenCalledTimes(1);
        },
        { timeout: 300 }
      );
      expect(onChangeMock).toHaveBeenCalledTimes(1);
    });

throws this output:

Expected number of calls: 1
Received number of calls: 3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants