Finish and ship unified document zoom — #529 - #552
Merged
Conversation
Provide Actual Size (Cmd+0), Zoom In (Cmd++), and Zoom Out (Cmd+-) menu items plus a toolbar dropdown so users can rescale the rendered preview for readability, presentations, or larger displays. Zoom level persists across launches and snaps to preset increments: 50, 75, 90, 100, 110, 125, 150, and 200 percent.
Implements transient per-document zoom as requested in #335: - Cmd+ zooms in (10% increments, max 300%) - Cmd- zooms out (10% decrements, min 50%) - Cmd+0 resets to actual size - Zoom applies to both editor and preview when preference is enabled - Zoom is transient (not saved to preferences) - Menu items validate at zoom limits - Does not mutate base font preference Fixes #335
- Move kMinZoom/kMaxZoom to file-scope constants - Fix preview zoom when previewZoomRelativeToBaseFontSize is off - Fix floating-point comparison in resetZoom: validation
Adds MPZoomTests.m with 22 tests covering zoom multiplier basics, menu validation, preference observer behavior, and tab stop calculation. Two tests are expected to FAIL against current code as TDD regression guards: - testSetupEditorPreservesZoomedFontSize: guards against setupEditor: resetting the editor font to its unzoomed base size, clobbering any active zoom. - testTabStopsReflectZoomedFontSize: guards against tab stops being computed from the base font instead of the zoomed font. Related to #335
Bug 1: Cmd+0 conflict. "Actual Size" was bound to Cmd+0, which collides with Format > "Paragraph". Remove the shortcut from "Actual Size" (menu only) and change "Zoom In" from "+" to "=" so Cmd+= works without Shift on US keyboards (standard macOS zoom behavior). Bug 2: Editor font reset on preference changes. setupEditor: applied the raw base font via self.editor.font = font, clobbering any active zoom when the font, style, or line-spacing preference changed. Bug 3: Tab stops computed from base font. setupEditor: computed tab stops from the unzoomed base font's space width, so tabs appeared at the wrong width after zoom. Add a zoomedEditorFont helper that returns base font x zoomMultiplier. setupEditor: now uses it for both the tab stop calculation and the editor font assignment, fixing bugs 2 and 3 in one change. Simplify applyCurrentZoom to delegate to setupEditor: (which already calls scaleWebview), so zoom actions also refresh tab stops and the syntax highlighter's font cache. Related to #335
XIB: The previous fix moved Cmd+0 off "Actual Size" but introduced two new collisions - Cmd+= with Format > "Highlight" and Cmd+- with Format > "Strikethrough". Change "Zoom In" to Cmd+Plus (keyEquivalent "+") and add Shift+Cmd to "Zoom Out" so both use the Shift modifier and neither collides with the Format menu shortcuts. Performance: applyCurrentZoom previously routed through setupEditor:, which does far more than the zoom path needs - it deactivates and reactivates the PEG Markdown highlighter, re-reads and re-applies the stylesheet from disk, and replaces the editor's CALayer. Extract the font and paragraph-style logic into a dedicated applyEditorFontAndParagraphStyle helper that both setupEditor: and applyCurrentZoom call, so zoom keystrokes skip the highlighter re-parse and CALayer rebuild. Tests: remove outdated "expected to FAIL" comments now that the bugs are fixed, and drop testSetupEditorDoesNotResetZoomMultiplier, which only checked that a CGFloat property stayed unchanged across a method that never touched it - the real Bug 2 regression is covered by testSetupEditorPreservesZoomedFontSize. Related to #335
Extract previewScale from scaleWebview so the scale computation is unit-testable without mocking WebView. The PR's scaleWebview change removed the early-return when previewZoomRelativeToBaseFontSize is OFF, so add four tests that pin both branches of the calculation: - preference OFF + zoom 1.0 -> 1.0 (no-op equivalence) - preference OFF + non-default zoom -> tracks zoomMultiplier - preference ON + non-default zoom -> (fontSize/14) * zoomMultiplier - preference ON + zoom 1.0 -> matches legacy fontSize/14 ratio Each test saves and restores the preference values it touches. Related to #335
Unified document zoom claims Cmd+/Cmd-/Cmd0. Rather than dropping the three colliding Format shortcuts as the draft did, relocate them: - Strikethrough: Cmd- -> Cmd-Shift-X (Google Docs/Slack convention) - Paragraph: Cmd0 -> Cmd-Opt-0 (sits with the numeric heading family) - Highlight: drop the shortcut (optional extension feature; no free idiomatic home). Rebinding tracked as a follow-up. Zoom In/Out/Actual Size remain on Cmd+/Cmd-/Cmd0. Verified no new collisions: Cmd-Shift-X and Cmd-Opt-0 are otherwise unused. Related to #529
The zoom preset popup set each item's target to self.document at construction time, when the document outlet is still nil, so clicking a preset silently did nothing. Route the items through the controller (target=self) and resolve self.document lazily at click time in a new -documentZoomPopUpClicked: forwarder, matching the file's existing deferred-dispatch idiom. The forwarder sends selectDocumentZoom: directly (it is a declared IBAction visible via the import chain), which reads the level from the clicked item's representedObject. Related to #529
MPZoomTests.m carried three tests whose docstrings asserted a flat +/-0.1 stepping model. The shipped model is a non-uniform preset list (0.5, 0.75, 0.9, 1.0, 1.1, 1.25, 1.5, 2.0, 3.0) snapped by stepDocumentZoomDirection:, so that model is false even though the old tests' specific data points happened to line up. Replace them with tests that exercise the real snap-to-nearest-preset behavior from off-grid values via the public zoomIn:/zoomOut: actions (1.05 -> 1.1 up, 1.05 -> 1.0 down) and a corrected preference-cycle test (1.5 -> 2.0). All model-independent tests (menu validation, rendering, tab stops, previewScale) are kept unchanged. Net count preserved at 25. MPPreviewZoomTests.m remains authoritative. Related to #529
Two related zoom changes in MPDocument: Fix #4 (production side): the shared-preference KVO registration that drives cross-window zoom sharing lived inside windowControllerDidLoadNib:, so it never ran in headless tests. Extract it into -registerSharedPreferenceObservers / -unregisterSharedPreferenceObservers (called from windowControllerDidLoadNib: and, under the existing needsToUnregister guard, from -close). Pure extract-method: same key set, options, context, and ordering relative to the editor-KVO loop. This lets a headless test register the observer directly and assert propagation. Fix #5: resetZoom: and selectDocumentZoom: wrote documentZoomLevel directly, bypassing the bounds clamp in setZoomMultiplier:. Route both through self.zoomMultiplier so clamping lives in one place. setZoomMultiplier: persists the clamped value to the shared documentZoomLevel pref, so cross-window sharing is unaffected; behavior is unchanged for in-range presets. Related to #529
Prove the headline behavior: a zoom change in one document window propagates to others via the shared documentZoomLevel preference. The observer registration was extracted out of the nib path (prior commit) precisely so a headless test can drive it. The test spies on -applyCurrentZoom via an MPDocument subclass rather than asserting on -zoomMultiplier: the latter is a passthrough over the shared pref and would read the new value even if the observer never fired, proving nothing. Doc A writes the shared zoom; the test asserts Doc B's handler ran exactly once and observed the propagated value. Doc B registers observers outside the nib path, so -close (gated on needsToUnregister) will not tear them down; the test unregisters explicitly before releasing Doc B to avoid a KVO-dealloc crash. Related to #529
Contributor
Code Coverage ReportCurrent Coverage: 64.81% Coverage Details (Summary) |
schuyler
changed the title
This was referenced Aug 3, 2026
schuyler
added
the
rc-pending
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Takes over draft PR #518 and finishes it per the execution plan in #529. Unified document zoom becomes a single document-level control: ⌘+ / ⌘− / ⌘0 and a toolbar preset dropdown drive one shared zoom level that applies to every open document window and persists across launches.
This branch is #518's 8 zoom commits rebased onto current
main, plus five fix commits addressing the wiring bug, keybinding collisions, test hygiene, and testability called out in #529.Changes
MPDocument.m/MPDocument.h— Zoom constants and preset list (0.5–3.0).zoomMultiplierproperty with clamped setter. Preset stepping viastepDocumentZoomDirection:.zoomIn:/zoomOut:/resetZoom:/selectDocumentZoom:actions. RefactoredapplyEditorFontAndParagraphStylefor zoom-scaled editor font and tab stops. Preview re-zoom on WebView reload. KVO registration extracted intoregisterSharedPreferenceObservers/unregisterSharedPreferenceObserversfor cross-window sync and testability. Menu validation for zoom items.MPToolbarController.m— Zoom preset dropdown. Items targetselfwith deferred document resolution at click time (the outlet is nil at construction), forwarding toselectDocumentZoom:. KVO ondocumentZoomLevelkeeps the dropdown synced.MPPreferences.m/MPPreferences.h—documentZoomLevelproperty, default1.0, migration v6 (additive).MainMenu.xib— Added Zoom In (⌘+), Zoom Out (⌘−), Actual Size (⌘0) to View menu. Relocated colliding Format shortcuts:MPZoomTests.m(new) — 26 tests: multiplier basics, clamping, preset snap-step from off-grid values, menu validation, preference observers, tab-stop scaling, preview scale, cross-window propagation.MPPreviewZoomTests.m(new) — 8 tests: preset snap-step, default value, boundary clamp, actual-size reset.MPPreferencesTests.m— Migration version assertions updated (5 → 6).MPToolbarControllerTests.m— Toolbar item count assertions updated for the new zoom dropdown.Testing
Build and full test suite verified by macOS CI (this branch was prepared on Linux).
Follow-ups
stepDocumentZoomDirection:throughsetZoomMultiplier:for a single write path (safe as-is; consistency only).Related to #529, #518, #470, #335, #504, #519.