Skip to content

Conversation

@yassilah
Copy link

PR Checklist

What is the current behavior?

There is no way to know if the user cancelled or confirmed.

What is the new behavior?

A "cancelled" event is emitted when the return value of the picker is null.

Fixes #41.

Add a cancelled event when the return value is null
@cla-bot
Copy link

cla-bot bot commented Aug 19, 2019

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign the CLA at https://www.nativescript.org/cla.
CLA has not been signed by users: @yassipad.
After signing the CLA, you can ask me to recheck this PR by posting @cla-bot check as a comment to the PR.

@yassilah
Copy link
Author

@cla-bot check

@cla-bot cla-bot bot added the cla: yes label Aug 21, 2019
@cla-bot
Copy link

cla-bot bot commented Aug 21, 2019

The cla-bot has been summoned, and re-checked this pull request!

@elena-p
Copy link
Contributor

elena-p commented Aug 29, 2019

@yassipad,

Could you review and fix the lint errors - https://travis-ci.org/NativeScript/nativescript-datetimepicker/jobs/575108066?

@cla-bot
Copy link

cla-bot bot commented Sep 1, 2019

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: yassipad.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@cla-bot cla-bot bot removed the cla: yes label Sep 1, 2019
@yassilah
Copy link
Author

yassilah commented Sep 1, 2019

@cla-bot check

@cla-bot cla-bot bot added the cla: yes label Sep 1, 2019
@cla-bot
Copy link

cla-bot bot commented Sep 1, 2019

The cla-bot has been summoned, and re-checked this pull request!

@tgpetrov
Copy link
Contributor

tgpetrov commented Sep 3, 2019

Hi @yassipad
I agree that such event would be useful to determine for sure if a user pressed ok or not. My only concern is that now we would not raise the datePickerClosed (and timePickerClosed) event (as the picker is also closed when the user pressed cancel). Maybe we can leave the datePickerClosed (and timePickerClosed) event as it is and add datePickerCancelled and datePickerConfirmed events to differentiate between the press of OK and cancellation. What do you think?

@yassilah
Copy link
Author

yassilah commented Sep 4, 2019

Hi,
Yes this could work :) But couldn’t we just emit both events cancel+closed at the same time?

@tgpetrov
Copy link
Contributor

tgpetrov commented Sep 4, 2019

Yes, that's exactly what I mean - two events at the same - cancel+close vs confirm+close. Something like:

            .then((result: Date) => {
                const args = <EventData>{
                    eventName: DatePickerFieldBase.datePickerCancelledEvent,
                    object: this
                };

                if (result) {
                    this.date = result;
                    args.eventName = DatePickerFieldBase.datePickerConfirmedEvent;
                }
                this.notify(args);

                let closedArgs = <EventData>{
                    eventName: DatePickerFieldBase.datePickerClosedEvent,
                    object: this
                };
                this.notify(closedArgs);
            })

@yassilah
Copy link
Author

yassilah commented Sep 5, 2019

Yes, great, should we send the closed event first or after the cancelled/confirmed event?

@tgpetrov
Copy link
Contributor

tgpetrov commented Sep 9, 2019

It sounds more logical to me to send the confirm/cancel first and then the close event.

@elena-p
Copy link
Contributor

elena-p commented Oct 29, 2019

@yassipad,
Thank you for your pull request and your interest to contribute to the {N} community!
However, there hasn't been any activity on this PR for more than a month and I will close it.
Please, open a new PR when you have handled the comments above.

@elena-p elena-p closed this Oct 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cancel, blur event for nativescript-datetimepicker

3 participants