You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Finish and ship unified document zoom (take over #518) — v3000.0.8 #529
Ship unified document zoom as a v3000.0.8 headline feature by taking over draft PR #518 (@leevi2010-cursor, "Unify editor and preview zoom") and finishing it maintainer-side. The design is specified in #470 (original request #335). The architecture in #518 was reviewed and judged sound — the remaining work is wiring bugs, a keybinding decision, and test hygiene, not a redesign.
This issue is the execution plan. Implementation has not started.
#518 is 9 zoom commits off main plus a duplicated tip commit (106aff0) that is byte-identical to PR #519's sole commit ("Remember editor pane visibility"). That change (overloading the user-facing editorStartInPreviewMode preference) has an unresolved design concern and is being handled separately on #519.
Branch our own work off origin/main from Unify editor and preview zoom #518's zoom commits, dropping the 106aff0 tip commit (e.g. rebase omitting it). The Remember editor pane visibility #519 content — both the -toggleEditorPane: change and its MPPaneToggleTests.m test — is a fully isolated tip commit with zero interweaving into the zoom code, so this is a clean drop.
1. Toolbar zoom dropdown does nothing (functional blocker)
MPToolbarController.m's new toolbarItemDocumentZoomPopUpWithIdentifier:label: sets added.target = self.document at construction time, when the document outlet is still nil, so clicking a preset is a silent no-op.
Route the popup through the file's existing deferred-dispatch idiom (standaloneToolbarItemClicked: / dropdownMenuItemClicked:): set target = self and resolve self.document lazily at click time. selectDocumentZoom: already accepts both NSPopUpButton and NSMenuItem senders and reads the level from representedObject, so forwarding is a few lines.
2. Format-shortcut relocation (decided)
Zoom takes ⌘+ / ⌘− / ⌘0, which collide with three Format items. #518 currently just deletes those bindings. Decision, grounded in cross-editor convention and MacDown's own free keys:
Item (id)
Was
New binding
Rationale
Strikethrough (9CN-Qi-Fln)
⌘−
⌘⇧X
Google Docs / Slack convention; ⌘⇧X is free in the menu.
Paragraph (xN5-GU-ASF)
⌘0
⌘⌥0
Google Docs "normal text"; free; sits with the numeric heading-level family (Header 1–6 = ⌘1–⌘6).
Highlight (2Os-ij-Aup)
⌘=
drop shortcut
No free idiomatic home — ⌘⇧H is taken by "Hide Preview Pane" and ⌘⇧= is the Zoom In key. Highlight is an optional extension feature (hidden unless extensionHighlight is enabled), so it's the lowest-stakes drop. See follow-up below.
Apply the three xib changes in MainMenu.xib; add the Zoom In / Zoom Out / Actual Size items on ⌘+ / ⌘− / ⌘0.
Verify no new collisions (⌘⇧X and ⌘⌥0 confirmed free against the current xib).
3. Reconcile the two contradictory zoom test suites
MPPreviewZoomTests.m (preset-snapping on a shared preference) matches the shipped implementation and is authoritative. MPZoomTests.m mixes ~3 tests that encode a false flat-±0.1 stepping model with a majority of model-independent tests (menu validation, zoomed-font/tab-stop rendering, previewScale).
Delete the ~3 wrong-model stepping tests; keep the rendering/validation tests (consider renaming the file to signal it covers rendering, not stepping).
Confirm CI green with the retained suite; test count ≥ baseline.
4. Cross-window zoom sharing test
The mechanism works for real open windows (documents observe NSUserDefaultsDidChangeNotification → applyCurrentZoom), but the observer is registered in windowControllerDidLoadNib:, so headless tests can't see propagation and none currently does.
Recommended: small refactor extracting the zoom-observer registration out of the nib-load path so a headless test can assert doc A's zoom change propagates to doc B. This proves the headline behavior.
Fallback if the refactor is riskier than it looks: a thin shared-preference test plus manual cross-window verification before the RC. Decision to confirm when implementation starts.
5. Quick include: clamp bypass
resetZoom: / selectDocumentZoom: write documentZoomLevel directly, bypassing the clamp in setZoomMultiplier:. Safe today only because presets match the bounds.
Route these through the clamping setter so bounds enforcement lives in one place.
Release / merge notes
Migration:Unify editor and preview zoom #518 bumps preference migration to v6 (sets documentZoomLevel = 1.0). Purely additive, touches no other keys — low RC risk.
Debug build succeeds; full test suite green; test count ≥ baseline.
Toolbar zoom dropdown actually re-zooms the document when a preset is clicked.
⌘+ / ⌘− / ⌘0 drive zoom; Strikethrough (⌘⇧X) and Paragraph (⌘⌥0) work at their new bindings; Highlight's drop is intentional and documented.
Zoom level is shared across open document windows and persists across launches.
Migration v6 applies cleanly for existing users.
Rule-of-Two review passed before merge.
Follow-ups (not this issue)
Rebind Highlight to a sensible shortcut (its own issue).
Optional future parity: move headings from ⌘1–⌘6 to ⌘⌥1–⌘⌥6 (Google Docs scheme), which would let Paragraph ⌘⌥0 sit with its siblings. Out of scope for the zoom PR.
I'd totally love having ⌘+ / ⌘− / ⌘0 do zooming (at least in the preview pane).
FWIW, personally, I'd be ok if I had to first click in the preview pane to remove cursor focus from the editor (e.g., if there are other conflicting expectations about certain keyboard shortcuts in the editor pane; I don't know about that myself, as I'd anticipate zoom to do both panes and from anywhere, but that's not my primary concern in my comment at this time).
In any case, a simple Mac paradigm way to get basic zooming of the rendered markdown would be fantastic for my reading comfort.
I love seeing this recent consolidation issue. It looks like some real effort and thought have gone into zooming, showing people want zooming. And this issue seems to help forge the way ahead. Keep it up!
As a current workaround, I do see that the "Scale preview based on editor font size" option exists in Settings/Preferences > Rendering. I'm grateful for that. I believe ⌘+ / ⌘− / ⌘0 (or whatever zooming alternative) would be great for adjustments on the fly.
Goal
Ship unified document zoom as a v3000.0.8 headline feature by taking over draft PR #518 (@leevi2010-cursor, "Unify editor and preview zoom") and finishing it maintainer-side. The design is specified in #470 (original request #335). The architecture in #518 was reviewed and judged sound — the remaining work is wiring bugs, a keybinding decision, and test hygiene, not a redesign.
This issue is the execution plan. Implementation has not started.
Branch plan — de-entangle from #519 first
#518 is 9 zoom commits off
mainplus a duplicated tip commit (106aff0) that is byte-identical to PR #519's sole commit ("Remember editor pane visibility"). That change (overloading the user-facingeditorStartInPreviewModepreference) has an unresolved design concern and is being handled separately on #519.origin/mainfrom Unify editor and preview zoom #518's zoom commits, dropping the106aff0tip commit (e.g. rebase omitting it). The Remember editor pane visibility #519 content — both the-toggleEditorPane:change and itsMPPaneToggleTests.mtest — is a fully isolated tip commit with zero interweaving into the zoom code, so this is a clean drop.Fixes
1. Toolbar zoom dropdown does nothing (functional blocker)
MPToolbarController.m's newtoolbarItemDocumentZoomPopUpWithIdentifier:label:setsadded.target = self.documentat construction time, when thedocumentoutlet is still nil, so clicking a preset is a silent no-op.standaloneToolbarItemClicked:/dropdownMenuItemClicked:): settarget = selfand resolveself.documentlazily at click time.selectDocumentZoom:already accepts bothNSPopUpButtonandNSMenuItemsenders and reads the level fromrepresentedObject, so forwarding is a few lines.2. Format-shortcut relocation (decided)
Zoom takes ⌘+ / ⌘− / ⌘0, which collide with three Format items. #518 currently just deletes those bindings. Decision, grounded in cross-editor convention and MacDown's own free keys:
9CN-Qi-Fln)xN5-GU-ASF)2Os-ij-Aup)extensionHighlightis enabled), so it's the lowest-stakes drop. See follow-up below.MainMenu.xib; add the Zoom In / Zoom Out / Actual Size items on ⌘+ / ⌘− / ⌘0.3. Reconcile the two contradictory zoom test suites
MPPreviewZoomTests.m(preset-snapping on a shared preference) matches the shipped implementation and is authoritative.MPZoomTests.mmixes ~3 tests that encode a false flat-±0.1 stepping model with a majority of model-independent tests (menu validation, zoomed-font/tab-stop rendering,previewScale).4. Cross-window zoom sharing test
The mechanism works for real open windows (documents observe
NSUserDefaultsDidChangeNotification→applyCurrentZoom), but the observer is registered inwindowControllerDidLoadNib:, so headless tests can't see propagation and none currently does.5. Quick include: clamp bypass
resetZoom:/selectDocumentZoom:writedocumentZoomLeveldirectly, bypassing the clamp insetZoomMultiplier:. Safe today only because presets match the bounds.Release / merge notes
documentZoomLevel = 1.0). Purely additive, touches no other keys — low RC risk.MacDown 3000.xcodeproj/project.pbxprojconflict against the Internal anchor links are not clickable in exported PDF (inherited from upstream MacDown) #504 work already on the milestone branch (both add to different subsections). No source-file conflicts.Acceptance criteria
Follow-ups (not this issue)
Related to #470, #335, #518, #519, #504.