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

Datepicker is-open weirdness #3705

Closed
johnnyreilly opened this issue May 25, 2015 · 9 comments
Closed

Datepicker is-open weirdness #3705

johnnyreilly opened this issue May 25, 2015 · 9 comments

Comments

@johnnyreilly
Copy link

Hi,

I've noticed that when setting is-open to true using an ng-click on the text element itself the date picker popup opens and closes as you would expect. However, if you move the ng-click to a separate button (the calendar glyph for instance) that this does not work.

This plunk demonstrates the problem.

The solution is the pass $event and then call $event.stopPropagation(). This is a behaviour is little unexpected and prompts these questions:

  1. Is this intentional / would someone be able to explain why this works the way it does?
  2. Would it be worth making the documentation spell this behaviour out a little more clearly? I've stumbled over this a couple of times now and ended up writing this up to remind myself in future. I can't be the only person this has confused...
@rvanbaalen
Copy link
Contributor

@RobJacobs Mr. Datepicker, any thoughts on this?

@RobJacobs
Copy link
Contributor

The date picker popup binds a document click event here. I believe if we wrap that in a $timeout the originating click event would not get captured. I'll test that theory later today or tomorrow.

@rvanbaalen
Copy link
Contributor

Cool @RobJacobs thanks for looking into it. Although I must say that my Angular-alert-system went off on the keywords 'wrap that in a $timeout'. Are you sure that's the way to go on this?

@RobJacobs
Copy link
Contributor

Using $timeout(function() { ... }, 0, false) prevents angular from triggering another digest cycle while waiting for the current cycle to complete. So there should not be a performance hit, but I could be wrong.

@rvanbaalen
Copy link
Contributor

I would say go for it -- it's better to have a fix and refactor later if someone else has a better plan instead of not applying a fix at all.

@johnnyreilly
Copy link
Author

Well this is encouraging! Assuming the fix is made I'm guessing it would be non-breaking? i.e. People out there using $event.stopPropagation() wouldn't find their datepickers stopped working as a result of the upgrade.

@RobJacobs
Copy link
Contributor

Under PR #3666 I moved the $timeout call for setting focus to the datepicker popup to this line:

https://github.com/angular-ui/bootstrap/pull/3666/files#diff-6240fc17e068eaeef7095937a1d63eaeR695

Moving the document click bind to the $timeout like so:

$timeout(function() {
  scope.$broadcast('datepicker.focus');
  $document.bind('click', documentClickBind);
 }, 0, false);

Does negate the need for calling stopPropagation and preventDefault in the trigger element click event. I want to wait until #3666 is merged to master before creating another PR so I don't have to worry about merge conflicts.

@johnnyreilly
Copy link
Author

Great - thanks for the update!

@ghost
Copy link

ghost commented Dec 24, 2015

Thank you for this, but it seems that the solution, at least for me was the both
$event.preventDefault();
$event.stopPropagation();

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

3 participants