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

Setting search (experimental) #16502

Closed
wants to merge 6 commits into from
Closed

Conversation

ABuffEr
Copy link
Contributor

@ABuffEr ABuffEr commented May 7, 2024

Link to issue number:

Possible experimental implementation for #16500

Summary of the issue:

NVDA does not provide any search/filter in settings.

Description of user facing changes

User can type in the search field, above the category list, and wait that the first result is focused.
Then, he can press F3 to move on other results.

Description of development approach

First, this PR is just a proof-of-concept; it's not completed, and I don't know whether I'll be able to continue it further.
I publish it today with the hope this work would be useful in any way.; so contributions or completely new PRs based on it are absolutely welcome.

So, what I did and how:

I worked only under MultiCategorySettingsDialog class.
In makeSettings, added a search field with a timer, that launches search after 500 ms after user stops typing.
In onFilterEditTextChange, the timer is restarted each time user digit a new char.
In onCharHook, F3 calls onFilterEditTextReady to get new results (or cycle over them).
In onFilterEditTextReady, the search field value is split into a list (to implement OR search), and I use itertools.cycle to manage circularly the result generator (and to cache its values). The method takes care also to adjust focus and selection of results.
In searchGenerator there are two main parts: the for loop that loops over categories, that creates a generator for each category panel and yields results as long as there are ones; and the inner generator, that recursively searches on category panel controls to find one with value/label matching any search terms (via searchInLabel method), and yields a tuple as result, containing the matching wx control and the index (on first item) or the wx control to move focus to.

(see known issues)

Testing strategy:

Manual, searching various terms like exit, pitch, color, bookmark, and so on.

Known issues with pull request:

  • search is quite slow when many category panels are not instantiated yet;
  • closing settings dialog after a search often does not terminates all instances, raising MultiInstance error when you try to reopen it (I was not able to understand why);
  • implementing a shift+F3/previous result feature seems quite tricky;
  • having used no threading mechanism, the search is blocking against main thread, I suppose; I thought to implement a consumer-producer pattern, with a result list filled by background threads, but it's just an idea for now.

Code Review Checklist:

  • Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • API is compatible with existing add-ons.
  • Security precautions taken.

@AppVeyorBot
Copy link

  • FAIL: Translation comments check. Translation comments missing or unexpectedly included. See build log for more information.
  • PASS: Unit tests.
  • FAIL: Lint check. See test results for more information.
  • PASS: System tests (tags: installer NVDA).
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/k170y5q9tp10vux5/artifacts/output/nvda_snapshot_pr16502-31774,66660def.exe
  • CI timing (mins):
    INIT 0.0,
    INSTALL_START 1.1,
    INSTALL_END 0.9,
    BUILD_START 0.0,
    BUILD_END 29.3,
    TESTSETUP_START 0.0,
    TESTSETUP_END 0.4,
    TEST_START 0.0,
    TEST_END 2.4,
    FINISH_END 0.3

See test results for failed build of commit 66660defc8

@Adriani90
Copy link
Collaborator

@ABuffEr thanks for this work. Here are some points to consider from #872 raised by Reef.

Some UX points to consider:

  • How do you present the matching settings?
  • Consider visual, speech, braille.
  • Consider multiple matches in multiple settings categories

Approach ideas on the table sofar in addition to what you described:

  1. All panels / options remain available to receive focus / be changed
  2. Panels with matching options are visually highlighted, options that match are visually highlighted. it might be challenging to visually highlight options with WX python. Is it possible?
  3. Pressing f6 or shift+f6 jumps between the currently visible category and the search field
  4. I propose when using f3 or shift+f3 to jump focus to the matching options, the profile, the group and the pannel name the control is located in, is reported in speech and Braille, along with the control label and its state / value.
    e.g.
    a. searching for "UIA" and pressing f3, NVDA would report something like "Profile: Firefox, Advanced settings, Microsoft UIA grouping / Register for UIA events combo box colapsed automatic (prefer selective)."
    This verbose reporting should happen only when entering the settings category, the grouping name should be reported only when entering a new grouping.
    b. Pressing again f3 reports only the next matching setting name and its value / state and if needed the grouping name, until the setting category changes.
    c. When the setting category changes upon pressing f3, NVDA reports the whole stuff again as proposed in a.

Drawbacks:

  • F3 and shift+f3 navigation might lead to much verbosity when entering and leaving categories on the fly
  • Visually highlighting the settings and categories directly when changing focus will definitely be challenging and might have negative performance impact

Second alternative approach, more consistent with what other software do:

  • Use the searchCtrl in wx, fill the dropdown menu with control name and its state / value something like
  1. Search for "br"
  2. A dropdown menu appears, open it with alt+down arrow
  3. First setting is reported "Braille mode combo box colapsed: follow cursors. Press enter to move to the setting"
  4. Other related settings to the keyword "br" are in this dropdown menu, you can navigate by arrow keys
  5. Pressing enter on a setting in the dropdown menu will move the focus to the control in the coresponding category, NVDA reports the whole stuff proposed in a. above when entering the category.
  6. F6 can be used to return focus to the searchCtrl to select another item in the dropdown menu.

Seems most promising in my view, maybe even in combination with the first alternative. The only drawback I see is that the dropdown menu might be a bit confusing.
Advantages:

  • Matches can be isolated as text strings and highlighted in the dropdown menu visually, no need for highlighting matches when the focus changes through the settings categories
  • Less verbosity, the dropdown menu can be less detailed because pressing enter on the desired result will trigger the detailed reporting anyway.

Third alternative approach

  • Show a new settings panel, with just the matching controls (these could be in groups to identify the settings panel and group)
    Drawback: Doesn't allow for changing nearby located settings, higher complexity in the implementation.
    Advantages: people could focus only on the matching results.

@ABuffEr
Copy link
Contributor Author

ABuffEr commented May 7, 2024

Hi @Adriani90,
I think to have implemented substantially the first approach proposed by Reef, even something is not so clear to me (all panels available?), or needed in my opinion (like F6/shift+F6, even if search field shortcut can be surely changed).
Verbosity of the result announcement can be adjusted, but in current implementation it should be sufficient to announce the panel title when it changes.
No idea about visual highlighting, it should be the same you get during usual navigation.
Happy to receive other feedback.

@seanbudd
Copy link
Member

Any progress on this? Wider feedback on the UX is encouraged from the community

@seanbudd seanbudd added the merge-early Merge Early in a developer cycle label Jun 17, 2024
@ABuffEr
Copy link
Contributor Author

ABuffEr commented Jun 17, 2024

Hi,
I have some changes locally, but not decisive (particularly about MultiInstance error).
No other news, sorry...

@seanbudd
Copy link
Member

seanbudd commented Sep 1, 2024

@ABuffEr - do you plan to do any further work? Otherwise I think this should be closed as Abandoned

@seanbudd seanbudd added the Abandoned requested reports or updates are missing since more than 1 year, author or users are not available. label Sep 1, 2024
@seanbudd seanbudd closed this Sep 1, 2024
@ABuffEr
Copy link
Contributor Author

ABuffEr commented Sep 2, 2024

Hi,
yes, it's abandoned. I'll make a completely new PR if I'll pick it up again in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Abandoned requested reports or updates are missing since more than 1 year, author or users are not available. merge-early Merge Early in a developer cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants