issue 1554

[packlnd]

First deck name is shown twice when deleting two decks. Fixed by
removing dialog after it is closed instead of just dismiss.

[packlnd]

I tested the code and it works. In a similar issue (1371) the information that should be updated is in onPrepareDialog but it does not update because the dialog is already built. That issue would also be fixed by using removeDialog. Should removeDialog be avoided even if it works?

[flerda]

I notice now that someone tried already to fix it with onPrepareDialog.
The problem is that the condition there on whether the collection is open is incorrect, it needs to be negated.

As for your fix, remove dialog in the code is called only if the progress dialog is shown and it is not shown if you press cancel.

I tried it and it does not seem to work for me at least.
I did the following:

1. Create d1
2. Create d2
3. Long press on d1
4. Select delete deck
5. Press cancel
6. Long press on d2
7. Select delete deck

At this point the dialog shows d1 instead of d2.

The simpler fix is probably:
https://github.com/ntsp/Anki-Android/commit/9578b083b6c7f949baa8eff6fc0ba4eac4faf5ec

What do you think?

[flerda]

In terms of using removeDialog itself, removeDialog is less efficient as it means the dialog needs to be recreated instead of just being reused. It is usually better to instead update the dialog as necessary in onPrepareDialog.

In this particular case, we could add removeDialog both to the case of pressing ok and cancel, but since onPrepareDialog should already be doing the right thing, there is no need.

[packlnd]

Sorry, I missed the cancel part. I still had issue 1371 in mind where onPrepareDialog doesn't seem work. Changing the if-statement is, as you said, a simpler and more clean fix.

[flerda]

No problem. I probably would have missed it myself if I were doing the same fix maybe.
This is the reason while I suggested having code reviews.
Having too people look at the code will help us make AnkiDroid's code better!

I will close this pull request and ask ntsp to continue with his.