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
Document formatting dialog (issue #6348) #6402
Conversation
@jcsteh, would you mind reviewing this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
source/gui/settingsDialogs.py.orig
seems to have been added (unless GitHub is doing something odd?) and should be removed.
Thanks!
@@ -254,7 +255,7 @@ def addItem(self, item): | |||
if isinstance(item, LabeledControlHelper): | |||
raise NotImplementedError("Use addLabeledControl instead") | |||
|
|||
if isinstance(toAdd, wx.StaticBoxSizer): | |||
if isinstance(toAdd, (wx.StaticBoxSizer, scrolledpanel.ScrolledPanel)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar query in #6287: should this be an elif
?
@@ -1056,75 +1057,109 @@ class DocumentFormattingDialog(SettingsDialog): | |||
title = _("Document Formatting") | |||
|
|||
def makeSettings(self, settingsSizer): | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Extraneous blank line?
# Translators: This is the label for a group of document formatting options in the | ||
# document formatting settings dialog | ||
otherGroupText = _("Elements") | ||
otherGroup = guiHelper.BoxSizerHelper(scrolledItems, sizer=wx.StaticBoxSizer(wx.StaticBox(scrolledItems, label=otherGroupText), wx.VERTICAL)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be renamed to elementsGroup
given the label change to Elements
.
|
||
sHelper.addItem(scrolledItems) | ||
scrolledItems.Layout() | ||
scrolledItems.SetupScrolling() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is also called above when creating the panel. Are both these calls necessary?
scrolledItems.SetupScrolling() | ||
self.scrolledItems = scrolledItems | ||
checkboxItems = guiHelper.BoxSizerHelper(scrolledItems, orientation=wx.VERTICAL) | ||
checkboxItemsBorder = wx.BoxSizer(wx.VERTICAL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
checkboxItems
does mostly contain check boxes, but the line indentation setting is a choice control, not a check box. Therefore, I think this should be renamed, as I'm concerned it might be misleading in future. Maybe we could call this scrolledItems
and rename the panel to scrolledPanel
?
- Landmarks | ||
- Frames | ||
- When something is clickable | ||
The document formatting options are organised into groups, you can configure reporting of: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please split this into two sentences:
The document formatting options are organised into groups.
You can configure reporting of:
The document formatting settings dialog is restructured so that items can no longer "disappear" off the screen. The options are now in a scrolling section, grouped for clarity (paritcularly for new users / sighted users). A dialog description has been added, for further clarity.
Updated the user guide, and some small wording changes.
d4879e2
to
6278636
Compare
I like these changes, however when tabbing through the dialog, I think it is unclear that "Report formatting changes after the cursor" is an option that doesn't belong to any grouping. That is, when you tab through the elements grouping and reach the after the cursor option, no other grouping is announced. So, people might possibly think that they're still in the elements grouping. |
That's true when leaving any grouping and we don't really have a good
solution for it. Saying "out of grouping" would be annoying verbosity in
many cases. Putting a labelled grouping around that control just to
mitigate this issue is not feasible because it's otherwise redundant.
It's worth noting that if you shift+tab, you'll hear that you entered the
Elements grouping again. Also, I'd argue the option name is different
enough that one might at least suspect it's not related to Elements.
|
Agreed. Another possibility would be putting back the option as first in the tab order, but, it is not that important really. I came across it and thought, let's report this in case you guys would like to account for it. |
We moved it to the end of the dialog because it's a rather obscure option
(and to be honest, I'd prefer we didn't have it at all, but that's another
issue entirely). I appreciate you raising the concern. It was certainly
worth raising and discussing, but I don't think it will really hurt anyone
practically speaking, whereas the potential "solutions" will detriment
other use cases.
|
Merges in branches - 'i6101_SymbolsListCtrl' - 'i6348-documentFormattingDialog' - 'fixGuiAlignmentIssues' Adjust the symbols list. See PR #6339 Put the 'symbol' label above the list and have the list itself take up the full width of the dialog. Columns are spaced automatically, with one nominated to take up any extra available room. The first column was selected to expand more, as the information in the other columns can be viewed in the "change symbol" group. Fixes #6101 Restructure the document formatting dialog. See PR #6402 The document formatting settings dialog is restructured so that items can no longer "disappear" off the screen. The options are now in a scrolling section, grouped for clarity (paritcularly for new users / sighted users). A dialog description has been added, for further clarity. Fixes #6348 Fix Gui Alignment issues. See PR #6287 Fixes up various alignment and padding problems in the GUI. Introduces a new helper to abstract some of these details. Fixes #6317 (Gestures treeview does not look like a treeview) Fixes #5548 (Text control to specify the path when creating a portable copy is quite narrow) Fixes #6342 (Spacing between labels and combo boxes) Fixes #6343 (Inconsistency with vertical spacing between checkbox/label options) Fixes #6349 (input gestures dialog tree view too small)
This has been merged into master with commit: 87ed4d1 |
For PR #6287 - Various padding and alignment issues have been resolved. (#6317, #5548, #6342, #6343, #6349) For PR #6402 - The document formatting dialog has been adjusted so that the contents scrolls. (#6348) For PR #6339 - Adjusted the layout of the symbols pronunciation dialog so the full width of the dialog is used for the symbols list. (#6101)
This address the issue that the document formatting options dialog can be too long to fit on some screens. To resolve this the formatting options have been put into a scrollable section, the options have been grouped by similarity, and some dialog text has been added to explain the dialog better.
Fixes #6348