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

Switch and Checkbox is not updated when reverted in change event #5147

Open
datvm opened this issue Nov 2, 2023 · 2 comments
Open

Switch and Checkbox is not updated when reverted in change event #5147

datvm opened this issue Nov 2, 2023 · 2 comments

Comments

@datvm
Copy link
Contributor

datvm commented Nov 2, 2023

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)

@asyncLiz
Copy link
Collaborator

asyncLiz commented Nov 2, 2023

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>.

@datvm
Copy link
Contributor Author

datvm commented Nov 2, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants