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

This will now allow the dialog to call the resolve function when cancelled by clicking outside. #1086

Merged
merged 5 commits into from Nov 17, 2015
Merged

Conversation

NathanaelA
Copy link
Contributor

All the dialogs also suffer on Android from the click outside and then not having the promise resolved or rejected.
This code does two different things:

  1. The promises to be resolved if clicked outside of dialog.
  2. The ability to disable the click outside to cancel. (This might be worth setting as default on the login dialog)

@ns-bot
Copy link

ns-bot commented Nov 11, 2015

Can one of the admins verify this patch?

@ns-bot ns-bot added cla: yes and removed cla: no labels Nov 11, 2015
@enchev
Copy link
Contributor

enchev commented Nov 12, 2015

Hey @NathanaelA,

According to your code you should have cancelable property for dialog options. Will you add iOS implementation also?

Thanks

@NathanaelA

This comment was marked as abuse.

@enchev
Copy link
Contributor

enchev commented Nov 13, 2015

Hey @NathanaelA,

Maybe it will be useful if we can have ActionOptions configurable as well? Something like this:

    /**
     * Provides options for the dialog.
     */
    export interface CancelableOptions {
        /**
         * [Android only] Gets or sets if the dialog can be canceled by taping outside of the dialog.
         */
        cancelable?: boolean;
    }

    /**
     * Provides options for the dialog.
     */
    export interface ActionOptions extends CancelableOptions {
        /**
         * Gets or sets the dialog title.
         */
        title?: string;

        /**
         * Gets or sets the dialog message.
         */
        message?: string;

        /**
         * Gets or sets the Cancel button text.
         */
        cancelButtonText?: string;

        /**
         * Gets or sets the list of available actions.
         */
        actions?: Array<string>;
    }

    /**
     * Provides options for the dialog.
     */
    export interface DialogOptions extends CancelableOptions {
        /**
         * Gets or sets the dialog title.
         */
        title?: string;

        /**
         * Gets or sets the dialog message.
         */
        message?: string;
    }

@NathanaelA

This comment was marked as abuse.

@@ -262,6 +275,9 @@ export function action(arg: any): Promise<string> {
var alert = new android.app.AlertDialog.Builder(activity);
var message = options && types.isString(options.message) ? options.message : "";
var title = options && types.isString(options.title) ? options.title : "";
if (options && (options.cancelable === false || options.cancelable === 0)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @NathanaelA,

Please remove options.cancelable === 0 - this cannot be compiled since the property is boolean.

Thanks

@NathanaelA

This comment was marked as abuse.

@ErjanGavalji
Copy link
Contributor

@NathanaelA: well, there is a problem with the grunt-ts task, where it just does not stop the entire build when broken (I'm sorry I did not spare the time to either fix that or find a workaround for it)

I believe this is what you hit. Try running grunt ts:build and check its result.

@enchev
Copy link
Contributor

enchev commented Nov 17, 2015

run ci

@ns-bot
Copy link

ns-bot commented Nov 17, 2015

Test PASSed.

@enchev
Copy link
Contributor

enchev commented Nov 17, 2015

👍

enchev pushed a commit that referenced this pull request Nov 17, 2015
This will now allow the dialog to call the resolve function when cancelled by clicking outside.
@enchev enchev merged commit 3cf3dcc into NativeScript:master Nov 17, 2015
@enchev enchev added this to the 1.5 milestone Nov 17, 2015
@lock
Copy link

lock bot commented Aug 29, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Aug 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants