tamble / jquery-ui-daterangepicker

A JQuery UI based date range picker.
MIT License
184 stars 110 forks source link

call reset() on $mask.click #40

Closed ahawryla closed 8 years ago

ahawryla commented 8 years ago
  1. select any value in a datepicker
  2. open the datepicker again and mark another value, click 'cancel' button
  3. open the datepicker again - you will see a value selected in point 1.
  4. select another value and click somewhere outside the datepicker
  5. open datepicker again - you will see that value from point 4. is marked (getRange will still return value from point 1)

solution: in $mask.click call both close and reset function as it is done for 'cancel' button click handler

op16 commented 8 years ago

So you are arguing that a click somewhere outside the daterangepicker should mean "Cancel", and not just "hide the daterangepicker (and maybe I'll decide later to click Apply or Cancel)"?

The current semantics looks fine to me. Still your proposal might be better.

ahawryla commented 8 years ago

Yes, I am. Right now, in situation described in points 4 and 5 above, there is an inconsistency between what a user can see and what he actually have selected and it may be confusing.

op16 commented 8 years ago

OK, if you want consistency, the same behaviour is needed when the user presses Escape (line 607). If you modify your PR, we'll merge it.

op16 commented 8 years ago

And let's not forget clicking the trigger button (toggle - line 644).

ahawryla commented 8 years ago

I've added reset() also to places indicated by you. Thanks for pointing them out!

ahawryla commented 8 years ago

Thanks a lot! :)

On 2015-12-10 15:27, op16 wrote:

Merged #40 https://github.com/tamble/jquery-ui-daterangepicker/pull/40.

— Reply to this email directly or view it on GitHub https://github.com/tamble/jquery-ui-daterangepicker/pull/40#event-488226577.

op16 commented 8 years ago

Thank you for your contribution!