Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

Fix to allow a form inside a dropdown toggle #324

Closed
wants to merge 3 commits into from

Conversation

lukemurray
Copy link

Moving a login form to angular-ui bootstrap and it was not working. Looking at https://github.com/twitter/bootstrap/blob/master/js/bootstrap-dropdown.js they stop click propagation from a form in the dropdown.

Not sure if it is the best way to find the form (new to angular) but it is working for me.

…ooking at https://github.com/twitter/bootstrap/blob/master/js/bootstrap-dropdown.js they stop click propagation from a form in the dropdown.

Not sure if it is the best way to find the form but it is working.
@ajoslin
Copy link
Contributor

ajoslin commented May 22, 2013

Hi @lukemurray! Finally looking at this :-)

I guess this works, but it seems messy. Thoughts @angular-ui/bootstrap?

Also we will need a unit test showing that this fixes the bug you are talking about.

@bekos
Copy link
Contributor

bekos commented Jun 3, 2013

It think this must be more general. Probably an optional attribute close-on-click (default: true) ?

This way we can use the dropdown to keep multi-select options, datepicker, timepicker etc
It would be even better if we could close it programmatically.

@lukemurray
Copy link
Author

I like making it more customizable. And Yes it should have unit tests. Sorry I have been flat out. I'll jump on it this weekend.

Thanks for the feedback

@lukemurray
Copy link
Author

I finally was working on my other project that was using this popup to show a login form and thought I'd make the change a little more generic like suggested.

I wrote some tests, although I am quite new with karma so let me know if they're OK.

You can now have an attribute close-on-click="false" on the dropdown-toggle element and it will prevent the dropdown from closing when clicking in the dropdown-menu area.

The issue I had it finding the dropdown-menu element. I loop through the parent element items to find one with the class set.

I thought about just using the second child but I'm not sure if the order for the dropdown-toggle and dropdown-menu is important or not.

Let me know what you think.

Cheers,
Luke

@ProLoser
Copy link
Member

  1. What if you use <form ng-click="$event.stopPropagation">?
  2. I think if this option is implemented, it belongs on <ul class="dropdown-menu">, not on DOM that is not involved with the interaction

@eschaefer
Copy link

@lukemurray This saved my day. Kudos.

@ajoslin
Copy link
Contributor

ajoslin commented Jul 27, 2013

Refactor being worked on, #284

@ajoslin ajoslin closed this Jul 27, 2013
@ajoslin ajoslin reopened this Jul 27, 2013
@ajoslin
Copy link
Contributor

ajoslin commented Jul 27, 2013

I guess no need to close this yet though, it is mergeable.

@ajoslin
Copy link
Contributor

ajoslin commented Jul 27, 2013

OK, talked with Pawel and we decided that code needing refactor (original dropdown.js) should not have very specific fixes like this - it is able to cause more problems. Thanks for the work @lukemurray - even though it didn't get merged in you have shown us some problems with current implementation :-)

@ajoslin ajoslin closed this Jul 27, 2013
@lukemurray
Copy link
Author

That's OK.

Will this be something that we can do in a future version? Or is ng-click="$event.stopPropagation" the way to go? To be hinest if the $event way works and I knew about it I wouldn't have done this change :)

Seems there is still a lot to pick up about Angular.

Cheers

@ken210
Copy link

ken210 commented Aug 14, 2013

Looking foward for this merge. Until that i'm using stop-event directive; I think it's cleaner than $event.stopPropagation.

@oludiro
Copy link

oludiro commented Oct 25, 2013

Thank you for this ... saved my fanny big time :)

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

Successfully merging this pull request may close these issues.

7 participants