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

Datepicker popup doesn't show in Angularjs 1.2.0rc2 #961

Closed
countableSet opened this issue Sep 6, 2013 · 15 comments
Closed

Datepicker popup doesn't show in Angularjs 1.2.0rc2 #961

countableSet opened this issue Sep 6, 2013 · 15 comments

Comments

@countableSet
Copy link

Looks like the popup for the datepicker no longer works in angular 1.2.0rc2.
I'm using a snapshot of angular-ui-bootstrap since there were some previous issues with datepicker in release 0.5.0. This is the commit I'm running off of: 015625d

Here's the plunker for the issue.

@hallister
Copy link

Unless this is new in RC2 this should be a duplicate of #813. Please feel free to let us know if you think this is a different issue.

@countableSet
Copy link
Author

If you look at the plunker and change the angular include from rc2 to rc1- it worked in rc1 and now doesn't work in rc2. I don't see anything related to ng-bind-html-unsafe in the templates with datepicker.

@bekos
Copy link
Contributor

bekos commented Sep 6, 2013

@countableSet Indeed. Popup datepicker breaks by the fixes in ngTransclude introduced in rc2.
We will have to check again.

@bekos bekos reopened this Sep 6, 2013
@pkozlowski-opensource
Copy link
Member

@bekos do you know which AngularJS commit is at fault here? Is this something that should be taken with the AngularJS team?

@bekos
Copy link
Contributor

bekos commented Sep 7, 2013

@pkozlowski-opensource This is the commit that breaks it: angular/angular.js@bf79bd4

The weird thing is that our tests don't fail. Do you have any idea before debugging more closely? I am sure you know better than me :-)

@pkozlowski-opensource
Copy link
Member

@bekos you are highly over-estimating my super-powers :-) I've got no idea what is going on here, I'm afraid....

@bekos
Copy link
Contributor

bekos commented Sep 7, 2013

No way @pkozlowski-opensource :-)

I think it has to do with the order of the transclusion, but I cannot be sure atm to open an issue to AngularJS.

Anyway, I believe it's better to avoid the transclusion in the popup template since there is no actual benefit. The difference, if someone wants to override the popup template, is instead of having a ng-transclude where we inject the datepikcer, he will have to write the datepicker element himself. I 'll open a PR for this.

@bekos
Copy link
Contributor

bekos commented Sep 7, 2013

Above fix is not valid :-(
I have to create the datepicker element and inject it in the template because I have to manipulate it first, like adding attributes.

@bekos
Copy link
Contributor

bekos commented Sep 8, 2013

The problem is that the ngModelController is not available in the linking function of the datepicker, after the $compile of <datepicker-popup-wrap>.

Here is a plunker with a reproduce scenario.

Again, if you replace the templateUrl with inline template in the test directive then there is no problem.

@bekos
Copy link
Contributor

bekos commented Sep 8, 2013

Opened issue in angular: angular/angular.js#3923

@boneskull
Copy link

I was able to workaround this by modifying some lines of code:

      // popup element used to display calendar
      var popupEl = angular.element('<datepicker-popup-wrap><datepicker></datepicker></datepicker-popup-wrap>');
      popupEl.attr({
        'ng-model': 'date',
        'ng-change': 'dateSelection()'
      });
      var datepickerEl = popupEl.find('datepicker');
      if (attrs.datepickerOptions) {
        datepickerEl.attr(angular.extend({}, originalScope.$eval(attrs.datepickerOptions)));
      }

becomes:

      // popup element used to display calendar
      var popupEl = angular.element('<datepicker-popup-wrap><datepicker></datepicker></datepicker-popup-wrap>');
      var datepickerEl = popupEl.find('datepicker');
      datepickerEl.attr({
        'ng-model': 'date',
        'ng-change': 'dateSelection()'
      });
      if (attrs.datepickerOptions) {
        datepickerEl.attr(angular.extend({}, originalScope.$eval(attrs.datepickerOptions)));
      }

But I don't know if that breaks anything else.

@bekos
Copy link
Contributor

bekos commented Sep 27, 2013

@boneskull It actually breaks everything else :-) but it's on the right track for a workaround.
The right change on your code would be like this:

datepickerEl.attr({
        'ng-model': '$parent.$parent.date',
        'ng-change': '$parent.$parent.dateSelection()'
 });

Anyway, we can have this in mind if nothing changes in rc3 that I hope to be soon.

@boneskull
Copy link

Awesome, thanks @bekos

@dotcom9
Copy link

dotcom9 commented Oct 11, 2013

I'm interested in developments on this thread

@pkozlowski-opensource
Copy link
Member

@dotcom9 it was fixed in AngularJS via angular/angular.js#3923, things seem to be in order with the latest AngularJS from master: http://plnkr.co/edit/a292hNtkjeimxq6u6yle?p=preview

I guess we can close this one as the AngularJS fix will be available in RC3

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

6 participants