Multiple dictionary dialogs can now be opened simultaneously if the dictionaries are of different types#12967
Conversation
…naries are of different types.
|
Have you tested updating and saving a dialog, with multiple dialogs open, and then reopening the dialog to confirm the changes were saved?
This can be done by using the Abstract Base Class pattern (other docs). |
Assuming this is going to be merged in 2021.3 we cannot do that, as there are some add-ons (I know about Application dictionary but there are probably others) which would be broken as a result of this change. |
Thanks. I have just done this test. I will update the initial description accordingly.
So if I understand correctly:
Actually, at point 2, if I take example on the class At last, as indicated by @lukaszgo1, we cannot make existing classes and methods abstract without breaking the compatibility. Thus an option could be to merge this PR as is in 2021.3 (beta) to have the fix at least for the majority of dictionaries (NVDA default, NVDA voice, NVDA temporary and Emoticons add-on which subclasses the DictionaryDialog). The bug wouldn't be resolved only in case two add-ons use |
I agree, this is probably the best approach. Making sure that |
seanbudd
left a comment
There was a problem hiding this comment.
Thanks @CyrilleB79, I've confirmed your manual testing.
@feerrenrut - can this be included in 2021.3 beta1 ?
|
Yes, happy for this to be included. As far as I can tell it maintains backwards compatibility, is low risk, and fixes a regression in the release. |
Link to issue number: Follow-up of #12967 Summary of the issue: Since the merge of #12800, trying to re-open an already opened dialog allows to focus it rather than issuing an error message box. The dialog is refocused according to its class, i.e. trying to open a dialog of the same class rather refocus the first already opened dialog of the same class. But with #12800, it was not possible to open 2 dictionaries of different type (e.g. speech and default). This was fixed with #12967 by subclassing DictionaryDialog. However, addons which directly use DictionaryDialog will still have the issue of opening 2 dictionaries at once i.e. #5383. Description of how this pull request fixes the issue: force DictionaryDialog to be subclassed by making the initializer and abstract method
Opened against beta branch since it is a follow-up of #12800.
Cc @seanbudd
Link to issue number:
Fix-up of #12800.
Described in #12800 (comment), paragraph "Test almost successful", "Test 2".
Summary of the issue:
Since the merge of #12800, trying to re-open an already opened dialog allows to focus it rather than issuing an error as done previously. However, in the case of dictionaries, if you try to open the voice dictionary while the default is already open will lead to focus the default dictionary rather than opening the voice dictionary dialog;
Description of how this pull request fixes the issue:
Since the check of an existing opened dialog is made on the class of the dialog, I have sub-classed
DictionaryDialogfor the three types of dictionaries to make them distinct one from another.Testing strategy:
Test 1:
Test 2:
Known issues with pull request:
Maybe it would be interesting to forbid to instance an object of class
DictionaryDialogdirectly, i.e. force subclassing.I do not know however how it can be acheived and do not even know if it is pythonic.
Comments are welcome.
Change log entries:
Not needed if merged along with #12800 in 2021.3.
Code Review Checklist: