fix(toolbar): anchor each centre switcher to the capsule that opens it - #2678
Merged
Conversation
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.
The connection chooser and the container chooser both anchored to
MainWindowToolbar.connectionGroup, which is the whole centred group: two capsules wide. So both popovers opened on the seam between Connection and Database rather than under the one the user pressed.Measured
A compiled
NSToolbarharness, group of two bordered subitems,centeredItemIdentifiers = [group], window 1200pt:A subitem does resolve as a popover anchor and lands on its own capsule to within a point. The code did not do that because of a claim in its own comment:
NSToolbar.itemsholds groups only and a native group's subitems carry noview, so the subitem "cannot" anchor. The first half is true, the conclusion is not;NSPopover.show(relativeTo:)does not go throughtoolbar.items.There is a real trap, though. Once AppKit clips the group into the overflow menu its subitems have no view, and the same call raises
NSInvalidArgumentException("view has no window"), which Swift cannot catch. Sweeping the window width:toolbar.visibleItemsnames the group down to 640pt and stops at 600pt, and the throw begins at exactly the same width. The group itself keeps working there, because AppKit presents a clipped item from another affordance in the window.Change
ToolbarSwitcherPresenter.anchornow resolves an identifier that may name a subitem: the toolbar's own item first, then the subitem while its group is invisibleItems, then the group once it is not, then nil so a removed group still falls back to the floating panel. Switch Connection anchors toMainWindowToolbar.connection, the container and schema choosers toMainWindowToolbar.database.CompareEndpointToolbarControllershares the same resolver but its identifiers are top-level, so its behaviour is unchanged.Tests
Five cases in
ToolbarSwitcherAnchorTests, over a resolver seam that takes items and visible items rather than a live toolbar: subitem of a visible group, fallback to an overflowed group, group absent from the toolbar (what Customize Toolbar leaves behind, since neither subitem is an allowed identifier of its own), a clipped top-level item, and the window lookup end to end.visibleItemsnames a group even for a window never ordered front, measured, so the visible branch is reachable from a unit test.10/10 pass,
swiftlint --strictclean on the touched files, app builds.https://claude.ai/code/session_01DT51WAbwXF3AxSReQxNhnD