search

ing-bank / lion

Fundamental white label web component features for your design system.
https://lion-web.netlify.app/
MIT License
1.72k stars 284 forks source link

[LionSwitch]: Clicking on label tiggers checked-changed twice #2284

Open gyulus3 opened 2 months ago

gyulus3 commented 2 months ago

Expected behavior

Clicking on the label changes the checked value of the switch.

Actual Behavior

Clicking on the label doesn't change the checked value of the switch, because event checked-changed is triggered twice, so it changes back to the original value.

gerjanvangeest commented 2 months ago

The expected behavior is that it only should focus the switch.

But indeed ideally it should not send the checked-changed event at all.

@gyulus3 are you willing to dive into this and and create a fix PR?

gyulus3 commented 2 months ago

In my opinion and as I see in the implementation clicking on the label was intended to trigger the switch and change the checked value, but since it triggers the checked-change twice it does not do anything visually (switching the checked value to opposite and back).

I've already dived into it and trying to solve the issue. The component uses _preventDuplicateLabelClick from the ChoiceInputMixin. The same issue was present once in other components, but this function does not solve the issue in this case. As soon as I find a solution I create a fix PR.

gerjanvangeest commented 2 months ago

@gyulus3 you are indeed correct. I answered without double checking the code. And we even have a test saying so:

it('clicking the label should toggle the checked state', async () => {

./packages/ui/components/switch/test/lion-switch.test.js#L42

So only the duplicate clicking should be fixed.