Skip to content

Add automatic appearance theme pairs#426

Open
yusufm wants to merge 1 commit into
schuyler:mainfrom
yusufm:Codex/add-auto-appearance-themes-019def52-b3da-7962-96cd-afa05d8d1592
Open

Add automatic appearance theme pairs#426
yusufm wants to merge 1 commit into
schuyler:mainfrom
yusufm:Codex/add-auto-appearance-themes-019def52-b3da-7962-96cd-afa05d8d1592

Conversation

@yusufm

@yusufm yusufm commented May 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add light/dark editor and rendering stylesheet pairs behind a View menu toggle
  • use the effective paired theme for editor highlighting, preview rendering, and exports
  • refresh open documents when macOS appearance changes
  • cover paired theme fallback and setter behavior in preferences tests

Tests

  • xcodebuild test -workspace "MacDown 3000.xcworkspace" -scheme MacDown -only-testing:MacDownTests/MPPreferencesTests -destination 'platform=macOS'
  • xcodebuild test -workspace "MacDown 3000.xcworkspace" -scheme MacDown -only-testing:MacDownTests/MPDocumentStyleUpdateTests -only-testing:MacDownTests/MPNotificationTests -destination 'platform=macOS'

Related to #292

@schuyler schuyler left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for the PR, @yusufm!

Nice preference modeling and good coverage on the resolver logic. The correctness risk is mostly in when and where the appearance is read.

Issues

  • Appearance-change race. usesDarkSystemAppearance reads NSApp.effectiveAppearance synchronously inside the AppleInterfaceThemeChangedNotification handler. That notification is known to fire before effectiveAppearance flips, so the first switch can apply the previous theme (off-by-one). Re-reading on the next runloop, or driving off -viewDidChangeEffectiveAppearance / KVO on effectiveAppearance, avoids it.
  • Private notification + app-only scope. AppleInterfaceThemeChangedNotification is undocumented and only fires for the system light/dark switch — it misses per-app appearance overrides. Observing the view/window's effectiveAppearance is public API and covers both.
  • Bundled theme names. The defaults reference @"Mou Night+" (editor dark) and @"Github2 (dark)" (html dark — note the lowercase "h" vs the light @"GitHub2"). If a stylesheet with that exact, case-sensitive name isn't in the bundle, effective…StyleName resolves to a missing style and silently falls back. Worth verifying the resources exist under those exact names.
  • No Preferences UI. The feature is reachable only via a View-menu item injected at runtime; the Preferences panes still show a single theme picker (whichever matches the current appearance). There's no checkbox and no way to see or set both halves of a pair at once — configuring the other appearance's theme requires switching the system appearance first. A paired toggle + light/dark pickers would make it discoverable.

Suggestions

  • Menu placement: every other menu item lives in MainMenu.xib; installAppearanceThemeMenuItem appends one programmatically. Defining it in the xib would be consistent and give control over its position.
  • setEditorStyleName:forDarkAppearance: always also overwrites editorStyleName, so later disabling "follow system" leaves the single value as whatever appearance you last edited — intentional fallback, but worth a comment.
  • The tests assert the selection logic with an explicit bool, but nothing covers the notification-driven refresh under a real appearance, so the race above would slip through.

Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants