GUI: Standard dialog separated dismiss buttons and borders (#6468)#10159
Conversation
|
PR introduces Flake8 errors 😲 See test results for Failed build of commit 2c4c0d850e |
|
CC @nishimotz for confirmation please. Thanks.
|
…ator. This impacts all derived class of `SettingsDialogs`, such as `MultiCategorySettingsDialog`, `SynthesizerSelectionDialog`, `DictionaryDialog`, `BrailleDisplaySelectionDialog`, `SpeechSymbolsDialog`, `InputGesturesDialog`.
b1f0a11 to
7f5ec02
Compare
|
Are dismiss buttons really a standard? Many go away using Escape.
|
By "standard" I mean "typical Windows look and feel as tailored by tools from Microsoft and exhibited by their software". By the way, adopting a standard look and feel is not only for aesthetics: It helps users with low vision to easily rely on common patterns and visual cues. |
|
tested nvda_snapshot_pr10159-18482,8a96fb7c.exe and confirmed it works as expected regarding visual separators. |
|
cc @feerrenrut |
| buttonSizer.addButton(self, label=_("OK"), id=wx.ID_OK) | ||
| # Translators: The cancel button on a NVDA dialog. This button will discard any changes and dismiss the dialog. | ||
| buttonSizer.addButton(self, label=_("Cancel"), id=wx.ID_CANCEL) | ||
| buttons = wx.OK | wx.CANCEL |
There was a problem hiding this comment.
The downside with this approach (providing ID's for the buttons) is that we don't get to control the translations for these buttons. I believe there are languages where these buttons will not be translated. If we provide the label, we are able to have them translated.
We have a mixture of sources for these buttons (created via ID or created with a translated label).
There was a problem hiding this comment.
You are definitely right about translation here.
I took the ID approach because of the current mixture you mention.
Actually, every single time we use a standard message box with Yes/No/Apply/OK/Cancel or even the standard file picker or folder picker, these currently appear in the language of the OS, not the language of NVDA.
It is pretty common practice, and I guess native speakers of languages in which Windows itself is not translated are quite used to this.
To fully circumvent this situation, we would have to:
- Re-implement every single standard message box we emit. A lot of work, but doable.
- Step back from taking advantage of the
wx.Dialog.CreateSeparatedButtonSizerfacility. - Implement custom file and folder pickers. This one is much more tricky if you want - and should - provide the appropriate look and feel for every single version of Windows NVDA supports.
Thus, IMHO, the easiest and safest path is to use IDs instead of custom translations everywhere applicable, be it only to ensure coherence among the different part of the GUI.
| elif isinstance(buttons, (wx.Sizer, wx.Button)): | ||
| toAdd = buttons | ||
| elif isinstance(buttons, int): | ||
| toAdd = self._parent.CreateButtonSizer(buttons) |
There was a problem hiding this comment.
As per my other comment about translations of the button labels, perhaps it would be better to construct the buttons with our own translated labels here?
|
Merged latest master and aligned PR #10076 to follow the same paradigm. |
|
I think for now, it would be best to leave the translated button labels, since it is not necessary for achieving the main goal of this PR. I understand that there is a mix of translated and not buttons at the moment, but what to do about that is a decision on its own. Working out a strategy for getting a uniform approach to button translation can be done in another PR. One question to answer on a new PR is if there is a way to make WX use use a specified locale rather than the OS one. |
feerrenrut
left a comment
There was a problem hiding this comment.
Overall I'm not too sure about the look for dialogs like the settings dialog. When there is already a groupbox outline or a panel outline the extra horizontal line makes it look busy. I'll accept and merge this change for now, since I don't think it is a big harm, but we may wish to re-visit those dialogs specifically.
Link to issue number:
Fixes #6468
Summary of the issue:
Several dialogs in NVDA lack to visually present dismiss buttons with standardized separator and borders.
Description of how this pull request fixes the issue:
gui.guiHelper.BoxSizerHelper.addDialogDismissButtonsfor use by cases other than message / single input dialogs.Testing performed:
Accessed all the above-mentioned dialogs to check their proper look and behavior.
Known issues with pull request:
Change log entry:
Section: Changes for developers
The GUI Helper's
BoxSizerHelper.addDialogDismissButtonssupports a new "separated" keyword argument, for add a standard horizontal separator on dialogs other than messages and single inputs.