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

DateInput - Navigating to a different month automatically selects a new date #2418

Closed
rclai opened this issue Apr 23, 2018 · 11 comments
Closed

Comments

@rclai
Copy link

rclai commented Apr 23, 2018

Bug report

@blueprintjs/datetime 1.25.4
Chrome Version 65.0.3325.181 (Official Build) (64-bit)
OSX 10.13.4 (17E199)

Steps to reproduce

  • Select an initial date with the date picker
  • Open the date picker again, navigate to a different month, but don't pick anything
  • Close the date picker

Actual behavior

  • Initial date is picked
  • When opening the date picker, changing the month, not doing anything and closing, the date that was previously selected has changed

Expected behavior

  • Initial date is picked
  • When opening the date picker, changing the month, not doing anything, and closing, the date that was selected initially should stay

Comments

I don't know if this is behavior that you intend on keeping, but this sounds like something that could be controlled via a prop, and if that prop exists, I'm not seeing it in the documentation. It doesn't make sense to me to change the date that was selected using the month navigator by default.

@rclai rclai changed the title DateInput - Changing the month automatically selects a new date DateInput - Navigating to a different month automatically selects a new date Apr 23, 2018
@giladgray
Copy link
Contributor

@rclai this behavior is by design and works fine for us but we'll happily accept a PR to add such a prop.

@mcaneris
Copy link

mcaneris commented May 1, 2018

@rclai the onChange event has a secondary argument,

onChange: (selectedDate: Date, hasUserManuallySelectedDate: boolean) => void

hasUserManuallySelectedDate is false when date changes by navigation which allows you to control the update.

@rclai
Copy link
Author

rclai commented May 3, 2018

Very good to know, thanks. Is this available in v1?

@westrem
Copy link

westrem commented May 3, 2018

@mcaneris DatePicker has such secondary param, but DateInput does not, somehow the param is lost in between making it impossible to react to it when using the DateInput :(

@westrem
Copy link

westrem commented May 3, 2018

Adding it here on this line should help:

Utils.safeInvoke(this.props.onChange, newDate);

@giladgray
Copy link
Contributor

@westrem would you like to submit a PR for this?

@0xorial
Copy link

0xorial commented Jan 9, 2020

Could someone clarify the design? It seems strange to get onChange event since nothing was changed, except some internal state of the control. (the day selection box stays 'in the previous month') If the goal is to track user changing the month, wouldn't it make more sense to make a separate event?

@adidahiya
Copy link
Contributor

@0xorial it looks like this issue was closed a long time ago with the resolution that you have a secondary argument in the onChange callback which helps you discriminate between the two scenarios. This is basically the same as having two events. Does this address your use case?

@0xorial
Copy link

0xorial commented Jan 9, 2020

@adidahiya the second argument is certainly an acceptable workaround. it's just otherwise the API of the library is VERY good and intuitive and this one comes as a surprise.

@adidahiya
Copy link
Contributor

@0xorial fair enough, thanks for the feedback. I would consider a PR to refactor this API to be more intuitive, probably as a breaking change for v4.x

@mustafo
Copy link

mustafo commented May 29, 2021

Totally agree with @0xorial

In addition, the workaround with second argument has this problem:
When the user is selecting another date, which is in different month, the popover won't close. If the next date is within current date's month, popover closes properly. How can I fix this?

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants