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

Fix/switch double label click #2298

Closed gyulus3 closed 3 weeks ago

gyulus3 commented 1 month ago

What I did

This pull request fixes an issue where clicking the label of the lion-switch toggles the switch input twice.

Clicking on the label of a switch element and toggling the switch state is a native browser behaviour, so it automatically comes with any browser:

<label for="toggle">
   Label
   <input id="toggle" type="checkbox" switch>
</label>

Since the LionSwitch uses ChoiceInputMixin and its _toggleChecked function, the switch is toggled twice. In the PR I created I fixed this issue by not letting the native browser behavior to toggle the switch state.

Corresponding issue and the problem explanation: fixes https://github.com/ing-bank/lion/issues/2284

changeset-bot[bot] commented 1 month ago

🦋 Changeset detected

Latest commit: 7994117391941bcbed5e01c701d462106018eafa

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package | Name | Type | | -------- | ----- | | @lion/ui | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

gerjanvangeest commented 3 weeks ago

Hi @gyulus3, sorry i was occupied with something else last few weeks.

I tried out this solution locally saw it working, but tried to add some tests. Got distracted and left it as is. Normally i make sure all local changes are stashed aside, but somehow I pushed this fix along side with another PR: https://github.com/ing-bank/lion/pull/2297/files#diff-5528dfc4a7b246f3718cdb6ed68e593ef46cf6046ae41114b5452c3e77e581a4

So this fix is already merged. I will close this PR but thanks for the fix! 💪