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

Add a graphical user interface for the vision framework #10082

Closed
wants to merge 117 commits into from
Closed

Add a graphical user interface for the vision framework #10082

wants to merge 117 commits into from

Conversation

LeonarddeR
Copy link
Collaborator

@LeonarddeR LeonarddeR commented Aug 13, 2019

Link to issue number:

Follow up of #9064
Closes #971

Summary of the issue:

#9064 introduced the vision framework without a graphical user interface.

Description of how this pull request fixes the issue:

Added a new vision panel to the settings dialog. Vision enhancement providers can provide their own gui panel class to create a customized gui. Otherwise, they can rely on the supportedSettings property to be implemented on the provider.

Testing performed:

Tested that providers load and unload and that settings can be changed using the gui.

Known issues with pull request:

  • Providers load and unload directly when checking and unchecking check boxes
  • No user guide changes yet. I'd like to agree about the UX before doing that.

Change log entry:

There's also a wording issue in the changes for developers setting:

  • A framework allowing developers to create vision enhancement providers; > please add the word Àdded to the start of that sentence. You could also consider updating this entry to point at the new vision category in the gui.

@LeonarddeR
Copy link
Collaborator Author

@nishimotz I was thinking about your focus highlight add-on while looking at the changelog entry. Are you intending to discontinue your add-on, or will you rebase it onto the new framework? I think there are still some implementation differences that might cause people to prefer one or the other.
Of course, you're also welcome to extend on the current highlighter in core. One thing I'd like to make configurable at some point is the color and thickness of the highlights.

Copy link
Contributor

@feerrenrut feerrenrut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is your plan for the user guide? Could you update it in this PR, covering the vision panel and focus highlight. Then on the screen curtain PR add that?

I hope you don't mind, I have pushed a commit that addresses a few layout issues.

source/gui/settingsDialogs.py Outdated Show resolved Hide resolved
Copy link
Collaborator Author

@LeonarddeR LeonarddeR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do the user guide as part of this. Have two questions about your change

source/gui/settingsDialogs.py Outdated Show resolved Hide resolved
source/gui/settingsDialogs.py Outdated Show resolved Hide resolved
@feerrenrut
Copy link
Contributor

Overall, I am pretty happy with the default colors for focus highlight. But I might adjust them to have some transparency in a later PR. When they are over text, it can obscure adjacent text.

@nishimotz
Copy link
Contributor

We should consider people who have difficulty distinguishing between red and green colors.
Since the Focus Highlight decided the color first,
I have added the distinction according to the line type later.

Leonard de Ruijter and others added 5 commits September 6, 2019 16:34
@LeonarddeR
Copy link
Collaborator Author

@feerrenrut: This is ready for another review. I think everything from 641ccdc is new.

@feerrenrut
Copy link
Contributor

I'm seeing some unexpected changes in the diff ie libluis. I'll review the user guide to start with.

user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
@dpy013
Copy link
Contributor

dpy013 commented Sep 8, 2019

Current pr build failed
Can it be fixed?
thank

@LeonarddeR
Copy link
Collaborator Author

@dingpengyu Failed appveyor builds are pretty likely to be acted upon pretty soon after they fail.

@dpy013
Copy link
Contributor

dpy013 commented Sep 9, 2019

@dingpengyu Failed appveyor builds are pretty likely to be acted upon pretty soon after they fail.
problem solved
thank

@JulienCochuyt
Copy link
Collaborator

JulienCochuyt commented Sep 13, 2019

I was afraid that the Highlighter could suffer from #10211, that is not being functional while the Windows Magnifier is turned on.
It does not.
Thanks to the GUI implemented in this PR, I could successfully test the Highlighter with Windows Magnifier on and everything seems to work as expected.

@LeonarddeR
Copy link
Collaborator Author

LeonarddeR commented Sep 13, 2019 via email

@LeonarddeR LeonarddeR added the BabbageWork Pull requests filed on behalf of Babbage B.V. label Oct 8, 2019
Copy link
Collaborator Author

@LeonarddeR LeonarddeR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi This branch has conflicts that must be resolved

This is a known issue. This work is a continuous work in progress.

source/autoSettingsUtils/utils.py Show resolved Hide resolved
source/gui/settingsDialogs.py Show resolved Hide resolved
source/vision/providerBase.py Show resolved Hide resolved
It was introduced with this feature, but is not used.
This was introduced with this feature.
Not being used by the framework, and seems likely to mislead.
- class AutoSettings
- data class ProviderInfo

This is more consistent with class DriverSettings
It communicates the intended usage of the string better.
Copy link
Collaborator Author

@LeonarddeR LeonarddeR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks awesome, thanks @feerrenrut

@feerrenrut
Copy link
Contributor

This work was delivered via commit 46c5606

@nishimotz
Copy link
Contributor

Thank you for beautifully reimplementing my work.
I would like to ask from the translation standpoint.
Is the Browse Mode Cursor and the Review Cursor technically different?

@ruifontes
Copy link
Contributor

ruifontes commented Nov 24, 2019 via email

@nishimotz
Copy link
Contributor

Thank you for the suggestion.
Just confirmed that "Highlight browse mode cursor" does not work with review cursor.

STR:

  • Open NVDA menu and select Configuration Profile dialog.
  • Press NVDA+right arrow key (laptop layout)
  • The move of review cursor can be confirmed. However, the yellow rectangle is not available.

I understand the description regarding the vision framework is correct.

So far, "browse mode cursor" is used only in the Content Recognition chapter of the User Guide and almost undocumented.

@LeonarddeR
Copy link
Collaborator Author

LeonarddeR commented Nov 25, 2019 via email

@seanbudd seanbudd added the deprecated/2021.1 Label used to track deprecations due for removal in the 2021.1 release label Mar 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BabbageWork Pull requests filed on behalf of Babbage B.V. deprecated/2021.1 Label used to track deprecations due for removal in the 2021.1 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Visual highlight of focus, browse or caret location being read.
8 participants