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

buttons plugin : avoid multiple change event trigger #31000

Merged
merged 8 commits into from
Oct 2, 2020

Conversation

Lausselloic
Copy link
Contributor

Fixes #30924

  • add unit test to count how many event are thrown when widget contains multiple tags inside label
  • add a parameter to toggle, if click event is provided onto an input then don't trigger another change event already thrown by the browser
  • simplify the case where toggle interface is called click provide from input itself OR it's a button without label. If label is present, then browser propagate clic event from childrens thru label and then cause multiple calls to toggle
  • the test assume that .btn class is always set onto the label if there's one, otherwise need to update this plugin and look for label arround the input

Test with keyboard, mouse and js clic call

@Lausselloic Lausselloic requested a review from a team as a code owner June 10, 2020 13:36
@XhmikosR
Copy link
Member

XhmikosR commented Jun 10, 2020

@patrickhlauke Can you have a look please?

BTW I'm not sure if it's totally OK changing the public API method. Isn't there any other way?

js/src/button.js Outdated Show resolved Hide resolved
@XhmikosR XhmikosR requested a review from Johann-S June 10, 2020 13:45
@mdo
Copy link
Member

mdo commented Jul 14, 2020

@Johann-S Any chance you could review this PR this week? Thanks!

- add unit test to count how many event are thrown when widget contains multiple tags inside label
- add a parameter to toggle, if click event is provided onto an input then don't trigger another change event already thrown by the browser
- simplify the case where toggle interface is called click provide from input itself OR it's a button without label. If label is present, then browser propagate clic event from childrens thru label and then cause multiple calls to toggle
- the test assume that `.btn` class is always set onto the label if there's one, otherwise need to update this plugin and look for label arround the input

Test with keyboard, mouse and js clic call
@XhmikosR XhmikosR force-pushed the buttons_prevent_multiple_onchange branch from 92f03b0 to c3db204 Compare September 14, 2020 07:13
@mdo
Copy link
Member

mdo commented Sep 29, 2020

Friendly bump to @Lausselloic :).

Copy link
Member

@Johann-S Johann-S left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks @Lausselloic 👍

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

Successfully merging this pull request may close these issues.

4 participants