Address issue #499: force toolbar redraw after preference pane switch#500
Merged
Conversation
MPPreferencesViewController's -loadView (reworked in #481/#498 for locale-aware dynamic sizing) resolves each pane's wrapper view across several Auto Layout passes before finally assigning self.view. That delay lets the moment a pane's view lands in the window slip past when MASPreferencesWindowController updates the toolbar's selectedItemIdentifier during a tab switch, leaving the toolbar's selection highlight stuck on the first tab even though the correct pane is displayed. Add a -viewDidAppear override that forces the toolbar to revalidate and the window to redraw once the pane is actually visible, without touching the two-pass sizing logic itself. Related to #499
Contributor
Code Coverage ReportCurrent Coverage: 63.23% Coverage Details (Summary) |
schuyler
added
bug
schuyler
added a commit
that referenced
this pull request
Jul 1, 2026
…#500) ## Summary The Preferences window's toolbar tab highlight was getting stuck on the first tab (General) when switching panes — pane content and window resizing worked correctly, but the toolbar's selected-tab highlight never visually moved off the first item. This was a regression from the locale-aware dynamic pane-sizing rework in #481 and #498, which changed `MPPreferencesViewController`'s `-loadView` to resolve each pane's wrapper view across several Auto Layout passes before finally assigning `self.view`. That extra work delays the moment a pane's view actually lands in the window relative to when the vendored `MASPreferencesWindowController` updates the toolbar's `selectedItemIdentifier` during a tab switch, leaving the toolbar's highlight stale even though the identifier itself is set correctly. - Added a `-viewDidAppear` override to `MPPreferencesViewController` that forces the toolbar to revalidate (`validateVisibleItems`) and the window to redraw (`displayIfNeeded`) once a pane's view is actually visible, without touching the two-pass sizing logic that fixed real localization clipping bugs in #461/#481/#498. - Added a doc-comment noting that subclasses overriding `-viewDidAppear` must call `super`, or this fix is silently skipped for that pane (verified none of the five pane subclasses currently override `-viewDidAppear`; they only override `-viewWillAppear`). - Added a regression test exercising the new override directly against all five panes. ## Related Issue Related to #499 ## Manual Testing Plan This session ran in a Linux sandbox with no Xcode/macOS, so the fix could not be visually verified here — CI runs the automated suite, but toolbar highlight rendering isn't observable through any public API a headless test can assert on. Please verify manually on macOS: 1. **Setup:** Build and run MacDown 3000, open Preferences (⌘,). Toolbar should show General, Markdown, Editor, Html, Terminal. 2. **Core scenarios** — for each, confirm both the correct pane loads *and* the toolbar highlight moves to the clicked tab with no lag: - Forward order: General → Markdown → Editor → Html → Terminal - Backward order: Terminal → Html → Editor → Markdown → General - Skipping around: General → Terminal → Markdown → Html → Editor → General - Re-click the same tab twice in a row — highlight should stay put, no flicker - Close and reopen Preferences — should reopen on the last-used pane with correct highlight 3. **Edge cases:** - Rapid-fire clicking through all 5 tabs - Clicking a tab while the window is still mid-resize from the previous switch - Keyboard tab switching if bound (MASPreferences' goNextTab/goPreviousTab) - Watch specifically for new jank from the added `displayIfNeeded` call (double-flicker, brief flash of wrong highlight, redraw stutter) - Manually resize the Preferences window after switching tabs 4. **Locale:** The fix itself isn't locale-specific, but since #481/#498 were locale-driven, it's worth repeating scenario 2 once under Italian (the originally reported repro locale) to confirm both the highlight bug stays fixed and localized pane resizing still works correctly alongside it. ## Review Notes - Architectural review confirmed the vendored `MASPreferencesWindowController` (CocoaPods, unmodified) already sets `toolbar.selectedItemIdentifier` correctly on every pane switch, so the fix targets our own `-loadView`/view-lifecycle timing rather than the vendor pod. - Code review confirmed `-viewDidAppear` is the correct lifecycle hook (fires after the view lands in the window, unlike `-viewWillAppear` which fires earlier in the same content-view-swap call), and that none of the five pane subclasses currently shadow it. - Documentation review of `plans/` found no existing content referencing this behavior that needed updating. - Note: the new regression test only proves the override doesn't crash and runs for all five panes — it cannot prove the toolbar highlight actually repaints, since that requires a real window server session. Manual verification per the plan above is the actual confirmation this bug is fixed.
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
2 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
The Preferences window's toolbar tab highlight was getting stuck on the first tab (General) when switching panes — pane content and window resizing worked correctly, but the toolbar's selected-tab highlight never visually moved off the first item. This was a regression from the locale-aware dynamic pane-sizing rework in #481 and #498, which changed
MPPreferencesViewController's-loadViewto resolve each pane's wrapper view across several Auto Layout passes before finally assigningself.view. That extra work delays the moment a pane's view actually lands in the window relative to when the vendoredMASPreferencesWindowControllerupdates the toolbar'sselectedItemIdentifierduring a tab switch, leaving the toolbar's highlight stale even though the identifier itself is set correctly.-viewDidAppearoverride toMPPreferencesViewControllerthat forces the toolbar to revalidate (validateVisibleItems) and the window to redraw (displayIfNeeded) once a pane's view is actually visible, without touching the two-pass sizing logic that fixed real localization clipping bugs in Fix preference pane layout for localized text wrapping #461/Dynamic preferences pane sizing for localized strings #481/Fix checkbox label clipping in localized preference panes #498.-viewDidAppearmust callsuper, or this fix is silently skipped for that pane (verified none of the five pane subclasses currently override-viewDidAppear; they only override-viewWillAppear).Related Issue
Related to #499
Manual Testing Plan
This session ran in a Linux sandbox with no Xcode/macOS, so the fix could not be visually verified here — CI runs the automated suite, but toolbar highlight rendering isn't observable through any public API a headless test can assert on. Please verify manually on macOS:
displayIfNeededcall (double-flicker, brief flash of wrong highlight, redraw stutter)Review Notes
MASPreferencesWindowController(CocoaPods, unmodified) already setstoolbar.selectedItemIdentifiercorrectly on every pane switch, so the fix targets our own-loadView/view-lifecycle timing rather than the vendor pod.-viewDidAppearis the correct lifecycle hook (fires after the view lands in the window, unlike-viewWillAppearwhich fires earlier in the same content-view-swap call), and that none of the five pane subclasses currently shadow it.plans/found no existing content referencing this behavior that needed updating.Generated by Claude Code