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

Bug fix for uib-datepicker-popup as attribute #5264

Closed
wants to merge 1 commit into from
Closed

Bug fix for uib-datepicker-popup as attribute #5264

wants to merge 1 commit into from

Conversation

a-t
Copy link

@a-t a-t commented Jan 15, 2016

The uibDatepickerPopup directive's settings max-mode and min-mode are expected to take AngularJS expressions as values. In particular it's mentioned in the documentation. It works fine in versions prior to 1.0.0 of UI Bootstrap.

In 1.0.0 and higher though if uibDatepickerPopup is used as an HTML tag attribute, datepicker breaks with an error on init. Here is a Plunker example.

<input type="text" uib-datepicker-popup max-mode="month" ...>

The above code works, but is incorrect. The correct is like this:

<input type="text" uib-datepicker-popup max-mode="'month'" ...>

but it causes an error in version 1.0.0 an above.

@wesleycho
Copy link
Contributor

cc @Foxandxss - you mentioned wanting to take a look here since there was a big problem you noticed.

@Foxandxss
Copy link
Contributor

I am confused. You talk about maxDate and minDate but your fix is for minMode and maxMode (also your plunker).

The mode one is expected to get a string like in your first example, not an expression.

@a-t
Copy link
Author

a-t commented Jan 15, 2016

I'm sorry. Mode, not date of course. Edited above.
The doc says expression. Look at the ($) badge near the setting description.

@Foxandxss
Copy link
Contributor

Oh, this attribute accept both modes. I plan some major rewrites but not sure if for today or a future version.

For the time being, that $parse on your PR is not needed at all.

@wesleycho wesleycho closed this in 7e93ec9 Jan 15, 2016
@a-t
Copy link
Author

a-t commented Jan 17, 2016

this attribute accept both modes

Well, actually no.

At the present time the issue is fixed by @luber. Many thanks.

@a-t a-t deleted the patch-1 branch January 17, 2016 08:30
@Foxandxss
Copy link
Contributor

We want to release tomorrow, so it will be fixed there.

@a-t
Copy link
Author

a-t commented Jan 17, 2016

Thank you. Sorry for a duplicate issue.

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