Skip to content

Toggle settings for magnifier focus#19739

Open
Boumtchack wants to merge 11 commits intonvaccess:masterfrom
France-Travail:feature/MagnifierClarityTracking
Open

Toggle settings for magnifier focus#19739
Boumtchack wants to merge 11 commits intonvaccess:masterfrom
France-Travail:feature/MagnifierClarityTracking

Conversation

@Boumtchack
Copy link
Copy Markdown
Contributor

@Boumtchack Boumtchack commented Mar 3, 2026

Link to issue number:

#19712

Summary of the issue:

NVDA's magnifier lacked per-source tracking toggles: mouse, system focus, review cursor and navigator object could not be independently enabled or disabled.

Description of user facing changes:

Four new checkboxes in the magnifier settings panel let users independently enable or disable tracking and four new gestures to toggle these settings (no key bidding yet).:

  • mouse pointer
  • system focus
  • review cursor
  • and navigator object.

Description of developer facing changes:

  • FocusType enum gains a REVIEW
  • FocusManager.getCurrentFocusCoordinates() now gates each source based on config.

Description of development approach:

The priority chains has been updated

Testing strategy:

Unit

Known issues with pull request:

Mouse disabling doesn't take effect properly:
Magnifier will follow mouse after disabling it until a new focus is given.

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.

@Boumtchack Boumtchack marked this pull request as ready for review March 3, 2026 15:57
@Boumtchack Boumtchack requested a review from a team as a code owner March 3, 2026 15:57
@Boumtchack Boumtchack marked this pull request as draft March 3, 2026 15:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR extends NVDA's built-in magnifier by adding per-source tracking toggle settings: users can now independently enable or disable magnifier tracking for mouse, system focus, review cursor, and navigator object via four new checkboxes in the Magnifier Settings panel, as well as via four new (mostly unbound) keyboard commands. Internally, the change adds a REVIEW member to FocusType, separates the review cursor into its own priority level (between system focus and navigator object), and gates each tracking source behind a configuration check.

Changes:

  • Adds FocusType.REVIEW enum value, new MagnifierFollowFocusType enum, and TOGGLE_FOLLOW_SETTINGS to MagnifierAction in types.py
  • Adds four follow-setting config accessors in config.py, four new [magnifier] boolean config spec entries, and a toggleFollow() command in commands.py
  • Adds four GUI checkboxes in settingsDialogs.py, four new scripts in globalCommands.py, and updates FocusManager with review cursor tracking, per-source gating, and a new UpdateFollowedFocus method

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
source/_magnifier/utils/types.py Adds FocusType.REVIEW, MagnifierFollowFocusType enum, and TOGGLE_FOLLOW_SETTINGS action
source/_magnifier/utils/focusManager.py Adds review cursor tracking, per-source follow*() gating in priority chain, and UpdateFollowedFocus method
source/_magnifier/config.py Adds four config accessor functions (followMouse, followSystemFocus, followReview, followNavigatorObject)
source/_magnifier/commands.py Adds toggleFollow() command that toggles each follow setting and announces the new state
source/config/configSpec.py Adds four boolean config spec entries with default=True
source/gui/settingsDialogs.py Adds four new checkboxes in the magnifier settings panel with context-sensitive help bindings
source/globalCommands.py Adds four new toggle scripts (one with a default gesture, three without)
tests/unit/test_magnifier/test_focusManager.py Adds tests for review cursor position, per-source follow gating, and a new TestFollowSettings class

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@seanbudd seanbudd requested review from seanbudd and removed request for SaschaCowley March 5, 2026 02:42
@seanbudd seanbudd self-assigned this Mar 5, 2026
@seanbudd seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Mar 5, 2026
@seanbudd
Copy link
Copy Markdown
Member

Is this ready for review?

@Boumtchack Boumtchack marked this pull request as ready for review March 31, 2026 12:21
@Boumtchack Boumtchack requested a review from a team as a code owner March 31, 2026 12:21
@Boumtchack Boumtchack requested a review from Qchristensen March 31, 2026 12:21
Comment on lines +2906 to +2944
#### Follow mouse {#MagnifierFollowMouse}

This checkbox controls whether the magnifier should follow the mouse pointer.
When enabled, the magnified area will automatically move to follow the mouse pointer, which can be helpful for users who navigate primarily using the mouse rather than the keyboard.

This option is enabled by default.

| . {.hideHeaderRow} |.|
|---|---|
|Options |Disabled, Enabled|
|Default |Enabled|

#### Follow system focus {#MagnifierFollowSystemFocus}

This checkbox controls whether the magnifier should follow the system focus.
When enabled, the magnified area will automatically move to follow the system focus, which can be helpful for users who navigate primarily using the keyboard and want the magnifier to track their navigation.

This option is enabled by default.

| . {.hideHeaderRow} |.|
|---|---|
|Options |Disabled, Enabled|
|Default |Enabled|

#### Follow review cursor {#MagnifierFollowReviewCursor}

This checkbox controls whether the magnifier should follow the review cursor.
When enabled, the magnified area will automatically move to follow the review cursor, which can be helpful for users who use the review cursor to navigate through content and want the magnifier to track their navigation.

This option is enabled by default.

| . {.hideHeaderRow} |.|
|---|---|
|Options |Disabled, Enabled|
|Default |Enabled|

#### Follow navigator object {#MagnifierFollowNavigatorObject}

This checkbox controls whether the magnifier should follow the navigator object.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think rather than a set of checkboxes, we should use a system like the speech modes list, where you select multiple items. see nvdaControls.CustomCheckListBox and excludedSpeechModes for examples.

Thoughts @CyrilleB79 @Boumtchack ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am tempted to keep separated checkboxes here instead, so that one accelerator shortcut key can be attributed to each one of these items.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think checkboxes are the best, I could create a visual box tho to make it prettier

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

do you mean as a sub group? I think thats a good idea

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I will do a pr for settings dialog, I'll include it in that if that's ok for you

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think we need a separate PR, just move the code added in source/gui/settingsDialogs.py into a groupbox

@seanbudd seanbudd marked this pull request as draft April 1, 2026 03:21
@Boumtchack Boumtchack marked this pull request as ready for review April 7, 2026 09:20
def toggleAllFollowStates() -> bool:
"""
Check if magnifier should follow navigator object.
Toggle all follow states between forced-active and previously saved states.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
Toggle all follow states between forced-active and previously saved states.
Toggle all follow states between forced-active and previously saved states.

Comment on lines +139 to +129
"""
Set whether the magnifier should follow system focus.
_savedFollowStates: dict[MagnifierFollowFocusType, bool] = {}

:param value: True to follow system focus, False otherwise.
"""
config.conf["magnifier"]["followSystemFocus"] = value
_allFollowStatesForcedActive = False
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

could this be combined into a single dataclass forceFollowState or similar

Comment on lines +2906 to +2944
#### Follow mouse {#MagnifierFollowMouse}

This checkbox controls whether the magnifier should follow the mouse pointer.
When enabled, the magnified area will automatically move to follow the mouse pointer, which can be helpful for users who navigate primarily using the mouse rather than the keyboard.

This option is enabled by default.

| . {.hideHeaderRow} |.|
|---|---|
|Options |Disabled, Enabled|
|Default |Enabled|

#### Follow system focus {#MagnifierFollowSystemFocus}

This checkbox controls whether the magnifier should follow the system focus.
When enabled, the magnified area will automatically move to follow the system focus, which can be helpful for users who navigate primarily using the keyboard and want the magnifier to track their navigation.

This option is enabled by default.

| . {.hideHeaderRow} |.|
|---|---|
|Options |Disabled, Enabled|
|Default |Enabled|

#### Follow review cursor {#MagnifierFollowReviewCursor}

This checkbox controls whether the magnifier should follow the review cursor.
When enabled, the magnified area will automatically move to follow the review cursor, which can be helpful for users who use the review cursor to navigate through content and want the magnifier to track their navigation.

This option is enabled by default.

| . {.hideHeaderRow} |.|
|---|---|
|Options |Disabled, Enabled|
|Default |Enabled|

#### Follow navigator object {#MagnifierFollowNavigatorObject}

This checkbox controls whether the magnifier should follow the navigator object.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think we need a separate PR, just move the code added in source/gui/settingsDialogs.py into a groupbox

@seanbudd seanbudd marked this pull request as draft April 7, 2026 23:26
This was referenced Apr 7, 2026
@Boumtchack Boumtchack marked this pull request as ready for review April 14, 2026 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants