Dynamic preferences pane sizing for localized strings#481
Merged
Conversation
loadView previously locked pane width to the English XIB frame, so localized strings longer than English (French, Italian) caused controls to clip and overlap. Replace the single-pass approach with a two-pass layout: first resolve width with a >= floor at the English design width, then resolve height at that width with a >= floor at the English design height. XIB changes complete the constraint graphs so fittingSize returns meaningful values: Editor gets leading/trailing pins instead of centerX on the Behavior box, Markdown gets bottom >= constraints on both columns, Terminal drops the fixed-width text field in favor of a trailing pin. Related to #397
Contributor
Code Coverage ReportCurrent Coverage: 62.63% Coverage Details (Summary) |
schuyler
added a commit
that referenced
this pull request
Jun 30, 2026
## Summary - Replace single-pass `loadView` (fixed width, grow height only) with two-pass dynamic sizing that resolves both width and height using `>=` floors at the English design dimensions - Fix incomplete constraint graphs in three XIBs so `fittingSize` returns meaningful values: Editor (leading/trailing instead of centerX), Markdown (bottom constraints on both columns), Terminal (trailing pin instead of fixed-width text field) - Add `testResolvedWidthIsWithinSaneBounds` sanity test and defensive assertions in existing layout test ## Test plan - [x] 1004 tests pass locally (0 unexpected failures) - [x] New `testResolvedWidthIsWithinSaneBounds` confirms all five panes resolve to sane widths (200–2000pt) - [x] Existing `testContentIsSizedToFitItsContent` confirms height accommodates content at resolved width - [ ] CI passes on macOS runner - [ ] Visual verification with French and Italian locales (reporters @rcuisnier and @massimo-cassandro validate in rc.2) Related to #397
This was referenced Jul 1, 2026
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.
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
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
loadView(fixed width, grow height only) with two-pass dynamic sizing that resolves both width and height using>=floors at the English design dimensionsfittingSizereturns meaningful values: Editor (leading/trailing instead of centerX), Markdown (bottom constraints on both columns), Terminal (trailing pin instead of fixed-width text field)testResolvedWidthIsWithinSaneBoundssanity test and defensive assertions in existing layout testTest plan
testResolvedWidthIsWithinSaneBoundsconfirms all five panes resolve to sane widths (200–2000pt)testContentIsSizedToFitItsContentconfirms height accommodates content at resolved widthRelated to #397