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

Make dropdown menu close on $locationChangeSuccess optional #3683

Closed
markni opened this issue May 18, 2015 · 12 comments
Closed

Make dropdown menu close on $locationChangeSuccess optional #3683

markni opened this issue May 18, 2015 · 12 comments

Comments

@markni
Copy link

markni commented May 18, 2015

When ui-bootstrap used together with ui-router, we have a menu like to stay open, while a nested sub state reloads (with url changes) when user click on menu item.

Currently it seems impossible to do this, as dropdown is forced to close upon $locationChangeSuccess

Would be nice to have something like close-on-location-change='false' to make this behavior optional.

@rvanbaalen
Copy link
Contributor

Feel free to submit a PR implementing this feature. Although I must say I'm not sure if it will make it into the source since we want to keep the API as simple as possible. Adding very specific settings like this one does not add to that philosophy.

@rvanbaalen rvanbaalen added this to the Backlog milestone May 18, 2015
@markni
Copy link
Author

markni commented May 18, 2015

@rvanbaalen I don't want to introduce new API either. Another option would be included it in the new auto-close setting.

It's not very intuitive right now: closing on $locationChangeSuccess is clearly an action of "automatically close", but it always runs despite user explicitly choose auto-close to be disabled or outsideClick (with document states it only closes when click somewhere else).

I think it would make more sense to disable this behavior when auto-close is set to disabled or outsideClick, gives user total control.

@CloudNiner
Copy link
Contributor

This is an issue not only with the ui-router, but also when using lower level angular APIs such as $location.search to update a get param in the url.

I like the idea of not closing the dropdown on $locationChangeSuccess when auto-close is disabled as well since that seems consistent with the current dropdown API and is more inline with what the docs suggest the behavior would be.

I'd be happy to submit a PR that fixes this since the dropdown is currently broken for our use case.

@rvanbaalen Can you provide some direction as to which interface would be acceptable?

@chrisirhc chrisirhc modified the milestones: 0.13.x, Backlog May 23, 2015
@chrisirhc
Copy link
Contributor

Disabling this behavior when auto-close is specified as disabled or outsideClick sounds good to me. It's a newly added option (0.13), anyway, so it shouldn't break things for most users.

@CloudNiner
Copy link
Contributor

Great thanks for the feedback! I'll submit a PR with this change for auto-close="disabled" in the next day or so.

@rvanbaalen
Copy link
Contributor

Awesome! We always appreciate input from the community 👍

@CloudNiner
Copy link
Contributor

@rvanbaalen bumping since I'm not sure you guys would have seen a notification come through for the associated PR I submitted awhile back. If you're already aware of it, then nevermind!

@rvanbaalen
Copy link
Contributor

Thanks @CloudNiner. Did miss the PR notification indeed. Will take a look at it this week I hope.

@CloudNiner
Copy link
Contributor

Great news, thanks!

@joaovieira
Copy link

As @chrisirhc suggested (#3683 (comment)), it would be good to disable the auto close on location change when auto-close is either disabled or outsideClick.

As it is, to me, it reads:

  • auto-close="disabled" - never close, being from an outside click or location change (only explicitly via the toggle button, esc key, or another dropdown opened)
  • auto-close="outsideClick" - only close automatically on outside click, but not on location change (or explicitly, as above)

Imagine the case (our case) when you have a dropdown with a date picker inside:

  • we want the dropdown to close when user clicks outside
  • whenever the user selects a date, we add it to the URL for persistency (hence changing the location)

These two requirements are incompatible. Similarly, we also have filters inside dropdowns where we would like the same behaviour (add to URL without closing, but close on outside click).

Can I suggest changing to:

$scope.$on('$locationChangeSuccess', function() {
  if (scope.getAutoClose() === 'always') {
    scope.isOpen = false;
  }
});

I can make the PR if you agree.

Thanks.

@jasonbriggs
Copy link

I agree that outsideClick should ignore location changes as well (unless I'm missing something). Is there any reason that wasn't included in the fix that was brought in, @CloudNiner?

I'd be happy to create a PR for this as well. It would be incredibly helpful!

@icfantv
Copy link
Contributor

icfantv commented Jun 30, 2016

@jasonbriggs, we decided that as a library it's better to have users do this themselves and opt-in for this behavior rather than force it on everyone and have them opt-out.

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

No branches or pull requests

7 participants