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

Progressdialog improvements #2097

Merged
merged 7 commits into from
Aug 28, 2015

Conversation

thoemmi
Copy link
Collaborator

@thoemmi thoemmi commented Aug 27, 2015

As far as I can see there is some, er, room for improvements in ProgressDialogController:

  • ProgressDialogController.IsOpen is only set in the c'tor and when the dialog is closed. In fact, I see no reason to have that property, so I eliminated it.
  • By default the state of the progress bar in the dialog is indeterminated. IMHO this should be set by the user explicitly.
  • ProgressDialogController makes direct calls to several controls of ProgressDialog, e.g. WrappedDialog.PART_ProgressBar.IsIndeterminate = true. Instead I'd like to add appropriate methods to ProgressDialog, because the controls are implementation details of the dialog and not the concern of the controller.

@bigworld12
Copy link
Contributor

bigworld12 commented Aug 28, 2015 via email

@thoemmi
Copy link
Collaborator Author

thoemmi commented Aug 28, 2015

@bigworld12 Thanks for your feedback.

i use multi threading in my program which means that i can accidentally
open 2 dialogs at the same time
So i need to check the IsOpen property

Ok, I should have inspected the API more precisely. Indeed, an Closed would be helpful.

About changing style, additional buttons, etc: I'd leave ProgressDialog as is. If someone wants to customize the dialog, he still can derive his own implementation from BaseMetroDialog.

@punker76
Copy link
Member

@thoemmi

About changing style, additional buttons, etc: I'd leave ProgressDialog as is. If someone wants to customize the dialog, he still can derive his own implementation from BaseMetroDialog.

👍

@thoemmi
Copy link
Collaborator Author

thoemmi commented Aug 28, 2015

@punker76 When I add a Closed event to ProgressDialogController is this PR acceptable? (the removal of IsOpen is a breaking change though)

@thoemmi thoemmi changed the title [RFC] Progressdialog improvements [WIP] Progressdialog improvements Aug 28, 2015
@thoemmi
Copy link
Collaborator Author

thoemmi commented Aug 28, 2015

I recreated the IsOpen property and added a Closed event to ProgressDialogController. IMHO this PR is ready to be merged 😁

@thoemmi thoemmi changed the title [WIP] Progressdialog improvements Progressdialog improvements Aug 28, 2015
punker76 added a commit that referenced this pull request Aug 28, 2015
@punker76 punker76 merged commit 225ffe7 into MahApps:master Aug 28, 2015
@thoemmi thoemmi deleted the progressdialog-improvements branch August 28, 2015 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants