-
-
Notifications
You must be signed in to change notification settings - Fork 650
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
Allow the review cursor to be bounded to onscreen text #9735
Changes from 23 commits
7a123c3
a61dab0
a8b93bd
6bb21b2
deffcea
51fa56a
a1ad305
d7a00e8
1a937d2
536ccce
d84570a
4540972
b7126db
0f41ef3
768cf6b
4cc443f
291b00e
c94229b
b2924fa
2c24358
959a6c7
360ce53
7146fae
2f85736
87d67e7
d41476b
8c50383
cf11cdd
11f0d59
bd33c92
f3ff4a2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,7 +32,7 @@ | |
import treeInterceptorHandler | ||
import browseMode | ||
import scriptHandler | ||
from scriptHandler import script | ||
from scriptCategories import * | ||
import ui | ||
import braille | ||
import brailleInput | ||
|
@@ -44,49 +44,6 @@ | |
import winVersion | ||
from base64 import b16encode | ||
|
||
#: Script category for text review commands. | ||
# Translators: The name of a category of NVDA commands. | ||
SCRCAT_TEXTREVIEW = _("Text review") | ||
#: Script category for Object navigation commands. | ||
# Translators: The name of a category of NVDA commands. | ||
SCRCAT_OBJECTNAVIGATION = _("Object navigation") | ||
#: Script category for system caret commands. | ||
# Translators: The name of a category of NVDA commands. | ||
SCRCAT_SYSTEMCARET = _("System caret") | ||
#: Script category for mouse commands. | ||
# Translators: The name of a category of NVDA commands. | ||
SCRCAT_MOUSE = _("Mouse") | ||
#: Script category for speech commands. | ||
# Translators: The name of a category of NVDA commands. | ||
SCRCAT_SPEECH = _("Speech") | ||
#: Script category for configuration dialogs commands. | ||
# Translators: The name of a category of NVDA commands. | ||
SCRCAT_CONFIG = _("Configuration") | ||
#: Script category for configuration profile activation and management commands. | ||
# Translators: The name of a category of NVDA commands. | ||
SCRCAT_CONFIG_PROFILES = _("Configuration profiles") | ||
#: Script category for Braille commands. | ||
# Translators: The name of a category of NVDA commands. | ||
SCRCAT_BRAILLE = _("Braille") | ||
#: Script category for tools commands. | ||
# Translators: The name of a category of NVDA commands. | ||
SCRCAT_TOOLS = pgettext('script category', 'Tools') | ||
#: Script category for touch commands. | ||
# Translators: The name of a category of NVDA commands. | ||
SCRCAT_TOUCH = _("Touch screen") | ||
#: Script category for focus commands. | ||
# Translators: The name of a category of NVDA commands. | ||
SCRCAT_FOCUS = _("System focus") | ||
#: Script category for system status commands. | ||
# Translators: The name of a category of NVDA commands. | ||
SCRCAT_SYSTEM = _("System status") | ||
#: Script category for input commands. | ||
# Translators: The name of a category of NVDA commands. | ||
SCRCAT_INPUT = _("Input") | ||
#: Script category for document formatting commands. | ||
# Translators: The name of a category of NVDA commands. | ||
SCRCAT_DOCUMENTFORMATTING = _("Document formatting") | ||
|
||
class GlobalCommands(ScriptableObject): | ||
"""Commands that are available at all times, regardless of the current focus. | ||
""" | ||
|
@@ -972,8 +929,21 @@ def script_review_activate(self,gesture): | |
script_review_activate.__doc__=_("Performs the default action on the current navigator object (example: presses it if it is a button).") | ||
script_review_activate.category=SCRCAT_OBJECTNAVIGATION | ||
|
||
def _moveWithBoundsChecking(self, info, unit, direction, endPoint=None): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could probably be moved into the base TextInfo class. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a generalization of the console's old bounds checking code (i.e. we captured an |
||
""" | ||
Nondestructively moves a textInfo and returns a (newInfo, res) tuple. | ||
Res is 0 and the original C{textInfo} is returned if the move would be out of bounds. | ||
""" | ||
newInfo = info.copy() | ||
res = newInfo.move(unit, direction, endPoint=endPoint) | ||
if newInfo.obj.reviewBounded and newInfo.isOffscreen: | ||
return (info, 0) | ||
return (newInfo, res) | ||
|
||
def script_review_top(self,gesture): | ||
info=api.getReviewPosition().obj.makeTextInfo(textInfos.POSITION_FIRST) | ||
obj = api.getReviewPosition().obj | ||
pos = textInfos.POSITION_FIRSTVISIBLE if obj.reviewBounded else textInfos.POSITION_FIRST | ||
codeofdusk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
info = obj.makeTextInfo(pos) | ||
api.setReviewPosition(info) | ||
info.expand(textInfos.UNIT_LINE) | ||
ui.reviewMessage(_("Top")) | ||
|
@@ -987,7 +957,7 @@ def script_review_previousLine(self,gesture): | |
if info._expandCollapseBeforeReview: | ||
info.expand(textInfos.UNIT_LINE) | ||
info.collapse() | ||
res=info.move(textInfos.UNIT_LINE,-1) | ||
info, res = self._moveWithBoundsChecking(info, textInfos.UNIT_LINE,-1) | ||
if res==0: | ||
# Translators: a message reported when review cursor is at the top line of the current navigator object. | ||
ui.reviewMessage(_("Top")) | ||
|
@@ -1019,7 +989,7 @@ def script_review_nextLine(self,gesture): | |
if info._expandCollapseBeforeReview: | ||
info.expand(textInfos.UNIT_LINE) | ||
info.collapse() | ||
res=info.move(textInfos.UNIT_LINE,1) | ||
info, res = self._moveWithBoundsChecking(info, textInfos.UNIT_LINE,1) | ||
if res==0: | ||
# Translators: a message reported when review cursor is at the bottom line of the current navigator object. | ||
ui.reviewMessage(_("Bottom")) | ||
|
@@ -1033,7 +1003,9 @@ def script_review_nextLine(self,gesture): | |
script_review_nextLine.category=SCRCAT_TEXTREVIEW | ||
|
||
def script_review_bottom(self,gesture): | ||
info=api.getReviewPosition().obj.makeTextInfo(textInfos.POSITION_LAST) | ||
obj = api.getReviewPosition().obj | ||
pos = textInfos.POSITION_LASTVISIBLE if obj.reviewBounded else textInfos.POSITION_LAST | ||
info = obj.makeTextInfo(pos) | ||
api.setReviewPosition(info) | ||
info.expand(textInfos.UNIT_LINE) | ||
ui.reviewMessage(_("Bottom")) | ||
|
@@ -1047,7 +1019,7 @@ def script_review_previousWord(self,gesture): | |
if info._expandCollapseBeforeReview: | ||
info.expand(textInfos.UNIT_WORD) | ||
info.collapse() | ||
res=info.move(textInfos.UNIT_WORD,-1) | ||
info, res = self._moveWithBoundsChecking(info, textInfos.UNIT_WORD,-1) | ||
if res==0: | ||
# Translators: a message reported when review cursor is at the top line of the current navigator object. | ||
ui.reviewMessage(_("Top")) | ||
|
@@ -1078,7 +1050,7 @@ def script_review_nextWord(self,gesture): | |
if info._expandCollapseBeforeReview: | ||
info.expand(textInfos.UNIT_WORD) | ||
info.collapse() | ||
res=info.move(textInfos.UNIT_WORD,1) | ||
info, res = self._moveWithBoundsChecking(info, textInfos.UNIT_WORD,1) | ||
if res==0: | ||
# Translators: a message reported when review cursor is at the bottom line of the current navigator object. | ||
ui.reviewMessage(_("Bottom")) | ||
|
@@ -1206,7 +1178,7 @@ def _getCurrentLanguageForTextInfo(self, info): | |
curLanguage = speech.getCurrentLanguage() | ||
return curLanguage | ||
|
||
@script( | ||
@scriptHandler.script( | ||
# Translators: Input help mode message for Review Current Symbol command. | ||
description=_("Reports the symbol where the review cursor is positioned. Pressed twice, shows the symbol and the text used to speak it in browse mode"), | ||
category=SCRCAT_TEXTREVIEW, | ||
|
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.
It worries me that this is is on the object instead of a global command. Is there a particular reason for that?
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 too think this could be cleaner as a global command, but the
reviewBounded
variable is also on the object so keeping this there might be better from an organizational standpoint...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.
Question is whether we really believe that review Bounded should be on the object or on the textInfo, or even global. If an object gets destroyed and created again, the state of review Bounded is lost. Having review bounds on tree interceptors also makes sense, i.e. when you want to review with document review what is on screen on a particular page.
I also think that it doesn't make much sense to have review bounds doing anything when in screen review. So in that case, the command should just do nothing.
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 it may have been my idea to put this on the object. but @LeonarddeR brings up a good point in that the state will be lost if moving away from the object and back again. The problem with having it global though is that different controls work best when reviewBounded is set in a specific way by default. I.e. consoles probably want it on by default.
Would we be comfortable having bounded on by default globally? I think this could work, but it would be a bit of a change to NvDA's behaviour. Probably a good change though. Thoughts?
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.
Yes, having this as a global default makes sense to me.
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.
Yes, I also think a global default would be a plus.
It does not make much sense for a "screen" review to easily review what's not on screen.
It can be helpful at times, but is most often confusing when collaborating with a sighted person.
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 has now been made a global default.