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.

Sign up for GitHub

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

Jump to bottom

Progress dialogs should be not cancellable outside the window #28

Closed
wants to merge 1 commit into from
Closed

Progress dialogs should be not cancellable outside the window #28

wants to merge 1 commit into from

Conversation

optikalefx
Copy link

This change makes progress dialogs modal. In a sense that they can't be cancelled from an outside touch. This is expected behavior for a progress bar. Too many user errors by mis-tapping outside the progress and then having no clue what happened.

@purplecabbage
Copy link
Contributor

What are progress dialogs ??

@jsoref
Copy link
Contributor

jsoref commented Aug 22, 2014

I'm pretty sure that the pull request title doesn't match the description/implementation.

@optikalefx
Copy link
Author

Sorry about the whitespace. Automatic sublime setting.

A progress dialog (notification.progressDialog is a dialog that has a 1-100 progressbar. The user should not be able to click away. You're right the title should say "not cancellable." I'll update.

@optikalefx optikalefx changed the title Progress dialogs should be cancellable outside the window Progress dialogs should be not cancellable outside the window Aug 22, 2014
@optikalefx
Copy link
Author

The current implementation allows the progress 1-100 dialog box to be cancelled by clicking outside the dialog window. This is unexpected behavior for the user.

@purplecabbage
Copy link
Contributor

The issue as I meant to imply it is that only Android supports this functionality, ultimately it should be moved/removed, or implemented for more platforms.

@optikalefx
Copy link
Author

iOS is modal by default already. So it already performs the expected behavior. I don't know about the other platforms. At least on Android this is a necessary change, and on iOS we didn't have to do anything.

@janpio janpio added the bug label Sep 17, 2018
@brodycj
Copy link

brodycj commented Oct 3, 2018 •
edited
Loading

Looks OK

@brodycj
Copy link

brodycj commented Oct 3, 2018

but could be considered a breaking change.

I think we should increment the major version number before merging, will confirm in dev@cordova.apache.org before merging.

@brodycj brodycj mentioned this pull request Oct 3, 2018
Open
brodycj
brodycj suggested changes Oct 3, 2018
View reviewed changes
Copy link

@brodycj brodycj left a comment

Choose a reason for hiding this comment

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

Whitespace cleanup should not be part of this change. I just raised PR #109 to cleanup the whitespace.

brodycj pushed a commit to brodycj/cordova-plugin-dialogs that referenced this pull request Oct 3, 2018
apacheGH-28 Progress dialogs should be cancellable outside the window
79afd20
@brodycj brodycj mentioned this pull request Oct 4, 2018
Open
@brodycj
Copy link

brodycj commented Oct 30, 2018

Closing in favor of PR #110, with whitespace cleanup done in #109. Note that PR #110 gives both reference and primary author credit to this PR.

@brodycj brodycj closed this Oct 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brodycj brodycj brodycj requested changes

Assignees
No one assigned
Labels
bug platform: android
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@optikalefx @purplecabbage @jsoref @brodycj @janpio