angular-ui / bootstrap

PLEASE READ THE PROJECT STATUS BELOW. Native AngularJS (Angular) directives for Bootstrap. Smaller footprint (20kB gzipped), no 3rd party JS dependencies (jQuery, bootstrap JS) required. Please read the README.md file before submitting an issue!
http://angular-ui.github.io/bootstrap/
MIT License
14.28k stars 6.73k forks source link

maxDate and minDate not working on Date picker popup #6025

Open mobius127 opened 8 years ago

mobius127 commented 8 years ago

The issues forum is NOT for support requests. It is for bugs and feature requests only. Please read https://github.com/angular-ui/bootstrap/blob/master/CONTRIBUTING.md and search existing issues (both open and closed) prior to opening any new issue and ensure you follow the instructions therein.

Bug description:

At 1.2.5 of UIBS min-date and max-date properties works perfect but at 1.3.3 this not work anymore and I'm trying with datepicker-options but is not working.

I left a picture of the example, it is a simple date range between two date picker.

PD: on every update, always have issues with date picker, I wonder why you guys make changes in this control that works fine in previous version and in a new version crush.

I'm starting to get tired of this, because there are no documentation of migrating properties between versions of something like that

thanks

Link to minimally-working plunker that reproduces the issue:

image

Version of Angular, UIBS, and Bootstrap

Angular: 1.5.6

UIBS: 1.3.3

Bootstrap: 3.3.6

mobius127 commented 8 years ago

I forgot this pic:

image

mobius127 commented 8 years ago

The problem is datepicker-options are static and doesn't update when Scope changes. In this case, I've made change method in my controller that changes datepicker-options when datepicker input changes.

PD: Still don't see the improvement against min-date and max-date tags in UIBootstrap previous versions.

Html: image

Javascript: image

As you can see, this is more tricky than min-date and max-date tags

wesleycho commented 8 years ago

Please post a reproduction in Plunker of what is intended - otherwise, I will have to close this issue as invalid.

mobius127 commented 8 years ago

Hi,

There is a plunker of working version: https://plnkr.co/edit/RBIdkMd6cG2fN8YFux1i?p=preview

As you can see in plunker example, I had to write an event for update minDate and maxDate, because the datepicker options are static.

In previous versions, minDate and maxDate are updatable by scope when you setted as properties in html markup.

PD: sorry for the naming variables because I dind't have time to change it.

plaflamme commented 8 years ago

I've made a simplified version to demonstrate this issue here

It uses only 2 input fields and does a simple validation: one field's date must be after the other. minDate is updated dynamically by $watching.

There are instructions in the example to reproduce the problem.

antch commented 8 years ago

What @mobius127 says is correct. Prior, you used to be able to do something in the HTML like min-date={{myCtrl.minDate}} and it would update dynamically. Now, I have to physically change the datepicker options object. I can't, for example, take the minDate as a one-way binding in a directive, set it to the datepicker options object, and expect it to change when the bound value changes.

If it's not possible to have these values get watched and automatically updated, it'd at least be nice for us to be able to specify datepicker options in the HTML so that they can be interpolated. Maybe something like, any attribute prefixed with datepicker-option-?

wesleycho commented 8 years ago

Would adding support for an extra configuration option such as minDateChange and maxDateChange for firing a callback bound to those properties of the options object be sufficient for addressing this? That way extra watches would not have to be bound for updating on change, and it gives a little more flexibility as to what can be done when the values change. In addition, it is more succinct.

antch commented 8 years ago

I don't think I understand what you're proposing; would you be willing to provide a short snippet of example usage?

wesleycho commented 8 years ago

I'm proposing

$scope.options = {
  minDate: myDate,
  minDateChange: date => {
    $scope.options2.minDate = date;
  }
};

and similar for maxDate - this would allow a user specified action without creating a $watch, as well as still allowing update whenever one changes. It isn't as automatic as having attributes in the HTML, but it prevents attribute clutter in the templates (a bad practice, decreasing readability), and it avoids creating extra $watches on $scope.

The naming could also change, it could be onMinDateChange to stress it is a change callback.

antch commented 8 years ago

Works for me.

Edit: Wait, I confused myself. I need to be able to automatically change the datepicker's minDate/maxDate option whenever some value on my scope changes; this seems to be achieving the opposite of that.

wesleycho commented 8 years ago

Hm, so it's not updating automatically? That sounds very strange to me, as there is code in the library itself for that, as well as tests around that...

antch commented 8 years ago

I'll try to come up with a minimal repro

antch commented 8 years ago

So, at least in my case I think the crux of the issue is that getting minDate and/or maxDate to update dynamically is a chore when datepicker-options references an object in my controller, for example: datepicker-options="ctrl.dpOptions". The following works but can bloat your HTML quite a bit if you have a lot of datepicker options:

datepicker-options="{ minDate: ctrl.minDate, maxDate: ctrl.maxDate, someOtherDatepickerOption: ctrl.someOtherDatePickerOption, someOtherDatepickerOption2: ctrl.someOtherDatePickerOption2 }"

It would be really neat to allow some sort of attribute convention like datepicker-option-* (so in this case, datepicker-option-min-date and datepicker-option-max-date) that would watch those values and update the datePicker options object dynamically. Then I could do something like:

datepicker-options="ctrl.dpOptions" datepicker-option-min-date="ctrl.minDate" datepicker-option-max-date="ctrl.maxDate"

Just a thought.

Also I'll caveat this by saying that I'm not entirely sure whether this is exactly the same issue raised by others here.

wesleycho commented 8 years ago

One shouldn't put all that information in the view - it's an antipattern. It also was responsible for significant complexity internally in the datepicker, as well as limiting/triggering bugs in the datepicker popup, which is why we moved to a simpler object for cleaner separation & mental overhead.

One should just do something like datepicker-options="datepickerOptions", and in the controller define the datepickerOptions object there. If one needs to update it, one can just do $scope.datepickerOptions.minDate = ... and similarly with the maxDate. The most common scenario when it comes to updating the min date and max date is when using two datepickers to represent a range of dates (for the bounds to be used). My proposed solution solves that common use case.

There are obviously other use cases that my proposal will not solve. I argue that abusing two-way data binding to do everything complicates control of data flow, especially with Angular's pull-based change detection for model changes.

antch commented 8 years ago

Right, so in my case I am taking min/max dates as one-way bindings in a component. In that case, I have to use the $onChanges lifecycle event and then update the object as you suggested (similarly for two-way bound min/max dates you'd have to use $watch).

I did that shortly after my first comment way back when and it does work, but honestly it just seemed like a bit of a step backwards to have to add more moving parts when it worked perfectly fine before. With that said I do understand the motivation behind the change.

wesleycho commented 8 years ago

I understand, we sort of are dealing with limitations in Angular 1 here unfortunately. A little more verbosity isn't terrible for better separation/cleaner templates IMO.

bhamde commented 7 years ago

+1 for this issue to be fixed - very annoying to upgrade to a 'better' version only to have more issues and to have to watch the object and update manually

mbostwick commented 7 years ago

I think this issue relates to https://github.com/angular-ui/bootstrap/issues/6376 as well

guy-roberts commented 7 years ago

+1

ndvalencia commented 7 years ago

Hi, based on the @wesleycho "wesleycho commented on Aug 18 2016" comment, I tested this approach and it is working for me:

Html:

<input type="text" class="form-control" ng-change="onChangeFrom()" datepicker-options="datepickerFromSearchOptions" uib-datepicker-popup="{{appConfig.patterns.date}}" ng-model="searcher.fromSearch" is-open="fromSearchPopup.opened" show-button-bar="false" readonly="true"/>
    <span class="input-group-btn">
      <button type="button" class="btn btn-default" ng-click="openFromSearch()"><i class="icon icon-calendar2"></i></button>
    </span>
<input type="text" class="form-control" ng-change="onChangeTo()" datepicker-options="datepickerToSearchOptions" uib-datepicker-popup="{{appConfig.patterns.date}}" ng-model="searcher.toSearch" is-open="toSearchPopup.opened" show-button-bar="false" readonly="true"/>
    <span class="input-group-btn">
      <button type="button" class="btn btn-default" ng-click="openToSearch()"><i class="icon icon-calendar2"></i></button>
    </span>

Js:

   $scope.datepickerFromSearchOptions = {
        minDate: null,
        maxDate: $scope.searcher.toSearch || null
    };
    $scope.datepickerToSearchOptions = {
        minDate: $scope.searcher.fromSearch || null,
        maxDate: null
    };

    $scope.onChangeFrom = function() {
        $scope.datepickerToSearchOptions.minDate = $scope.searcher.fromSearch || null;
        $scope.onSearch();
    };

    $scope.onChangeTo = function() {
        $scope.datepickerFromSearchOptions.maxDate = $scope.searcher.toSearch || null;
        $scope.onSearch();
    };

Take a look to:

    ng-change="onChangeFrom()" datepicker-options="datepickerFromSearchOptions" 
    ng-change="onChangeTo()" datepicker-options="datepickerToSearchOptions"

    $scope.datepickerFromSearchOptions
    $scope.datepickerToSearchOptions
    $scope.onChangeFrom
    $scope.onChangeTo

Thanks...

b4dnewz commented 7 years ago

a year an half later this is still a issue

AndrewLosikhin commented 7 years ago

As a workaround we can use attributes min-date, max-date.

gString commented 7 years ago

@AndrewLosikhin, I think the point is that you can't - I'm running into the same problem

kevryan2 commented 6 years ago

And almost a year later now.. is this still an issue for anybody else or was there a fix/workaround that I'm unaware of?