Add filtering functionality to the symbols list #8790

LeonarddeR · 2018-09-27T12:01:00Z

Link to issue number:

Summary of the issue:

Now that emoji's have been introduced in NVDA symbol dictionaries, the list of symbols was getting pretty big. This also introduced a major lag when opening the speech symbols dialog.

Description of how this pull request fixes the issue:

Added a filter edit box to the speech symbols dialog. @feerrenrut: your opinion about the visuals of this edit box are greatly appreciated. Feel free to push directly to this branch if you need to change things visually.
Converted the list control to a list control with the wx.LC_VIRTUAL style. I'm really exited about the major performance boost this gives when loading the dialog for the first time, as well as when filtering. It is just really fast now!
Added some functionality to gui.nvdaControls.AutoWidthColumnListCtrl to support getting item text from a data storage without the requirement of subclassing the list control. It is now also possible to forcefully send a wx.EVT_LIST_ITEM_FOCUSED to the control

Testing performed:

Filtering, adding and removing items, closing the dialog and reopening it again quickly after that.

Known issues with pull request:

The visuals might not be ideal yet, though guiHelper is really amazing in creating stuf that just looks good.
We should probably wait for Allow add-ons to supply version compatibility information in manifests, and use this information to disable incompatible add-ons #8006 to be in master and apply scaling support to this dialog as well while at it.
This does not yet apply to speech dictionaries. I think it should at some point, but that won't be part of this pr.

Change log entry:

New features
- You can now filter symbols in the punctuation/symbol pronunciation dialog, similar to how filtering works in the elements list and input gestures dialog. (Punctuation dialog needs a way to search symbols #5761)
Bug fixes
- Loading the punctuation/symbol pronunciation dialog is now much faster when using symbol dictionaries containing over 1000 entries.

dnz3d4c · 2018-09-27T14:24:42Z

Korean symbol dictionary has many items. Can you expect to see speed improvements when navigating the list box with this patch?

LeonarddeR · 2018-09-27T14:40:45Z

Wow! I'm coming across like 31000 symbols here, taking like 20 seconds to load with the current Alpha version of NVDA. This pr reduces that to 2 seconds.

dnz3d4c · 2018-09-27T14:51:15Z

Thanks.

Wow! I'm coming across like 31000 symbols here, taking like 20 seconds to load with the current Alpha version of NVDA. This pr reduces that to 2 seconds.

dpy013 · 2018-09-27T22:58:02Z

@hi leonardder I am the maintainer of NVDA Simplified Chinese. I found that NVDA can't generate Simplified Chinese emoticon dictionary using the scons command.

josephsl · 2018-09-27T23:36:13Z

Hi, that’ll be because it isn’t clear at the first glance which XML file is for which Chinese variant. This is something that should be addressed sometime in October. Thanks.

dpy013 · 2018-09-28T00:03:27Z

thank josephsl

LeonarddeR · 2018-09-28T05:56:34Z

I must have missed the Chinese annotations.

@dingpengyu: Could you please open a new issue for this? I'm not yet known with the various differences in the Chinese language, so it is quite difficult to decided what would be the best assignments. In NVDA, we have the zh-cn, zh-hk and zh-tw locales.

josephsl · 2018-09-28T06:02:09Z

Hi, I can assist with this, as I work with Asian languages in a regular basis. When creating this new issue, please at mention me (CC me). Thanks.

dpy013 · 2018-09-28T12:09:31Z

Hi, thanks to josephsl's reply, this is the issue address:
NVDA cannot generate Simplified Chinese emoticon dictionary using the scons command · Issue #8799
#8799

LeonarddeR · 2018-12-06T12:21:00Z

@feerrenrut: I've removed the blocked label since #8006 is merged. However, I'm not yet sure what to do to make this dialog scale correctly, other than setting resizable to True. The size of the list control and its columns is undefined.

feerrenrut

When filtering, the list does not update. The entries only update as they gain focus. Visually this is quite confusing.

Because this dialog is a SettingsDialog Then there is no need to do anything to get the DPI scaling, except, if you specify a size manually, you must pass it through self.scaleSize().

I think this dialog could be wider, because the text in the replacement column is often long enough to exceed the width of the column. The replacement field (once populated) often isn't wide enough. There is plenty of space to widen this field, and the dialog itself isn't very wide either.

Some emoji don't seem to be supported by the font. Visually they are just squares. Not a blocking issue right now, since I believe this is already the case. An issue should be created for this.

I can take a look at these points.

source/gui/settingsDialogs.py

LeonarddeR · 2019-01-18T13:11:14Z

When filtering, the list does not update. The entries only update as they gain focus. Visually this is quite confusing.

Ugh, might this be because the list is virtual? Does this also mean that there's only one item visible at a time or something like that?

source/gui/nvdaControls.py

feerrenrut · 2019-01-18T14:28:10Z

Does this also mean that there's only one item visible at a time or something like that?

No, the list looks correct. After entering a filter the list does not seem to change, until you move focus to the first item, and it suddenly changes. Moving to the next item causes that one to change.

Otherwise the items only change when they receive focus. It appears (visually) as if the list has not changed, until you start to scroll or select items.

…em text at construction time of an AutoWidthColumnListCtrl. A weak reference to the callable is saved on the control. If we wouldn't, closing the punctuation dialog and reopening it would result in a multi instance error. The extensionPoints module provides functionality to deal with instance method weakrefs. Updated that code to allow weak references without an unregister callback.

LeonarddeR · 2019-01-19T12:07:19Z

@feerrenrut: Thanks for fixing the list updating issue. Would be great if you can fix the replacement column width as well, since it would really help to do that with a pair of eyes.

feerrenrut · 2019-01-21T17:57:31Z

Yep, I'll take a look at the widths in the dialog tomorrow.

source/gui/guiHelper.py

feerrenrut

I've looked through this again and I'm happy for it to be merged. @LeonarddeR Are you happy with the latest changes?

LeonarddeR

I've got some comments. Also, you could consider adding a convenience wrapper for the new LabeledTextControl to BoxSizerHelper.

source/gui/guiHelper.py

source/gui/settingsDialogs.py

dpy013 · 2019-04-04T04:15:31Z

hi
Is there a developer review for this PR? Seeing that it is already approved

This was a clumsy attempt to be able to support making the textCtrls resize with the dialog. The real problem is that GuiHelper is not felixible enough to do it in a simple way. For now, we will just set a reasonable starting size.

The parameter was deceptively named "index" where it affected columns numbers, first column is 1 not 0.

Reduce nesting, clarify some comments.

feerrenrut · 2019-04-11T21:29:44Z

@LeonarddeR I've made a few changes here, if you are happy with this I think it is ready to be merged.

…y to focus a nonexistent item. Also properly disable the remove button when disabling the controls

LeonarddeR · 2019-04-13T11:23:01Z

@feerrenrut: I'm ok with your changes and did a small round of tests.

Found and fixed the following:

Add a new symbol, for example called test
Filter on test
Remove the test symbol

What happened is that the list of symbols was empty, yet the old logic tried to select a new symbol. Removing a symbol didn't check for the item count properly. Now it does.

feerrenrut

Looks good. I'll merge this in.

New features: - You can now filter symbols in the punctuation/symbol pronunciation dialog, similar to how filtering works in the elements list and input gestures dialog. (#5761) Bug fixes: - Loading the punctuation/symbol pronunciation dialog is now much faster when using symbol dictionaries containing over 1000 entries. (#8790)

…8790, nvaccess#10216)

The replacement control was added to the wrong sizer. Issues: (#8790, #10216)

…0216) (#10218)

Add filtering functionality to the symbols list

2cbdc01

LeonarddeR added the component/NVDA-GUI label Sep 27, 2018

LeonarddeR requested a review from feerrenrut September 27, 2018 12:01

Leonard de Ruijter added 3 commits September 27, 2018 16:43

Removed extra line at end of nvdaControls which was accidentally added

5c74b48

Default the filter to an empty string instead of None

ebcc9bf

Updated user guide

4692c5e

This comment has been minimized.

Sign in to view

LeonarddeR mentioned this pull request Sep 28, 2018

A filter edit field for the speech dictionaries #2478

Open

LeonarddeR added the blocked label Oct 5, 2018

LeonarddeR removed the blocked label Dec 6, 2018

LeonarddeR mentioned this pull request Jan 7, 2019

Punctuation/Symbol Pronunciation Dialog: Allow Filtering for Characters #9128

Closed

feerrenrut suggested changes Jan 18, 2019

View reviewed changes

source/gui/settingsDialogs.py Outdated Show resolved Hide resolved

source/gui/settingsDialogs.py Outdated Show resolved Hide resolved

feerrenrut reviewed Jan 18, 2019

View reviewed changes

source/gui/nvdaControls.py Outdated Show resolved Hide resolved

feerrenrut and others added 4 commits January 18, 2019 16:13

Refresh visible items after filter is changed.

1ef9414

Otherwise the items only change when they receive focus. It appears (visually) as if the list has not changed, until you start to scroll or select items.

Merge remote-tracking branch 'origin/master' into filterSymbols

b8fd53f

Review actions

b26f2a0

Leonard de Ruijter and others added 4 commits January 21, 2019 19:39

Review action, revert changes to extensionPoints

7719e52

Tidy up list filtering.

9e4703e

Fix resizing of the symbol pronunciation dialog.

8eb259e

Merge remote-tracking branch 'origin/master' into addFilteringPR8790

892f7ac

LeonarddeR commented Feb 13, 2019

View reviewed changes

source/gui/guiHelper.py Show resolved Hide resolved

feerrenrut approved these changes Feb 14, 2019

View reviewed changes

LeonarddeR commented Feb 14, 2019

View reviewed changes

source/gui/guiHelper.py Outdated Show resolved Hide resolved

source/gui/guiHelper.py Outdated Show resolved Hide resolved

LeonarddeR commented Feb 16, 2019

View reviewed changes

source/gui/settingsDialogs.py Outdated Show resolved Hide resolved

feerrenrut added 3 commits April 9, 2019 21:49

Revert new control

3535718

This was a clumsy attempt to be able to support making the textCtrls resize with the dialog. The real problem is that GuiHelper is not felixible enough to do it in a simple way. For now, we will just set a reasonable starting size.

rename misleading parameter

ba72ecd

The parameter was deceptively named "index" where it affected columns numbers, first column is 1 not 0.

Simplify selection restoration logic.

c12a0b7

Reduce nesting, clarify some comments.

Leonard de Ruijter added 2 commits April 13, 2019 13:02

Merge remote-tracking branch 'origin/master' into filterSymbols

1ddb2d9

Removing the last item of a filtered list, make sure that we don't tr…

4f0959d

…y to focus a nonexistent item. Also properly disable the remove button when disabling the controls

LeonarddeR requested a review from feerrenrut April 13, 2019 11:21

feerrenrut approved these changes Apr 23, 2019

View reviewed changes

feerrenrut merged commit 07e48a9 into nvaccess:master Apr 23, 2019

nvaccessAuto added this to the 2019.2 milestone Apr 23, 2019

JulienCochuyt added a commit to accessolutions/nvda that referenced this pull request Sep 15, 2019

Fix up of: Add filtering functionality to the symbols list (nvaccess#…

6e8227d

…8790, nvaccess#10216)

JulienCochuyt mentioned this pull request Sep 15, 2019

Symbol replacement edit not visible in punctuation / symbol pronunciation dialog #10216

Closed

JulienCochuyt added a commit to accessolutions/nvda that referenced this pull request Sep 15, 2019

Fix up of: Add filtering functionality to the symbols list (nvaccess#…

0eef70c

…8790, nvaccess#10216)

This was referenced Sep 15, 2019

Fix up of: Add filtering functionality to the symbols list (#8790, #10216) - Py2 #10218

Merged

Fix up of: Add filtering functionality to the symbols list (#8790, #10216) - Py3 #10217

Merged

feerrenrut pushed a commit that referenced this pull request Sep 16, 2019

Fix up of: Add filtering functionality to the symbols list (PR #10217)

30f6cec

The replacement control was added to the wrong sizer. Issues: (#8790, #10216)

michaelDCurran pushed a commit that referenced this pull request Sep 17, 2019

Fix up of: Add filtering functionality to the symbols list (#8790, #1…

96504b8

…0216) (#10218)

LeonarddeR mentioned this pull request Oct 26, 2019

Add filtering functionality to speech dictionaries #10428

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add filtering functionality to the symbols list #8790

Add filtering functionality to the symbols list #8790

LeonarddeR commented Sep 27, 2018 •

edited

dnz3d4c commented Sep 27, 2018 •

edited

LeonarddeR commented Sep 27, 2018

dnz3d4c commented Sep 27, 2018

dpy013 commented Sep 27, 2018

josephsl commented Sep 27, 2018 via email •

edited by feerrenrut

dpy013 commented Sep 28, 2018

LeonarddeR commented Sep 28, 2018

josephsl commented Sep 28, 2018 via email •

edited by feerrenrut

This comment has been minimized.

dpy013 commented Sep 28, 2018

LeonarddeR commented Dec 6, 2018

feerrenrut left a comment

LeonarddeR commented Jan 18, 2019

feerrenrut commented Jan 18, 2019

LeonarddeR commented Jan 19, 2019

feerrenrut commented Jan 21, 2019

feerrenrut left a comment

LeonarddeR left a comment

dpy013 commented Apr 4, 2019

feerrenrut commented Apr 11, 2019

LeonarddeR commented Apr 13, 2019

feerrenrut left a comment

Add filtering functionality to the symbols list #8790

Add filtering functionality to the symbols list #8790

Conversation

LeonarddeR commented Sep 27, 2018 • edited

Link to issue number:

Summary of the issue:

Description of how this pull request fixes the issue:

Testing performed:

Known issues with pull request:

Change log entry:

dnz3d4c commented Sep 27, 2018 • edited

LeonarddeR commented Sep 27, 2018

dnz3d4c commented Sep 27, 2018

dpy013 commented Sep 27, 2018

josephsl commented Sep 27, 2018 via email • edited by feerrenrut

dpy013 commented Sep 28, 2018

LeonarddeR commented Sep 28, 2018

josephsl commented Sep 28, 2018 via email • edited by feerrenrut

This comment has been minimized.

dpy013 commented Sep 28, 2018

LeonarddeR commented Dec 6, 2018

feerrenrut left a comment

Choose a reason for hiding this comment

LeonarddeR commented Jan 18, 2019

feerrenrut commented Jan 18, 2019

LeonarddeR commented Jan 19, 2019

feerrenrut commented Jan 21, 2019

feerrenrut left a comment

Choose a reason for hiding this comment

LeonarddeR left a comment

Choose a reason for hiding this comment

dpy013 commented Apr 4, 2019

feerrenrut commented Apr 11, 2019

LeonarddeR commented Apr 13, 2019

feerrenrut left a comment

Choose a reason for hiding this comment

LeonarddeR commented Sep 27, 2018 •

edited

dnz3d4c commented Sep 27, 2018 •

edited

josephsl commented Sep 27, 2018 via email •

edited by feerrenrut

josephsl commented Sep 28, 2018 via email •

edited by feerrenrut