Fix preference pane layout for localized text wrapping#461
Merged
Conversation
The Preferences panes pinned their content to the hard-coded English XIB frame size, so longer localized strings (French, Italian) overlapped or were clipped regardless of window size. PR #399 bumped the fixed sizes but did not generalize. - MPPreferencesViewController.loadView now keeps the designed width but measures the height the content needs at that width for the active locale (Auto Layout fitting size), never shrinking below the English design height. Resizable + centered-content behavior is preserved. - Editor "Behavior" box: drop the fixed height=200 constraint and anchor the box content to its last checkbox so it hugs wrapped text. - Compilation/HTML pane: decouple the "CSS:" label width from the "Default path:" label (it forced the CSS column wide in French and overlapped the Theme controls); give the CSS label its own leading. - Enable word wrapping on checkbox titles across all panes. - Add tests asserting boxes hug content, label widths are independent, checkboxes wrap, panes are unambiguous, and content is sized to fit. Related to #397
67f64b6 to
7e3439f
Compare
Contributor
Code Coverage ReportCurrent Coverage: 61.61% Coverage Details (Summary) |
The locale-aware loadView forced a layout pass and activated constraints on
the NIB's content view before re-parenting it into the wrapper. That
corrupted the responder-chain setup performed by -setView:, raising
NSInternalInconsistencyException ("The next responder should never be
yourself!") whenever a pane's view was loaded — which the new tests do.
- loadView now re-parents and assigns the wrapper exactly as before, then
measures and grows the content height afterward, so responder setup runs
in the original (working) order.
- Pin the Editor "Behavior" box's leading edge so its width is fully
determined; its now-wrapping checkboxes can't introduce ambiguous layout.
Related to #397
Loading a pane's view in a unit test raised NSInternalInconsistencyException
("The next responder should never be yourself!"). When the NIB loads,
NSViewController points the content view's next responder at the controller;
re-homing that view inside the centering wrapper and re-pointing -view at the
wrapper leaves a responder cycle that trips the assertion the first time the
chain is traversed (e.g. when a test forces layout).
Detach the content view's next responder before adding it to the wrapper, and
restore a clean chain (content -> wrapper -> controller) after assigning the
wrapper as the view.
Related to #397
The earlier responder fix targeted -loadView, but the crash is actually raised by -[NSViewController dealloc]: tearing down a short-lived pane (as a unit test does) makes the superclass splice itself out of the responder chain the centering wrapper created, hitting "The next responder should never be yourself!". - Override -dealloc to detach both the controller's and the view's next responder before the superclass runs, so its cleanup is a no-op. (The running app keeps the panes for the process lifetime, so this only affected transient instances.) - Revert the -loadView next-responder pokes, which addressed the wrong phase. - Drop the Editor "Behavior" box leading pin: it conflicted with the box's existing leading chain (already pinned at the 20pt margin), and the box width was already fully determined without it. Related to #397
testGroupingBoxesHaveNoFixedHeight matched AppKit's auto-generated NSContentSizeLayoutConstraint (every NSBox's low-priority intrinsic-height placeholder), not just author-set pins. Restrict the check to required-priority constraints, which is what an actual fixed-height pin (the removed Editor height==200) uses. Related to #397
The priority-based filter for testGroupingBoxesHaveNoFixedHeight was not a reliable way to skip AppKit's auto-generated NSContentSizeLayoutConstraint (its reported priority varies by SDK). Distinguish author-set pins from the private NSLayoutConstraint subclasses by class membership instead, so the intrinsic-height constraint every NSBox carries is ignored. Related to #397
This was referenced Jun 22, 2026
schuyler
added a commit
that referenced
this pull request
Jun 22, 2026
The HTML preferences pane was 430pt wide, which clipped the CSS theme popup and its Reveal/Reload controls — most visible now that the bundled "GitHub Dark Default" theme ships a long name. The pane is constraint- driven and loadView pins each pane to its XIB design width, so widening the content view to 482pt gives the theme controls room; the inner controls reflow automatically. Adds a regression test guarding the width. Reproduces the HTML-pane fix from yusufm's #419 on top of #461, preserving #461's checkbox word-wrap. Related to #419. Co-authored-by: yusufm <[email protected]>
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
Fixes preference pane layouts to properly accommodate localized text that wraps onto multiple lines, preventing text clipping in languages like French and Italian. This addresses Issue #397.
Key Changes
Dynamic height calculation in
MPPreferencesViewController.loadView(): Modified to measure the actual height needed by content at its designed width, accounting for text wrapping in different locales. The panel now grows vertically to fit wrapped text instead of using a fixed English design height.Checkbox text wrapping enabled: Added
lineBreakMode="wordWrap"to all checkbox buttons across preference panes (General, Markdown, Editor, Html) so long localized titles wrap to multiple lines instead of being truncated or clipped.Comprehensive locale-aware layout tests: Added four new test methods to
MPPreferencesViewControllerResizabilityTests:testGroupingBoxesHaveNoFixedHeight: Ensures grouping boxes hug their content rather than pinning fixed heightstestHtmlLabelWidthsAreIndependent: Verifies label widths aren't coupled, preventing layout overlap with longer localized texttestCheckboxTitlesWrap: Confirms all checkboxes have word wrapping enabledtestPanesHaveNoAmbiguousLayout: Validates no pane has ambiguous Auto Layout constraintstestContentIsSizedToFitItsContent: Verifies content is sized to fit at the active localeHelper utilities: Added static helper functions for view tree traversal and analysis (
MPCollectViews,MPFirstAmbiguousView,MPContentView,MPCheckboxes) to support the new tests.Implementation Details
The core fix in
loadView()now:fittingSizeat that widthThis ensures preference panes expand vertically for longer localized text while maintaining the designed width and never shrinking below the English baseline.