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

fix(datepicker): compatibility with angular 1.1.5 and no jquery #782

Closed
wants to merge 3 commits into from

Conversation

Swiip
Copy link
Contributor

@Swiip Swiip commented Aug 7, 2013

Fix for #760

As I'm pretty noob on good GitHub contrib, don't hesitate to make comments.

There was no easy way to make unit tests in the bug conditions (no jquery and angular 1.1) so I simply verified that it doesn't breaks existings ones.

For the initial $document.unbind which failed, I didn't found better than using a boolean preventing unbind before first bind.

For the open attribute problem, as I understand this new directive : ngOpen, the open attribute is reserved in HTML5 and can only be a boolean. I therefore proposed to switch to is-open with a backward compatibilty trick allowing to use open attribute if the angular version allow it (and is-open is not used).

Hope this helps.
Matthieu

@bekos
Copy link
Contributor

bekos commented Aug 7, 2013

@Swiip Very nice. I don't think atm we care about backward compatibility, since librabry is on 0.x version. So I suggest to simplify code and readability and just rename the attribute. Just make sure you also change the tests, and write about your breaking change in the commit message. An example can be found here: f45815c

For the binding I like your soultion and I agree that it is not testable with the current setup. Doesn't the same problem happens and with the elementFocusBind ?

@Swiip
Copy link
Contributor Author

Swiip commented Aug 7, 2013

For the breaking change, I'll modify the test, thanks for the commit example.

For elementFocusBind, it just doesn't produce the same problem. It should be a difference to find inside jqLite.

BREAKING CHANGE: 'is-open' replace 'open' attribute for the datepicker popup directive.
  Open is an HTML5 reserved attribute which angularjs 1.1.x protects.

  To migrate your code, replace simply replace open by is-open in datepicker-popup directives.

  Before:

  <input ng-model="date" datepicker-popup open="open" ...>

  After:

  <input ng-model="date" datepicker-popup is-open="open" ...>
@bekos
Copy link
Contributor

bekos commented Aug 9, 2013

For elementFocusBind, it just doesn't produce the same problem. It should be a difference to find inside jqLite

@Swiip If attribute is initially open it tries to unbind the elementFocusBind which is not already binded. You should probably address this case in the same way .

@Swiip
Copy link
Contributor Author

Swiip commented Aug 9, 2013

No problem, I was not sure. It's done.

@pkozlowski-opensource
Copy link
Member

@bekos Is this one good to be merged?

@bekos
Copy link
Contributor

bekos commented Aug 10, 2013

LGTM

@pkozlowski-opensource
Copy link
Member

Landed as bf30898. Thnx @Swiip !

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.

3 participants