Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: ion-datetime with a min/max does not update valid values on first picker change #12319

Closed
eisene opened this issue Jul 11, 2017 · 13 comments
Labels
ionitron: v3 moves the issue to the ionic-v3 repository

Comments

@eisene
Copy link

eisene commented Jul 11, 2017

Ionic version: (check one with "x")
[ ] 1.x (For Ionic 1.x issues, please use https://github.com/ionic-team/ionic-v1)
[ ] 2.x
[x] 3.x

I'm submitting a ... (check one with "x")
[x] bug report
[ ] feature request
[ ] support request => Please do not submit support requests here, use one of these channels: https://forum.ionicframework.com/ or http://ionicworldwide.herokuapp.com/

Current behavior:

  1. Place a DateTime component on a page with a minimum (or maximum) date in the middle of a month/year, for example 06/15/2013
  2. In the model set the initial date that will be shown in the DateTime to the minimum date
  3. Open the date picker and make exactly one change that makes the valid values in the picker columns to change. For example, change the year to 2014
  4. The picker columns don't update, many valid dates are missing
  5. Make one more change in the same column. For example, change the year to 2015
  6. Everything updates and the picker is now in a correct state

Expected behavior:
The valid days and months should update at step 4

Steps to reproduce:
http://plnkr.co/edit/eHapiVZtvMqjjsTJT9Hs?p=preview

  1. Open the datetime picker at the top of the page
  2. Change the year to 2014. The valid values of the month and day picker columns don't update
  3. Change the year to 2015. Now the picker columns update

Related code:

Other information:
This is being caused by the logic in component/picker/picker-column.ts:

if (this.lastIndex === undefined) {
  // have not set a last index yet
  this.lastIndex = this.col.selectedIndex;

} else if (this.lastIndex !== this.col.selectedIndex) {
  // new selected index has changed from the last index
  // update the lastIndex and emit that it has changed
  this.lastIndex = this.col.selectedIndex;
  var ionChange = this.ionChange;
  if (ionChange.observers.length > 0) {
    this._zone.run(ionChange.emit.bind(ionChange, this.col.options[this.col.selectedIndex]));
  }
}

This prevents ionChange from firing the first time a picker column changes. The DateTime subscribes to this event in order to update the valid values of the picker columns. I tried making ionChange fire in both cases, can confirm it fixes the problem. I'm using a fork with this fix in my app and am not experiencing any other problems.

It's also what's causing #12070.

Ionic info: (run ionic info from a terminal/cmd prompt and paste output below):

global packages:

    @ionic/cli-utils : 1.4.0
    Ionic CLI        : 3.4.0

System:

    Node       : v8.1.2
    OS         : macOS Sierra
    Xcode      : Xcode 8.3.3 Build version 8E3004b
    ios-deploy : 1.9.1
    ios-sim    : not installed
    npm        : 5.0.3
@eisene
Copy link
Author

eisene commented Jul 11, 2017

I can make a pull request with the fix I outlined in the issue, but I'm not sure what the intended behavior of picker-column is

@eisene eisene closed this as completed Jul 24, 2017
@eisene eisene reopened this Jul 24, 2017
@eisene
Copy link
Author

eisene commented Jul 26, 2017

It's been more than 2 weeks. Bump

@quedicesebas
Copy link

I have the same problem. In this example, I set the min time to 2017-10-02T11:00:00
image

I changed the day to 3, and as you can see, the hours colun didn't change:
image
Then, I return he day o 2, and again o 3. but this time, the hours columns changed:
image

@eisene
Copy link
Author

eisene commented Oct 16, 2017

I have a fix for this that we've been using for months now without issue. I'm happy to make a pull request. I was hoping to first get some info on the intended behavior from the core dev team but I can also just make the request.

@christianguevara
Copy link

christianguevara commented Nov 6, 2017

Hey @eisene can you please share your solution ?

Edit: Found the commit Bug fix commit

Thank you, will test it.

Edit 2: worked with no problems, i just tested with a ion-datetime no other pickers.

@eisene
Copy link
Author

eisene commented Nov 9, 2017

Cool. Very busy for a bit, will try to make a pull request end of next week.

@olivercodes
Copy link
Contributor

Hey @eisene were you planning on creating the PR? If not I'm going to send one (and I'll just credit you with the fix in the PR).

@olivercodes
Copy link
Contributor

olivercodes commented Nov 28, 2017

I pushed up a PR and credited you there (linked above). If you find the time, feel free to open your own PR and I'll close mine, so that you get some cool points on github if it gets accepted.

@wnabil
Copy link

wnabil commented Jan 24, 2018

PR works for me also, Thanks

@alexnu
Copy link

alexnu commented Mar 31, 2018

I see that the PR above was closed. So I guess this is not going to be fixed?

@kayasky
Copy link

kayasky commented Apr 15, 2018

I've created a new PR for v4 with the same fix: #14329

@Ionitron Ionitron added the ionitron: v3 moves the issue to the ionic-v3 repository label Nov 29, 2018
@ionitron-bot
Copy link

ionitron-bot bot commented Nov 29, 2018

This issue has been automatically identified as an Ionic 3 issue. We recently moved Ionic 3 to its own repository. I am moving this issue to the repository for Ionic 3. Please track this issue over there.

If I've made a mistake, and if this issue is still relevant to Ionic 4, please let the Ionic Framework team know!

Thank you for using Ionic!

@ionitron-bot
Copy link

ionitron-bot bot commented Nov 29, 2018

Issue moved to: ionic-team/ionic-v3#304

@ionitron-bot ionitron-bot bot closed this as completed Nov 29, 2018
@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Nov 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ionitron: v3 moves the issue to the ionic-v3 repository
Projects
None yet
Development

No branches or pull requests

8 participants