Skip to content

Fix preference pane layout for localized text wrapping#461

Merged
schuyler merged 6 commits into
mainfrom
claude/admiring-wright-0ujwm5
Jun 22, 2026
Merged

Fix preference pane layout for localized text wrapping#461
schuyler merged 6 commits into
mainfrom
claude/admiring-wright-0ujwm5

Conversation

@schuyler

@schuyler schuyler commented Jun 22, 2026

Copy link
Copy Markdown
Owner

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 heights
    • testHtmlLabelWidthsAreIndependent: Verifies label widths aren't coupled, preventing layout overlap with longer localized text
    • testCheckboxTitlesWrap: Confirms all checkboxes have word wrapping enabled
    • testPanesHaveNoAmbiguousLayout: Validates no pane has ambiguous Auto Layout constraints
    • testContentIsSizedToFitItsContent: Verifies content is sized to fit at the active locale
  • Helper 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:

  1. Pins the content view width to the designed width
  2. Measures the actual height needed via fittingSize at that width
  3. Uses the maximum of the measured height and English design height (as a floor)
  4. Applies this calculated height to the content view constraint

This ensures preference panes expand vertically for longer localized text while maintaining the designed width and never shrinking below the English baseline.

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
@schuyler schuyler force-pushed the claude/admiring-wright-0ujwm5 branch from 67f64b6 to 7e3439f Compare June 22, 2026 08:15
@github-actions

github-actions Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Report

Current Coverage: 61.61%

Coverage Details (Summary)
Name                                                                                                                                   Coverage            
-------------------------------------------------------------------------------------------------------------------------------------- ------------------- 
MASPreferences.bundle                                                                                                                  0.00% (0/0)         
MacDown 3000.app                                                                                                                       63.07% (8980/14238) 
    /Users/runner/work/macdown3000/macdown3000/MacDown/Code/Document/MPAsset.m                                                         93.69% (104/111)    
        -[MPAsset typeName]                                                                                                            100.00% (3/3)       
        -[MPAsset defaultTypeName]                                                                                                     100.00% (3/3)       
        +[MPAsset assetWithURL:andType:]                                                                                               100.00% (3/3)       
        -[MPAsset initWithURL:andType:]                                                                                                87.50% (7/8)        
        -[MPAsset init]                                                                                                                100.00% (3/3)       
        -[MPAsset templateForOption:]                                                                                                  0.00% (0/6)         
        -[MPAsset htmlForOption:]                                                                                                      100.00% (28/28)     
        -[MPStyleSheet defaultTypeName]                                                                                                100.00% (3/3)       
        +[MPStyleSheet CSSWithURL:]                                                                                                    100.00% (3/3)       
        -[MPStyleSheet templateForOption:]                                                                                             100.00% (20/20)     
        -[MPScript defaultTypeName]                                                                                                    100.00% (3/3)       
        +[MPScript javaScriptWithURL:]                                                                                                 100.00% (3/3)       
        -[MPScript templateForOption:]                                                                                                 100.00% (20/20)     
        -[MPEmbeddedScript htmlForOption:]                                                                                             100.00% (5/5)       
    /Users/runner/work/macdown3000/macdown3000/MacDown/Code/Utility/MPMathJaxListener.m                                                61.54% (16/26)      
        -[MPMathJaxListener callbacks]                                                                                                 100.00% (5/5)       
        -[MPMathJaxListener addCallback:forKey:]                                                                                       100.00% (3/3)       
        -[MPMathJaxListener invokeCallbackForKey:]                                                                                     100.00% (8/8)       
        +[MPMathJaxListener isSelectorExcludedFromWebScript:]                                                                          0.00% (0/5)         
        +[MPMathJaxListener isKeyExcludedFromWebScript:]                                                                               0.00% (0/5)         
    /Users/runner/work/macdown3000/macdown3000/Dependency/peg-markdown-highlight/HGMarkdownHighlightingStyle.m                         89.23% (58/65)      
        +[HGMarkdownHighlightingStyle colorFromARGBColor:]                                                                             100.00% (6/6)       
        -[HGMarkdownHighlightingStyle initWithType:attributesToAdd:toRemove:fontTraitsToAdd:]                                          88.89% (8/9)        
        -[HGMarkdownHighlightingStyle initWithStyleAttributes:baseFont:]                                                               88.00% (44/50)      
    /Users/runner/work/macdown3000/macdown3000/MacDown/Code/Utility/FileURLInlining.m                                                  26.60% (25/94)      
        +[FileURLInlining inlineFromIterable:]                                                                                         0.00% (0/11)        
        -[FileURLInlining initWithURL:]                                                                                                0.00% (0/6)         
        -[FileURLInlining init]                                                                                                        100.00% (3/3)       
        -[FileURLInlining inlineContent]                                                                                               0.00% (0/9)         
        -[FileURLInlining imageContent]                                                                                                0.00% (0/12)        
        +[FileURLInlining mimeTypeForFilePath:]                                                                                        100.00% (22/22)     
        -[FileURLInlining clippingContent]                                                                                             0.00% (0/4)         
        +[FileURLInlining isTextClippingAtPath:]                                                                                       0.00% (0/12)        
        +[FileURLInlining textContentFromClipping:]                                                                                    0.00% (0/15)        
    /Users/runner/work/macdown3000/macdown3000/MacDown/Code/Utility/MPResourceWatcherSet.m                                             93.62% (88/94)      
        -[MPResourceWatcherSet init]                                                                                                   100.00% (6/6)       
        -[MPResourceWatcherSet dealloc]                                                                                                0.00% (0/3)         
        -[MPResourceWatcherSet watchedPaths]                                                                                           100.00% (3/3)       
        -[MPResourceWatcherSet updateWatchedPaths:]                                                                                    100.00% (16/16)     
        -[MPResourceWatcherSet addWatcherForPath:]                                                                                     100.00% (32/32)     
        __42-[MPResourceWatcherSet addWatcherForPath:]_block_invoke                                                                    100.00% (6/6)       
        __42-[MPResourceWatcherSet addWatcherForPath:]_block_invoke.5                                                                  93.33% (14/15)      
        __42-[MPResourceWatcherSet addWatcherForPath:]_block_invoke_2                                                                  75.00% (6/8)        
        -[MPResourceWatcherSet stopAll]                                                                                                100.00% (5/5)       
    /Users/runner/work/macdown3000/macdown3000/MacDown/Code/Extension/hoedown_html_patch.c                                             86.57% (116/134)    

... (3258 more lines truncated)

📊 **Full coverage report available in workflow artifacts**

claude added 5 commits June 22, 2026 08:29
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
@schuyler schuyler merged commit ec23690 into main Jun 22, 2026
9 checks passed
@schuyler schuyler deleted the claude/admiring-wright-0ujwm5 branch June 22, 2026 18:23
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.
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