Address issue #313: make toolbar fully delegate-driven so Flexible Space can be dropped#473
Merged
Merged
Conversation
…ace can be dropped The toolbar was configured both in MPDocument.xib (archived nib instances of Space and Flexible Space inside <allowedToolbarItems>) and via the MPToolbarController delegate. In that hybrid state AppKit reuses the single archived Flexible Space instance instead of vending a fresh one per drop, so dragging Flexible Space onto the toolbar was rejected even though it showed the green (+) on hover (regular Space happened to still work). Remove the XIB's <allowedToolbarItems> and empty <defaultToolbarItems/> so allowed/default/selectable items come entirely from the delegate, which already advertises Flexible Space, Space, and Separator. Also give the toolbar a stable identifier and enable allowsUserCustomization and autosavesConfiguration so customizations persist across launches. The delegate behavior is unchanged and remains covered by the existing MPToolbarControllerTests; the drag-drop fix itself is in the nib/AppKit layer and is verified manually. Related to #313
Contributor
Code Coverage ReportCurrent Coverage: 62.03% Coverage Details (Summary) |
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
Fixes the long-standing inability to drop a Flexible Space into the document toolbar via Customize Toolbar (issue #313). Regular Space dropped fine; Flexible Space showed the green (+) on hover but was rejected mid-drag — still broken as of v3000.0.6.
Root cause. The toolbar was configured in two places at once:
MPToolbarController(the delegate) already advertised Flexible Space, Space, and Separator intoolbarAllowedItemIdentifiers:(the #322 work), andMPDocument.xibseparately archived nib instances of Space and Flexible Space inside<allowedToolbarItems>. In that hybrid IB-configured + delegate-driven state, AppKit reuses the single archived Flexible Space instance instead of vending a fresh one per drop, so the drop is rejected. The earlier fix was only delegate-unit-tested, which couldn't catch a defect that lives in the nib/AppKit layer.The change (a single XIB edit,
MacDown/Localization/Base.lproj/MPDocument.xib):<allowedToolbarItems>(the archived Space + Flexible Space nib instances) and the empty<defaultToolbarItems/>, so allowed/default/selectable items come entirely from the delegate.implicitIdentifierwith a stableidentifier="MPDocumentToolbar".allowsUserCustomization="YES"andautosavesConfiguration="YES"so customizations persist across launches.The delegate behavior is unchanged and remains covered by the existing
MPToolbarControllerTests(~40 tests).Related Issue
Related to #313
Testing
MPToolbarControllerTestscontinue to lock down the delegate's allowed/default/selectable identifiers (including Flexible Space, Space, Separator). The delegate is unchanged, so no test changes were needed. The actual drag-drop fix is in the nib/AppKit layer and cannot be exercised by the macOS unit-test target.identifieris the correct persistence key, and there is no stale-config risk (nothing was ever persisted before).Manual Testing Plan (verify on a real macOS build)
testDefaultIdentifiersExactOrder.MPDocumentToolbaridentifier +autosavesConfiguration).Review Notes
Generated by Claude Code