Persist magnifier zoom, filter, full screen mode#20050
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates NVDA's magnifier module so gesture-driven changes are written back to the running configuration, aligning keyboard commands with how magnifier settings are expected to persist across restarts and re-enablement.
Changes:
- Persist zoom level changes by writing from the magnifier zoom setter into config.
- Persist filter changes when cycling filters from commands.
- Persist full-screen mode changes when cycling focus modes from commands.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
source/_magnifier/magnifier.py |
Writes zoom-level updates to config from the shared magnifier setter. |
source/_magnifier/commands.py |
Persists filter and full-screen mode changes triggered by keyboard commands. |
|
Hi @CyrilleB79 - do you mind testing this PR? I will fix the remaining items from #20148 in a separate PR |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
source/_magnifier/commands.py:203
- New persistence behavior is introduced for gesture-driven changes (zoom persists via
setZoomLevel, filter viasetFilter, fullscreen mode viasetFullscreenMode), but there are unit tests for magnifier commands already and none cover these new persistence calls. Please add/update unit tests to assert the relevant config setters are invoked when these gestures run (and that they don’t crash in non-fullscreen views).
def toggleFilter() -> None:
"""Cycle through color filters"""
magnifier: Magnifier = getMagnifier()
log.debug(f"Toggling filter for magnifier: {magnifier}")
if magnifierIsActiveVerify(
magnifier,
MagnifierAction.TOGGLE_FILTER,
):
assert isinstance(magnifier, FullScreenMagnifier)
filters = list(Filter)
idx = filters.index(magnifier.filterType)
magnifier.filterType = filters[(idx + 1) % len(filters)]
if magnifier._MAGNIFIED_VIEW == MagnifiedView.FULLSCREEN:
fullscreenMagnifier: FullScreenMagnifier = magnifier
fullscreenMagnifier._applyFilter()
setFilter(magnifier.filterType)
ui.message(
pgettext(
"magnifier",
# Translators: Message announced when changing the color filter with {filter} being the new color filter.
"Color filter changed to {filter}",
).format(filter=magnifier.filterType.displayString),
)
def cycleMagnifiedView() -> None:
"""Cycle through magnifier views (full-screen, fixed, docked, lens)"""
magnifier: Magnifier = getMagnifier()
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
Hi @seanbudd I have not been able to test on time before it is merged. I have opened a fix-up PR (#20236) to solve the case when one presses the Apply button. Another point: Though, #19765 is already present for a long time and people do not complain, so I guess it's acceptable to keep things as they are for Magnifier in beta/2026.2. |
…ory even if Discard button is then pressed (#20236) Fixes #20235 Fix-up of #20050 Summary of the issue: When pressing apply in Magnifier settings panel, the values were not kept in memory. Thus, if dialog was then discarded, the initial values memorized when the dialog was opened were restored instead of the ones after Aply has been pressed. Description of user facing changes: When Mag settings dialog is discarded, the values of parameters present in the dialog when Apply was pressed are now restored as expected. Description of developer facing changes: N/A Description of development approach: Updated the *initially variables when settings panel is saved, i.e. when Apply or OK are pressed. For OK, this has no impact since the dialog is closed.
Link to issue number:
Fixes #19493
Fixes #20110
Fixes #20149
Summary of the issue:
Magnifier zoom, filter and full screen mode were not persisted when set via keyboard gestures
Description of user facing changes:
Persist Magnifier zoom, filter and full screen mode in settings
Description of developer facing changes:
none
Description of development approach:
Settings Synchronization and Immediate Application:
_applyCurrentSettingsToConfigAndRuntimemethod toMagnifierPanel, which applies the current UI control values to both the configuration and the active magnifier instance, ensuring settings changes are reflected immediately. Immediate updates are now handled for zoom, pan step, filter, fullscreen mode, "true centered", follow focus, and "keep mouse centered" settings.onSaveandonDiscardmethods now use the new synchronization logic, improving reliability when saving or discarding changes. Initial values are stored and restored as needed.Magnifier Runtime Updates:
setFilter,setFullscreenMode,setZoomLevel) is now called to ensure the running magnifier instance is updated immediately.Codebase Cleanup and Modernization:
getDefaultFullscreenModeandsetDefaultFullscreenModefunctions from the config module, aTesting Improvements:
zoomLeveland_filterType, improving test reliability and clarity.Testing strategy:
manual testing
Known issues with pull request:
none
Code Review Checklist: