Hide StatusProgress-bar if user cancels installation #2418

DasSkelett · 2018-04-22T13:54:45Z

Problem:
If you manually cancel the Installation process, the lower right Progress bar keeps moving.

Solution:
Reset and hides StatusProgress bar if user cancels installation.
Give a better StatusMessage too (as "Error!" is not only verry vague, it's incorrect too in this case.)

Fixes #2417

Gives a better `StatusMessage` too.

DasSkelett · 2018-04-22T14:01:06Z

Interestingly, the GUI switches to the Modlist tab afterwards, and I have no idea why, as it shouldn't do it on errors though, and I see no code difference between handling errors and handling user cancellation regarding tab switches.
I suspect, that CKAN switches tabs on errors too (despite the code comment), but I don't know how to simulate an error to test this.

HebaruSan

Added a few code comments. The overall approach looks reasonable to me. Nice job!

I believe the switching back to mod list happens in MainWait.cs, in CancelCurrentActionButton_Click, which calls HideWaitDialog at the end. Just mentioning that in case you want to investigate it.

HebaruSan · 2018-04-22T17:21:58Z

GUI/MainInstall.cs

@@ -390,6 +390,19 @@ private void PostInstallMods(object sender, RunWorkerCompletedEventArgs e)
                RetryCurrentActionButton.Visible = false;
                UpdateChangesDialog(null, installWorker);
            }
+            else if(!result.Key && installCanceled){


We don't need to check result.Key here, because we're already in the else block of if (result.Key).

Also, can you put the curly brace on a new line? This file follows that convention currently.

HebaruSan · 2018-04-22T17:25:04Z

GUI/MainInstall.cs

+                RetryCurrentActionButton.Visible = true;
+                Util.Invoke(statusStrip1, () =>
+                {
+                    StatusProgress.Value = 0;


It would be nice if we didn't have to directly manipulate specific properties of GUI objects here. Some sort of helper function to encapsulate this at a higher level.

MainWait.cs already has a partial API along those lines with ShowWaitDialog and HideWaitDialog. Maybe it should also have FailWaitDialog, which could then be called by the cancel and error blocks here? Looks like the parameters should be the messages to display.

HideWaitDialog already gets passed a bool success, but it's not used yet.
Either we keep this and differentiate success or we create an explicit function.

I thought the idea was to avoid hiding it, though, so the user can look at the errors.

Well, as you mention it...
It would be a bit odd to have a funtion called HideWaitDialog to not hide the WaitDialog in case of an error...

…encies

Moved all GUI manipulations for cancelled installation to FailWaitDialog() in MainWait.cs

DasSkelett · 2018-04-24T17:53:07Z

So, I created a new FailWaitDialog() with error messages as parameters wich executes all GUI calls.
Also I deleted the HideWaitDialog() on CancelCurrentActionButton_Click() so the status log stays open and it's unneccessary with FailWaitDialog().

And because I have bad reflexes and I clicked cancel for testing too slow a lot of times, I realized, that after clicking cancel we don't check if the installation still concluded successfully, resulting in those now false error messages and the GUI not recognizing the newly installed mods.
See here:

Edit: And I'll change the else block to use the new method, too.

DasSkelett · 2018-04-24T19:39:35Z

Need to sleep a night about this as this is some high level sh*t which definitely exceeds my capabilities by now.

Comments are addressed

politas · 2018-04-25T01:53:46Z

this is some high level sh*t which definitely exceeds my capabilities

Boy, I know what that feels like!

Gives a better `StatusMessage` too.

Moved all GUI manipulations for cancelled installation to FailWaitDialog() in MainWait.cs

…ix_#2417

DasSkelett · 2018-04-25T11:58:49Z

Okay this was some strange rebase...

Now clicking cancel first checks if everything was completed successfully despite cancling so there are no false error statements.

DasSkelett · 2018-04-26T17:33:15Z

So I added the check if the installation completed despite "canceling", a bit hacky, but working (I think)
Not ready-to-merge yet, because for unknown causes the retry button stopped working and the GUI laggs shortly after canceling...

Edit: The build fail is not my fault! It's TravisCI which failed on connecting to network again.
Just in order to scotch any rumours...

politas · 2018-04-27T05:14:49Z

What build fail? 😀

politas · 2018-10-07T01:58:42Z

@DasSkelett , is this ready to merge, or are you still needing to work on it some more?

DasSkelett · 2018-12-05T16:37:54Z

Hey, back again, sorry for the long wait.
I'll try to get behind what I did back then. As I just tested, if I cancel AFTER the install completed, it still says error, the "apply changes" button is still falsely active (not thaaaat bad, because if you click it it says already installed, but would be nicer if it would be deactivated) and the retry button moves to a buggy position.
I think I should open a new PR without all those rebase errors? @politas @HebaruSan

Resets and hides StatusProgress bar if user cancels installation.

9e28ec2

Gives a better `StatusMessage` too.

pjf added the Pull request label Apr 22, 2018

HebaruSan added the GUI label Apr 22, 2018

HebaruSan previously requested changes Apr 22, 2018

View reviewed changes

HebaruSan and others added 2 commits April 24, 2018 16:46

Fix "Collection was modified" exception for redundant optional depend…

Verified

This commit was signed with the committer’s verified signature.

politas Myk

GPG key ID: 47938AAEAD85818F

Verified
Learn about vigilant mode

cfe6edf

…encies

Created FailWaitDialog

b0ccd41

Moved all GUI manipulations for cancelled installation to FailWaitDialog() in MainWait.cs

Use FailWaitDialog() on else statement.

9e9c6fe

Treat installed DLC as compatible dependency

d0e8249

Uncheck checkbox after missing dependency error

1827fba

politas added 3 commits April 25, 2018 12:26

Merge #2423 Fix exception for redundant optional dependencies

95e8934

Merge #2424 Treat installed DLC as compatible dependency

6436219

Merge #2425 Uncheck checkbox after missing dependency error

e5c92de

DasSkelett mentioned this pull request Apr 25, 2018

Uncheck checkbox after missing dependency error #2425

Merged

DasSkelett added 4 commits April 25, 2018 13:51

Resets and hides StatusProgress bar if user cancels installation.

7289fb7

Gives a better `StatusMessage` too.

Created FailWaitDialog

41beb7c

Moved all GUI manipulations for cancelled installation to FailWaitDialog() in MainWait.cs

Use FailWaitDialog() on else statement.

6e2570c

Merge branch 'fix_#2417' of https://github.com/dasskelett/CKAN into f…

a394fa5

…ix_#2417

Adding check for successfull install

28a2410

Now clicking cancel first checks if everything was completed successfully despite cancling so there are no false error statements.

politas mentioned this pull request Oct 23, 2018

Canceled download keeps downloading in background #2546

Closed

HebaruSan mentioned this pull request Nov 1, 2018

Don't show status progress bar till actually installing #2560

Merged

Olympic1 closed this in #2560 Nov 4, 2018

DasSkelett mentioned this pull request Dec 7, 2018

Show correct error messages after canceling (un)installations/upgrades #2602

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hide StatusProgress-bar if user cancels installation #2418

Hide StatusProgress-bar if user cancels installation #2418

DasSkelett commented Apr 22, 2018

DasSkelett commented Apr 22, 2018 •

edited

Loading

HebaruSan left a comment

HebaruSan Apr 22, 2018

HebaruSan Apr 22, 2018

DasSkelett Apr 24, 2018

HebaruSan Apr 24, 2018

DasSkelett Apr 24, 2018

DasSkelett commented Apr 24, 2018 •

edited

Loading

DasSkelett commented Apr 24, 2018

politas commented Apr 25, 2018

DasSkelett commented Apr 25, 2018

DasSkelett commented Apr 26, 2018 •

edited

Loading

politas commented Apr 27, 2018

politas commented Oct 7, 2018

DasSkelett commented Dec 5, 2018

Hide StatusProgress-bar if user cancels installation #2418

Hide StatusProgress-bar if user cancels installation #2418

Conversation

DasSkelett commented Apr 22, 2018

DasSkelett commented Apr 22, 2018 • edited Loading

HebaruSan left a comment

Choose a reason for hiding this comment

HebaruSan Apr 22, 2018

Choose a reason for hiding this comment

HebaruSan Apr 22, 2018

Choose a reason for hiding this comment

DasSkelett Apr 24, 2018

Choose a reason for hiding this comment

HebaruSan Apr 24, 2018

Choose a reason for hiding this comment

DasSkelett Apr 24, 2018

Choose a reason for hiding this comment

DasSkelett commented Apr 24, 2018 • edited Loading

DasSkelett commented Apr 24, 2018

politas commented Apr 25, 2018

DasSkelett commented Apr 25, 2018

DasSkelett commented Apr 26, 2018 • edited Loading

politas commented Apr 27, 2018

politas commented Oct 7, 2018

DasSkelett commented Dec 5, 2018

DasSkelett commented Apr 22, 2018 •

edited

Loading

DasSkelett commented Apr 24, 2018 •

edited

Loading

DasSkelett commented Apr 26, 2018 •

edited

Loading