Skip to content
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

Symbol list is too small in NVDA Symbol / Punctuation dialog. #6101

Closed
Qchristensen opened this issue Jun 22, 2016 · 12 comments
Closed

Symbol list is too small in NVDA Symbol / Punctuation dialog. #6101

Qchristensen opened this issue Jun 22, 2016 · 12 comments
Assignees
Labels
Addon/management In NVDA management of addons by users. component/NVDA-GUI p3 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority
Milestone

Comments

@Qchristensen
Copy link
Member

Running Windows 10, NVDA 2016.2.1, latest next or back to 2015.4 (earliest build I have handy) all work the same.

On the Symbol / Punctuation dialog the symbol list takes up about 60% of the width of the dialog yet only displays the "symbol" and "replacement" fields. The "level" field is partially displayed, but the "Preserve" field is not displayed at all. Neither the symbol or preserve field are wide enough as it is to display their contents visually.

Proposed solution:
Put the "Symbol" label above the list and have the list itself take up the full width (minus a small margin) of the dialog.

@josephsl
Copy link
Collaborator

Hi,

Confirmed - in my case, "preserve" header isn't displayed (resolution is 1366x768).

Thanks.

@josephsl josephsl added Addon/management In NVDA management of addons by users. component/NVDA-GUI labels Jun 22, 2016
@josephsl
Copy link
Collaborator

josephsl commented Jun 22, 2016

Hi,

Looks like screen real estate calculation is wrong - assumes a small list when it isn't.

Technical: When you define wx.ListCtrl, you can ask it to let you specify how big the control should be. 360 by 350 pixels isn't going to be enough (no wonder ends of some columns were cut off). This was observed in both English and Korean. It seems the likely compromise would be 480 by 350.

@nishimotz: Can you give us some advice on this?

Thanks.

@jcsteh
Copy link
Contributor

jcsteh commented Jun 22, 2016

Ideally, everything would be proportional, but wxListCtrl doesn't allow you to specify proportional widths for columns.

@feerrenrut, could you please take a look at this?

@nvaccessAuto nvaccessAuto added the p3 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority label Jul 5, 2016
@feerrenrut
Copy link
Contributor

Firstly, to fix the issue of the symbol list, I propose the following:

  • put the "symbol" label above the symbol list (to free up more width)
  • make the dialog re-sizeable, and the list expand when the dialog is resized.

Secondly, while here there are a few other issues with this dialog I would like to tidy up:

  • There is currently no obvious way to "apply" changes to a symbol entry without exiting the dialog. This makes it hard to edit several entries.
  • Pressing the Add button actually applies changes as a side effect, this is not intuitive so I would consider this a bug.
  • Because the Add / Remove buttons follow the changes section, they initially seem to be related to that section.
  • Pressing the Add button only allows you to enter the symbol, but not the other details.

To fix these I propose:

  • Add a new button "Change" that launches a new dialog for changing the entry.
  • The Add button also allows you to edit the other details for the entry.
  • Remove the change section (use the add button instead).

@nishimotz
Copy link
Contributor

sorry for slow response.
Aside from "Apply" button discussion, work around the punctuation dialog is as follows,
based on current master branch.

This dialog is affected by the contents of dictionary of current system (or voice) language, and Windows UI language, and Windows versions.

As far as I understand, @Qchristensen suggests all the list column items is displayed without horizontal scroll, so this fix gives the same width for all columns, rather than specifying the width numbers by pixels.

--- a/source/gui/settingsDialogs.py
+++ b/source/gui/settingsDialogs.py
@@ -1598,25 +1598,25 @@ class SpeechSymbolsDialog(SettingsDialog):
                symbols = self.symbols = [copy.copy(symbol) for symbol in self.symbolProcessor.computedSymbols.itervalues()]
                self.pendingRemovals = {}

-               sizer = wx.BoxSizer(wx.HORIZONTAL)
+               sizer = wx.BoxSizer(wx.VERTICAL)
                # Translators: The label for symbols list in symbol pronunciation dialog.
                sizer.Add(wx.StaticText(self, wx.ID_ANY, _("&Symbols")))
-               self.symbolsList = wx.ListCtrl(self, wx.ID_ANY, style=wx.LC_REPORT | wx.LC_SINGLE_SEL, size=(360, 350))
+               self.symbolsList = wx.ListCtrl(self, wx.ID_ANY, style=wx.LC_REPORT | wx.LC_SINGLE_SEL)
                # Translators: The label for a column in symbols list used to identify a symbol.
-               self.symbolsList.InsertColumn(0, _("Symbol"), width=150)
-               self.symbolsList.InsertColumn(1, _("Replacement"), width=150)
+               self.symbolsList.InsertColumn(0, _("Symbol"))
+               self.symbolsList.InsertColumn(1, _("Replacement"))
                # Translators: The label for a column in symbols list used to identify a symbol's speech level (either none, some, most, all or character).
-               self.symbolsList.InsertColumn(2, _("Level"), width=60)
+               self.symbolsList.InsertColumn(2, _("Level"))
                # Translators: The label for a column in symbols list which specifies when the actual symbol will be sent to the synthesizer (preserved).
                # See the "Punctuation/Symbol Pronunciation" section of the User Guide for details.
-               self.symbolsList.InsertColumn(3, _("Preserve"), width=60)
+               self.symbolsList.InsertColumn(3, _("Preserve"))
                for symbol in symbols:
                        item = self.symbolsList.Append((symbol.displayName,))
                        self.updateListItem(item, symbol)
                self.symbolsList.Bind(wx.EVT_LIST_ITEM_FOCUSED, self.onListItemFocused)
                self.symbolsList.Bind(wx.EVT_CHAR, self.onListChar)
-               sizer.Add(self.symbolsList)
-               settingsSizer.Add(sizer)
+               sizer.Add(self.symbolsList, flag=wx.EXPAND)
+               settingsSizer.Add(sizer, flag=wx.EXPAND)

                # Translators: The label for the edit field in symbol pronunciation dialog to change the pronunciation of a symbol.
                changeSizer = wx.StaticBoxSizer(wx.StaticBox(self, wx.ID_ANY, _("Change symbol")), wx.VERTICAL)

image

nishimotz added a commit to nishimotz/nvda that referenced this issue Jul 15, 2016
… itself take up the full width of the dialog. Column items should have the same width, rather than specifying the widths by pixels.
@feerrenrut
Copy link
Contributor

Thanks @nishimotz, this dialog is going to get a bit of a makeover while addressing this ticket. I have described the changes currently in progress in the earlier comment.

As part of the changes, I am doing what you suggest in #6178.

feerrenrut added a commit that referenced this issue Jul 19, 2016
Fixes: #6101

This change widens the symbol list, and removes the symbol entry editing
option. There are now three buttons; Add, Change, Edit.
This has the following advantages:
- Able to specify all fields when adding a symbol
- Able to modify the entries from the change dialog

The spacing around the UI elements has been fixed and made more
consistent.
@jcsteh
Copy link
Contributor

jcsteh commented Jul 20, 2016 via email

@feerrenrut
Copy link
Contributor

Yep, it should be "Add, Change and Remove". I have updated this on the pull request.

I don't agree with "quite a bit slower". It is indeed one more step than what it could be. But compared to the previous implementation which required you to leave the dialog for each edit, it is quite a bit faster (if you are editing several entries). Firstly there are two steps less since you no longer have to re-open the symbol pronunciation dialog again after each edit. Secondly, now the entry you are editing is still highlighted, so you haven't lost your place in the list.

In terms of performance, I would argue that unless there is some serious user noticeable lag or slowdown its not a concern. This would be a one time performance hit, and not an ongoing degradation. Also I don't believe I have changed the code in any way that should make the performance worse, do you have a particular use-case that I should be thinking about?

@feerrenrut
Copy link
Contributor

Have just had a play with this again. It seems like my understanding is based on having used the mouse to move focus. When using the keyboard to move between the symbol list and the replacement text control, the fields update correctly, and the dialog does not need to be exited. However if the mouse is used, these fields do not update correctly.

If you are unhappy with the proposed ux, perhaps we could do the following to resolve your concerns:

  • Keep the change button, but focus the replacement field when it is launched.
  • This means that that the user will have the following steps:
    • Select an item in the list
    • Use alt+c to open the change dialog, nvda reads the title as "Change symbol: " and focus is set to replacement
    • The replacement and other fields can be edited in the same way as current implementation.
    • Enter commits the changes.
  • This results in the same amount of keyboard input as the current implementation, except that alt+c is used instead of alt+r. And enter is used to commit instead of tab or shift+tab.

I think the advantages of this are

  • The same number of key presses are required.
  • More intuitive visually.
  • Uses the same dialog for add / change which allows all fields to be set when adding.

@jcsteh
Copy link
Contributor

jcsteh commented Jul 21, 2016 via email

@feerrenrut
Copy link
Contributor

I have taken a look at the Firefox bookmarks, I think we can work something out. There seems to be a misunderstanding about the performance issue. I am not altering how the changes are applied. In my pull request changes are only applied once the symbol pronunciation dialog Ok button is pushed. When referring to "apply", I was trying to say that the list view is updated with the changes, and the list would be committed/applied/permanently saved when the symbol pronunciation dialog Ok button was pushed.

I'll put this issue on hold until we can discuss it more easily.

@feerrenrut
Copy link
Contributor

After discussing some of the issues around updating this dialog, we have decided to fix the other concerns raised here, separately from this issue. As such PR #6189 has been closed.

feerrenrut added a commit that referenced this issue Sep 9, 2016
re issue #6101
Merge branch 'i6101_SymbolsListCtrl' into next
feerrenrut added a commit that referenced this issue Sep 29, 2016
Issue: #6101
Extends PR: #6178

Merge branch 'i6101_SymbolsListCtrl' into next

Conflicts:
	source/gui/settingsDialogs.py
@nvaccessAuto nvaccessAuto added this to the 2016.4 milestone Oct 13, 2016
feerrenrut added a commit that referenced this issue Oct 13, 2016
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Addon/management In NVDA management of addons by users. component/NVDA-GUI p3 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants