material-web icon indicating copy to clipboard operation
material-web copied to clipboard

Switch and Checkbox is not updated when reverted in change event

Open datvm opened this issue 1 year ago • 2 comments

What is affected?

Component

Description

In some scenarios when a switch/checkbox is reverted back under certain conditions in change event handler, the underlying checkbox is not updated correctly and on the next click, the switch/checkbox does not get to the new value that it should be.

    s.addEventListener("change", () => {
        console.log(s.selected);
        if (true) { s.selected = false; } // In real scenarios the condition is met sometimes.
    });

Here's a simulation of what my app behavior is like: Lit Playground. If user checks the switch but then some condition is not satisfied, I revert it back. Then the next time user clicks it, nothing happen and user has to click it a second time.

Reproduction

Lit Playground

  • The switch has the problem: open the console then clicks on it. The console prints out true, false, true, false, ... . The correct behavior should print true every time.
  • The checkbox use a workaround with requestAnimationFrame.
  • The standard <input type="checkbox" /> does not have this issue and correctly prints out true every time.

Workaround

Change the state in the next frame using requestAnimationFrame or setTimeout.


I spent a few hours investigating this issue and strangely enough this probably isn't Material issue but maybe Lit's. See this SO question I made. At first I thought the issue was ?checked instead of .checked but even changing that, the issue still happens. As the above question stated, when render() is called, the selected property already has a correct value so it's not an event handling issue as well.

When tweakling with the source code, I found two solutions:

  1. Use live() as the answer suggested. I am not sure what performance impact this may have.
  2. In the switch's handleChange(), simply use this.selected = !this.selected instead of getting it from the underlying checkbox. This however leave the underlying checkbox state not "up-to-date" with the component.

I am happy to provide a PR for this fix if approved.

Is this a regression?

No or unsure. This never worked, or I haven't tried before.

Affected versions

1.0.1

Browser/OS/Node environment

Microsoft Edge Version 118.0.2088.76 (Official build) (64-bit)

datvm avatar Nov 02 '23 13:11 datvm

Thanks for the detailed issue!

We have plans to move the switch's aria to its host and remove the inner <input> implementation. I think as a side effect, that will fix this issue since the switch will be handling its own state instead of reading it from an <input>.

asyncLiz avatar Nov 02 '23 17:11 asyncLiz

That's great and yes this issue would be fixed as well.

datvm avatar Nov 02 '23 20:11 datvm