Skip to content
This repository has been archived by the owner. It is now read-only.

feat(dropdown): Make Auto-Close Dropdowns optional. Fix #2218 #3045

Closed
wants to merge 1 commit into from

Conversation

@mariocasciaro
Copy link
Contributor

mariocasciaro commented Dec 3, 2014

Introduces an auto-close option which - when specified - works as follows:

  • outsideClick - closes the dropdown automatically only when the user clicks any element outside the dropdown.
  • disabled - disables the auto close. You can then control the open/close status of the dropdown manually, by using is-open.
@asafdav
Copy link

asafdav commented Jan 16, 2015

+1

@wesleycho
Copy link
Member

wesleycho commented Mar 25, 2015

@chrisirhc I noticed you mentioned in #2218 about being willing to review a PR if someone implements the feature. This PR looks pretty good to me, but I'd like a second pair of eyes since I'm too tired at the moment.

That this is an optional feature makes this ok IMO.

@karianna karianna added this to the Backlog milestone Mar 25, 2015
@@ -2,3 +2,7 @@
Dropdown is a simple directive which will toggle a dropdown menu on click or programmatically.
You can either use `is-open` to toggle or add inside a `<a dropdown-toggle>` element to toggle it when is clicked.
There is also the `on-toggle(open)` optional expression fired when dropdown changes state.
By default the dropdown will automatically close if any of its elements it's clicked, you can change this behaviour by setting the `auto-close` option as follows:

This comment has been minimized.

Copy link
@Delapouite

Delapouite Mar 25, 2015

if any of its elements it's clicked -> if any of its elements is clicked

@@ -2,3 +2,7 @@
Dropdown is a simple directive which will toggle a dropdown menu on click or programmatically.
You can either use `is-open` to toggle or add inside a `<a dropdown-toggle>` element to toggle it when is clicked.
There is also the `on-toggle(open)` optional expression fired when dropdown changes state.
By default the dropdown will automatically close if any of its elements it's clicked, you can change this behaviour by setting the `auto-close` option as follows:

* `outsideClick` - closes the dropdown automatically only when the user clicks any element outside the dropdown.

This comment has been minimized.

Copy link
@chrisirhc

chrisirhc Mar 27, 2015

Member

Should add always here and state it's the default behavior.

@@ -85,6 +93,14 @@ angular.module('ui.bootstrap.dropdown', [])
return self.toggleElement;
};

scope.getAutoClose = function() {
return autoClose;

This comment has been minimized.

Copy link
@chrisirhc

chrisirhc Mar 27, 2015

Member

Can just return $attrs.autoClose here. This will allow support for dynamic autoClose (auto-close="{{ myAutoClose}}"). Though unlikely to be useful, as in most cases this will be a constant, we can save a variable and add support for that.

@chrisirhc
Copy link
Member

chrisirhc commented Mar 27, 2015

Great work. I did a review. This looks good. Just a few suggestions.

@chrisirhc chrisirhc added needs: work and removed needs: review labels Mar 27, 2015
@mariocasciaro
Copy link
Contributor Author

mariocasciaro commented Mar 27, 2015

I'll make the edits as soon as I have 5 minutes. Thanks for reviewing!

@mariocasciaro
Copy link
Contributor Author

mariocasciaro commented Mar 27, 2015

@chrisirhc Merged with upstream and addressed all the comments.

@karianna karianna added needs: review and removed needs: work labels Mar 28, 2015
@wesleycho
Copy link
Member

wesleycho commented Mar 29, 2015

Can you clean up the history? The merge commit should not be there.

@wesleycho wesleycho added needs: work and removed needs: review labels Mar 29, 2015
@wesleycho wesleycho modified the milestones: 0.13.0, Backlog Mar 29, 2015
@mariocasciaro
Copy link
Contributor Author

mariocasciaro commented Mar 30, 2015

Done (and merged with latest commits)

@wesleycho
Copy link
Member

wesleycho commented Mar 31, 2015

Thanks for the work here, this will make it in 0.13.0!

ljantzen pushed a commit to ljantzen/bootstrap that referenced this pull request Apr 14, 2015
fernando-sendMail added a commit to fernando-sendMail/bootstrap that referenced this pull request Jul 9, 2015
fernando-sendMail added a commit to fernando-sendMail/bootstrap that referenced this pull request Jul 16, 2015
fernando-sendMail added a commit to fernando-sendMail/bootstrap that referenced this pull request Jul 16, 2015
fernando-sendMail added a commit to fernando-sendMail/bootstrap that referenced this pull request Jul 16, 2015
fernando-sendMail added a commit to fernando-sendMail/bootstrap that referenced this pull request Jul 16, 2015
fernando-sendMail added a commit to fernando-sendMail/bootstrap that referenced this pull request Jul 16, 2015
fernando-sendMail added a commit to fernando-sendMail/bootstrap that referenced this pull request Jul 16, 2015
fernando-sendMail added a commit to fernando-sendMail/bootstrap that referenced this pull request Jul 16, 2015
fernando-sendMail added a commit to fernando-sendMail/bootstrap that referenced this pull request Jul 16, 2015
fernando-sendMail added a commit to fernando-sendMail/bootstrap that referenced this pull request Jul 16, 2015
fernando-sendMail added a commit to fernando-sendMail/bootstrap that referenced this pull request Jul 16, 2015
fernando-sendMail added a commit to fernando-sendMail/bootstrap that referenced this pull request Jul 16, 2015
fernando-sendMail added a commit to fernando-sendMail/bootstrap that referenced this pull request Jul 16, 2015
fernando-sendMail added a commit to fernando-sendMail/bootstrap that referenced this pull request Jul 16, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
You can’t perform that action at this time.