preparing the new magnifier types by Boumtchack · Pull Request #19915 · nvaccess/nvda

Boumtchack · 2026-04-07T14:49:01Z

Link to issue number:

pre - #19473
parts of #19810

Summary of the issue:

needed to add logic for changing types before new types

Description of user facing changes:

Magnifier got a new cycle gesture to change to soon to be implemented new types: fixed/docked/lens

Description of developer facing changes:

added a placeholder for easier developement to be done.

Description of development approach:

as the new modes will inherite from the same logics, first making sure they can be added correctly to the actual code

Testing strategy:

Unit

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.

Copilot

Pull request overview

Prepares NVDA’s magnifier subsystem for upcoming magnifier “types” (fullscreen/fixed/docked/lens) by adding type persistence, a cycle gesture, and placeholder implementations.

Changes:

Add magnifierType to config and wire initialization to create the appropriate magnifier instance.
Add a global command/gesture to cycle magnifier types and announce the selected type.
Introduce a PlaceholderMagnifier, expand MagnifierType enum, update docs and unit tests for the new lifecycle.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
`user_docs/en/userGuide.md`	Adds documentation for magnifier types and placeholder sections for upcoming modes.
`tests/unit/test_magnifier/test_magnifier.py`	Updates unit test expectations around magnifier type initialization.
`tests/unit/test_magnifier/test_fullscreenMagnifier.py`	Updates fullscreen tests to explicitly start the magnifier after constructor changes.
`source/globalCommands.py`	Adds a new script/gesture to cycle magnifier type.
`source/config/configSpec.py`	Adds `magnifierType` setting under `[magnifier]`.
`source/_magnifier/utils/types.py`	Adds `CHANGE_MAGNIFIER_TYPE` action and new `MagnifierType` members.
`source/_magnifier/placeholderMagnifier.py`	Adds placeholder magnifier class for not-yet-implemented types.
`source/_magnifier/magnifier.py`	Adjusts base class `_magnifierType` initialization behavior.
`source/_magnifier/config.py`	Adds getters/setters for `magnifierType`.
`source/_magnifier/commands.py`	Adds cycle-type command and updates start message to include type.
`source/_magnifier/__init__.py`	Refactors module initialization to create magnifier instances by type and support runtime type changes.

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

Copilot · 2026-04-07T15:00:56Z

source/_magnifier/utils/types.py

 class MagnifierType(DisplayStringStrEnum):
 	"""Type of magnifier"""

 	FULLSCREEN = "fullscreen"
+	FIXED = "fixed"
 	DOCKED = "docked"
 	LENS = "lens"
+	PLACEHOLDER = "placeholder"

 	@property
 	def _displayStringLabels(self) -> dict["MagnifierType", str]:
 		return {
 			# Translators: Magnifier type - full-screen mode.
 			self.FULLSCREEN: pgettext("magnifier", "Fullscreen"),
+			# Translators: Magnifier type - fixed mode.
+			self.FIXED: pgettext("magnifier", "Fixed"),
 			# Translators: Magnifier type - docked mode.
 			self.DOCKED: pgettext("magnifier", "Docked"),
 			# Translators: Magnifier type - lens mode.
 			self.LENS: pgettext("magnifier", "Lens"),
+			# Translators: Magnifier type - placeholder for unsupported types.
+			self.PLACEHOLDER: pgettext("magnifier", "Placeholder"),
 		}


MagnifierType.PLACEHOLDER is part of the public MagnifierType enum, but it isn’t handled by createMagnifier and will be included in list(MagnifierType) when cycling types. This can lead to runtime errors (e.g., switching to PLACEHOLDER would raise ValueError in createMagnifier). Consider removing PLACEHOLDER from the enum (keep placeholder as an implementation detail), or explicitly support it in createMagnifier and ensure it’s excluded from user-facing cycling/UI.

Copilot · 2026-04-07T15:00:56Z

source/_magnifier/__init__.py

+			return PlaceholderMagnifier()
+
+		case MagnifierType.DOCKED:
+			from .placeholderMagnifier import PlaceholderMagnifier
+
+			return PlaceholderMagnifier()
+
+		case MagnifierType.LENS:
+			from .placeholderMagnifier import PlaceholderMagnifier
+
+			return PlaceholderMagnifier()


createMagnifier returns PlaceholderMagnifier() for FIXED/DOCKED/LENS, but PlaceholderMagnifier currently reports its type as MagnifierType.PLACEHOLDER. This breaks type cycling (the current type no longer matches the configured/selected type) and causes user announcements/config (setMagnifierType) to diverge from the actual instance state. Consider passing the requested magnifierType into PlaceholderMagnifier (and setting self._magnifierType accordingly) so the instance reflects the selected type.

Suggested change

return PlaceholderMagnifier()

case MagnifierType.DOCKED:

from .placeholderMagnifier import PlaceholderMagnifier

return PlaceholderMagnifier()

case MagnifierType.LENS:

from .placeholderMagnifier import PlaceholderMagnifier

return PlaceholderMagnifier()

magnifier = PlaceholderMagnifier()

magnifier._magnifierType = magnifierType

return magnifier

case MagnifierType.DOCKED:

from .placeholderMagnifier import PlaceholderMagnifier

magnifier = PlaceholderMagnifier()

magnifier._magnifierType = magnifierType

return magnifier

case MagnifierType.LENS:

from .placeholderMagnifier import PlaceholderMagnifier

magnifier = PlaceholderMagnifier()

magnifier._magnifierType = magnifierType

return magnifier

Copilot · 2026-04-07T15:00:57Z

source/_magnifier/commands.py

+		types = list(MagnifierType)
+		currentType = magnifier._magnifierType
+		idx = types.index(currentType)
+		newType = types[(idx + 1) % len(types)]
+		log.debug(f"Changing magnifier type from {currentType} to {newType}")
+		changeMagnifierType(newType)
+		setMagnifierType(newType)
+		magnifier = getMagnifier()
+		ui.message(
+			pgettext(
+				"magnifier",
+				# Translators: Message announced when changing the magnifier type with {type} being the new magnifier type.
+				"Magnifier type changed to {type}",
+			).format(type=magnifier._magnifierType.displayString),


cycleMagnifierType iterates list(MagnifierType), which currently includes PLACEHOLDER. This can select a type that createMagnifier doesn’t support, raising at runtime. Also, when FIXED/DOCKED/LENS are implemented via PlaceholderMagnifier with _magnifierType == PLACEHOLDER, cycling will not behave as expected because currentType won’t equal the configured mode. Use an explicit ordered list of user-selectable types (e.g. FULLSCREEN/FIXED/DOCKED/LENS) and ensure the active magnifier instance reports the selected type.

Suggested change

types = list(MagnifierType)

currentType = magnifier._magnifierType

idx = types.index(currentType)

newType = types[(idx + 1) % len(types)]

log.debug(f"Changing magnifier type from {currentType} to {newType}")

changeMagnifierType(newType)

setMagnifierType(newType)

magnifier = getMagnifier()

ui.message(

pgettext(

"magnifier",

# Translators: Message announced when changing the magnifier type with {type} being the new magnifier type.

"Magnifier type changed to {type}",

).format(type=magnifier._magnifierType.displayString),

selectableTypes = [

MagnifierType.FULLSCREEN,

MagnifierType.FIXED,

MagnifierType.DOCKED,

MagnifierType.LENS,

]

currentType = (

magnifier._magnifierType

if magnifier._magnifierType in selectableTypes

else getMagnifierType()

)

if currentType not in selectableTypes:

currentType = selectableTypes[0]

idx = selectableTypes.index(currentType)

newType = selectableTypes[(idx + 1) % len(selectableTypes)]

log.debug(f"Changing magnifier type from {currentType} to {newType}")

changeMagnifierType(newType)

setMagnifierType(newType)

ui.message(

pgettext(

"magnifier",

# Translators: Message announced when changing the magnifier type with {type} being the new magnifier type.

"Magnifier type changed to {type}",

).format(type=newType.displayString),

Copilot · 2026-04-07T15:00:57Z

source/_magnifier/commands.py

+def cycleMagnifierType() -> None:
+	"""Cycle through magnifier types (full-screen, fixed, docked (to do), lens (to do))"""
+	magnifier: Magnifier = getMagnifier()
+	if magnifierIsActiveVerify(
+		magnifier,
+		MagnifierAction.CHANGE_MAGNIFIER_TYPE,
+	):
+		types = list(MagnifierType)
+		currentType = magnifier._magnifierType
+		idx = types.index(currentType)
+		newType = types[(idx + 1) % len(types)]
+		log.debug(f"Changing magnifier type from {currentType} to {newType}")
+		changeMagnifierType(newType)
+		setMagnifierType(newType)
+		magnifier = getMagnifier()
+		ui.message(
+			pgettext(
+				"magnifier",
+				# Translators: Message announced when changing the magnifier type with {type} being the new magnifier type.
+				"Magnifier type changed to {type}",
+			).format(type=magnifier._magnifierType.displayString),
+		)
+


After switching to a non-fullscreen magnifier type, other commands (e.g. toggleFilter) still assume magnifier.filterType exists. PlaceholderMagnifier doesn’t implement filterType, so toggling filters will raise AttributeError once users cycle into FIXED/DOCKED/LENS. Either move a filterType property to the base Magnifier class (using _filterType), or implement it on PlaceholderMagnifier (and other future types).

Copilot · 2026-04-07T15:00:58Z

source/_magnifier/placeholderMagnifier.py

+	def _doUpdate(self):
+		super()._doUpdate()


PlaceholderMagnifier._doUpdate calls super()._doUpdate(), which always raises NotImplementedError. Any path that triggers _doUpdate (e.g. Magnifier._pan calls _doUpdate) will crash when a placeholder magnifier is active. Implement _doUpdate as a safe no-op (or minimal placeholder behavior) instead of delegating to the abstract base method.

Suggested change

def _doUpdate(self):

super()._doUpdate()

def _doUpdate(self) -> None:

# Placeholder magnifier has no backing implementation to refresh.

# Intentionally do nothing instead of delegating to the abstract

# base implementation, which raises NotImplementedError.

return

Copilot · 2026-04-07T15:00:58Z

source/_magnifier/magnifier.py

 	def __init__(self):
 		self._displayOrientation = getPrimaryDisplayOrientation()
-		self._magnifierType: MagnifierType = MagnifierType.FULLSCREEN
+		self._magnifierType: MagnifierType


self._magnifierType: MagnifierType is only an annotation and does not assign a value, so instances of Magnifier won’t have a _magnifierType attribute until a subclass sets it. This is easy to trip into AttributeError (e.g. if any code inspects _magnifierType before subclass initialization completes). Consider initializing it to a safe default (e.g. MagnifierType.FULLSCREEN) or to None and handling that explicitly.

Suggested change

self._magnifierType: MagnifierType

self._magnifierType: MagnifierType | None = None

Copilot · 2026-04-07T15:00:59Z

source/globalCommands.py

+	@script(
+		description=_(
+			# Translators: Describes a command.
+			"Cycle through Magnifier type",


The new script description string has grammar/capitalization issues ("Cycle through Magnifier type"). For consistency with other command descriptions in this section, consider something like "Cycle through magnifier types" (lowercase magnifier, plural types).

Suggested change

"Cycle through Magnifier type",

"Cycle through magnifier types",

Copilot · 2026-04-07T15:00:59Z

user_docs/en/userGuide.md

-### Focus Tracking Modes {#MagnifierFullscreenFocusModes}
+### Magnifier Types {#MagnifierType}

 The magnifier offers three different modes for tracking focus and determining which part of the screen to magnify:


The new “Magnifier Types” section contains a leftover sentence about “three different modes for tracking focus”, which is both redundant and inaccurate (this section now lists 4 types). Remove or reword this line so the section only describes magnifier types, and keep “focus tracking modes” language in the fullscreen focus modes section.

Suggested change

The magnifier offers three different modes for tracking focus and determining which part of the screen to magnify:

Copilot · 2026-04-07T15:00:59Z

user_docs/en/userGuide.md

+Placeholder
+
+### Docked Magnifier {#MagnifierDocked}
+
+Placeholder
+
+### Lens Magnifier {#MagnifierLens}
+
+Placeholder


The “Fixed/Docked/Lens Magnifier” sections currently contain just “Placeholder”. If these modes aren’t implemented yet, it would be better to either omit these sections until they’re documented, or replace “Placeholder” with a short user-facing note explaining that these types are not yet available/are under development (and how users will encounter them, if at all).

Suggested change

Placeholder

### Docked Magnifier {#MagnifierDocked}

Placeholder

### Lens Magnifier {#MagnifierLens}

Placeholder

Fixed magnifier view is not currently available in NVDA.

If support for this view is added in a future version, this section will be updated with usage information.

### Docked Magnifier {#MagnifierDocked}

Docked magnifier view is not currently available in NVDA.

If support for this view is added in a future version, this section will be updated with usage information.

### Lens Magnifier {#MagnifierLens}

Lens magnifier view is not currently available in NVDA.

If support for this view is added in a future version, this section will be updated with usage information.

Copilot · 2026-04-07T15:01:00Z

source/_magnifier/commands.py

+def cycleMagnifierType() -> None:
+	"""Cycle through magnifier types (full-screen, fixed, docked (to do), lens (to do))"""
+	magnifier: Magnifier = getMagnifier()
+	if magnifierIsActiveVerify(
+		magnifier,
+		MagnifierAction.CHANGE_MAGNIFIER_TYPE,
+	):
+		types = list(MagnifierType)
+		currentType = magnifier._magnifierType
+		idx = types.index(currentType)
+		newType = types[(idx + 1) % len(types)]
+		log.debug(f"Changing magnifier type from {currentType} to {newType}")
+		changeMagnifierType(newType)
+		setMagnifierType(newType)
+		magnifier = getMagnifier()


New behavior (cycling magnifier types and persisting magnifierType in config) isn’t covered by unit tests. Adding tests that (1) cycle from FULLSCREEN through the supported types, (2) verify the created instance type matches the selected/configured type, and (3) verify commands like toggleFilter/pan don’t crash for placeholder modes would prevent regressions while the real implementations are still pending.

seanbudd · 2026-04-07T23:34:52Z

Let's focus on #19732 #19739 #19780 #19882 for now. Once most of these are merged, we can work on #19473 #19915 and #19913. Otherwise, there's too much conflicts between the different PRs. We want to ensure work which has already started gets completed before new work is started.

preparing the new magnifier types

b839c3c

Boumtchack marked this pull request as ready for review April 7, 2026 14:54

Boumtchack requested review from a team as code owners April 7, 2026 14:54

Boumtchack requested review from Qchristensen, SaschaCowley and Copilot April 7, 2026 14:54

Boumtchack marked this pull request as draft April 7, 2026 14:54

Copilot started reviewing on behalf of Boumtchack April 7, 2026 14:54 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes

seanbudd closed this Apr 7, 2026

seanbudd mentioned this pull request Apr 7, 2026

initial commit #19913

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

preparing the new magnifier types#19915

preparing the new magnifier types#19915
Boumtchack wants to merge 1 commit intonvaccess:masterfrom
France-Travail:NewMagTypes

Boumtchack commented Apr 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

seanbudd commented Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-	def _doUpdate(self):
-		super()._doUpdate()
+	def _doUpdate(self) -> None:
+		# Placeholder magnifier has no backing implementation to refresh.
+		# Intentionally do nothing instead of delegating to the abstract
+		# base implementation, which raises NotImplementedError.
+		return

	self._magnifierType: MagnifierType
	self._magnifierType: MagnifierType \| None = None

	"Cycle through Magnifier type",
	"Cycle through magnifier types",

Uh oh!

Conversation

Boumtchack commented Apr 7, 2026

Link to issue number:

Summary of the issue:

Description of user facing changes:

Description of developer facing changes:

Description of development approach:

Testing strategy:

Known issues with pull request:

Code Review Checklist:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

seanbudd commented Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants