Centre tab titles and close buttons in the tab bar - #46
Merged
Conversation
The title and close button were positioned by pinning their bottom edges 3pt and 4pt above the bottom of the tab. Those insets only centre a 16pt label and a 13pt button when the tab is 22pt tall, which is what OakTabBarView's intrinsicContentSize asks for. It never gets 22pt. The tab bar is a titlebar accessory, and AppKit fixes those at 36pt no matter what the view requests -- asking for 10, 23, 40 or 60 all come back 36, and no window configuration changes it. Tabs fill the accessory, so they are 35pt, and the two bottom-pinned subviews ended up well below the middle of them. Constrain both to the tab's centre instead, which holds at whatever height AppKit hands us, plus a point so they sit fractionally low rather than dead centre.
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.
Closes #37.
The bug
The tab title and close button were positioned by pinning their bottom edges 3pt and 4pt above the bottom of the tab:
Those insets centre a 16pt label and a 13pt button only when the tab is 22pt tall, which is what
OakTabBarView.mm:720'sintrinsicContentSizeasks for.It never gets 22pt. The tab bar is an
NSTitlebarAccessoryViewControllerview (DocumentWindowController.mm:208-212), and AppKit fixes those at a height of its own choosing. Probing it on macOS 26.5 (25F71):The request is discarded entirely — it is neither a floor nor a ceiling. No window configuration moves it either:
titlebarAppearsTransparent,titleVisibility,NSWindowStyleMaskFullSizeContentView, and both unified toolbar styles all give 36. OnlylayoutAttributeRight or Left differ (32pt), and those place the accessory inline beside the window title, which is no use for a tab bar.So the accessory is 36pt, tabs fill it at 35pt (
NSHeight(self.bounds)-1), and the two bottom-pinned subviews sat well below the middle of them.Measurements
Clear space above and below the title ink, in device pixels, in a 70px tab, taken from window captures at 2x:
The close button measures 28/28 after the change. Both figures land within a pixel of tagliala's build, which is the reference the issue points at.
The change
Both subviews are constrained to the tab's centre, which holds at whatever height AppKit hands us, with a 1pt constant so they sit fractionally low rather than dead centre — that reads better at this tab height.
Relationship to the upstream PRs
The issue links two PRs on tagliala's fork.
V:[close]-(10)-|andV:[title]-(9)-|, values that fork introduced as a workaround with a TODO admitting as much. This tree still has the original 4 and 3.±1insetCurrentLayout's frame arithmetic. It does not touch the pinning, so it does not address the cause; applied on its own here it moves the title 0.5pt further from centre and the close button 1pt. Not taken.Worth noting separately
intrinsicContentSizereturning 23 has no effect on the rendered height and will mislead the next person to read it. tagliala's issue #15 ("the tab navigation bar is too tall") is unfixable from that direction for the same reason — they observed that changingintrinsicContentSizedid nothing, and this is why. Left alone here to keep the change to the reported bug.