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

Dropdown closes when label with 'for' tag is clicked #85

Open
DanielReid opened this issue Sep 24, 2018 · 3 comments
Open

Dropdown closes when label with 'for' tag is clicked #85

DanielReid opened this issue Sep 24, 2018 · 3 comments

Comments

@DanielReid
Copy link
Contributor

DanielReid commented Sep 24, 2018

We've run into an issue where giving a label a for id causes the dropdown to close when the label is clicked (but not when the checkbox itself is clicked directly)

 <toggle>
    <a>click me</a>
  </toggle>
  <pane>
    <label for="foo">
      <input type="checkbox" id="foo" ng-model="clicked" >
      Checkbox label
    </label>
  </pane>

It seems this is because there are two click events triggered (one for the label, and a second one for the input element) but somewhere in between setting the event's target, and checking whether the clicked element is contained in the dropdown pane, angular does a digest cycle causing the comparison here to fail:
https://github.com/circlingthesun/angular-foundation-6/blob/master/src/dropdownToggle/dropdownToggle.js#L53

I'm not 100% sure how to deal with is so I figured I'd open an issue for discussion before trying to fix it myself.

@circlingthesun
Copy link
Owner

Hi Daniel, I'm pretty sure the for attribute is not required when you nest the input inside of a label. Otherwise the fix is probably to call stopPropagation() on the event.

@DanielReid
Copy link
Contributor Author

You're right that it's not required (and indeed our current workaround is to not use the id). I just thought it's a bit of a weird gotcha to have the dropdown close in this case.

I'm not well-versed in the DOM event system, but wouldn't calling stopPropagation() on the event potentially break other event handlers? Or should it only be called in some cases? Then we're maybe back at the ng-digest mismatch making it difficult to detect when it's appropriate.

@circlingthesun
Copy link
Owner

I might not have been clear here, but I was referring toggle function. You'll also need to change ng-click="$ctrl.toggle()" to ng-click="$ctrl.toggle($event)" in the template. It could conceivably break something for someone. Maybe some kind of analytics code? I'm not too concerned about that.

Alternatively maybe try:

const isOuterElement = elementContents.every((node) => node !== e.currentTarget);

currentTarget refers to the element that has the event listener attached to it. Where as target is the thing you clicked.

The easiest thing would probably just be to leave the superfluous for out.

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