Skip to content

Conversation

@Boumtchack
Copy link

@Boumtchack Boumtchack commented Nov 17, 2025

Link to issue number:

Closes #12539

Summary of the issue:

This pull request introduces a new NVDA magnifier feature, including docked and lens magnifier modes, color filter support, and a set of global commands for controlling magnification. The changes add new classes for handling magnifier windows, provide optimized color filtering, and implement keyboard shortcuts for toggling magnification, zooming, cycling modes, and color filters.

Description of user facing changes:

Implemented new script commands in globalCommands.py for starting/stopping the magnifier, zooming in/out, cycling color filters, toggling fullscreen mode, cycling magnifier types, and spotlighting the magnifier window, each with descriptive messages and gestures.

Description of developer facing changes:

Description of development approach:

Creating 3 types of magnifier that will be called based on prefered choice of the user. each one got his class and will share settings through parents class

Testing strategy:

unit test

Known issues with pull request:

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 mentioned this pull request Nov 17, 2025
8 tasks
@seanbudd seanbudd changed the title Nvda magnifier implementation Magnifier implementation Nov 18, 2025
@Boumtchack Boumtchack force-pushed the NvdaMagnifierImplementation branch from f5a86b5 to 7b3ee4f Compare November 24, 2025 10:10
@seanbudd
Copy link
Member

I think the behaviour with screen curtain is whatever gets activated should just deactivate the other. However, in settings, we need to make sure both aren't activated at the same time. We should add checks when saving settings to ensure NVDA won't start trying to enable both screen curtain and magnifier. When enabling each, check if the other is active, and if so, disable it.

@seanbudd
Copy link
Member

Actually I think the behaviour with screen curtain as you've designed is fine for now. It ensures the person intentionally wants to deactivate screen curtain.

Copy link
Member

@seanbudd seanbudd left a comment

Choose a reason for hiding this comment

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

Thanks @Boumtchack, the docs here are really good. I've reviewed everything except the tests

@CyrilleB79
Copy link
Contributor

I think the behaviour with screen curtain is whatever gets activated should just deactivate the other.

Why? I have tried to describe a valid use case in #19228 (comment), where screen curtain would be enabled for a short time while Magnifier is already active.

Note that I have not tested last devs of @Boumtchack against that use case.
Also, if I remember well, only the color filter usage conflicts with screen curtain, not all the Magnifier API; at least that is the case with Windows Magnifier.

@seanbudd
Copy link
Member

@CyrilleB79 - did you miss my follow up message #19228 (comment)

Actually I think the behaviour with screen curtain as you've designed is fine for now. It ensures the person intentionally wants to deactivate screen curtain.

@CyrilleB79
Copy link
Contributor

@CyrilleB79 - did you miss my follow up message #19228 (comment)

Actually I think the behaviour with screen curtain as you've designed is fine for now. It ensures the person intentionally wants to deactivate screen curtain.

Oh sorry, OK. From my quick reading of the description it sounds fine.
I'll come back after tests if needed.

@seanbudd seanbudd marked this pull request as ready for review January 6, 2026 03:06
@seanbudd seanbudd requested a review from a team as a code owner January 6, 2026 03:06
@seanbudd seanbudd requested a review from Qchristensen January 6, 2026 03:06

##### Default focus mode {#MagnifierDefaultFocusMode}

| . {.hideHeaderRow} |.|
Copy link
Member

Choose a reason for hiding this comment

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

please move this to below the summary paragraph, but above the other table


##### Default color filter {#MagnifierDefaultFilter}

| . {.hideHeaderRow} |.|
Copy link
Member

Choose a reason for hiding this comment

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

please move this to below the summary paragraph, but above the other table


##### Default zoom level {#MagnifierDefaultZoom}

| . {.hideHeaderRow} |.|
Copy link
Member

Choose a reason for hiding this comment

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

please move this to the bottom of the section

Comment on lines 2806 to 2807
|Options |Default (Enabled), Disabled, Enabled |
|Default |Enabled |
Copy link
Member

Choose a reason for hiding this comment

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

please make sure to list the actual options and defaults


##### Keep mouse centered {#MagnifierKeepMouseCentered}

| . {.hideHeaderRow} |.|
Copy link
Member

Choose a reason for hiding this comment

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

please move this to the bottom of the section

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. merge-early Merge Early in a developer cycle

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow Windows Magnifier to follow NVDA virtual cursor

3 participants