Skip to content

Commit 68a74b7

Browse files
committed
Refactor window layout calculation into iTermLayoutCalculator. Issue 12724
Extract layout logic from iTermRootTerminalView into a pure iTermLayoutCalculator class for testability. This fixes a bug where the tab bar height was double-deducted when calculating the toolbelt frame in fullscreen. The bug occurred because two mechanisms could both subtract tab bar height: 1. shouldLeaveEmptyAreaAtTop added it to decorationHeightTop 2. tabViewFrameByShrinkingForFullScreenTabBar shrunk the frame These are now mutually exclusive - if shouldLeaveEmptyAreaAtTop is true, the frame shrinking is skipped. Also rename rootTerminalViewWindowHasFullSizeContentView to rootTerminalViewFullScreenTabBarAccessoryOverlapsContent for clarity.
1 parent 0ce48bf commit 68a74b7

File tree

8 files changed

+1327
-228
lines changed

8 files changed

+1327
-228
lines changed

ModernTests/iTermLayoutCalculatorTest.swift

Lines changed: 564 additions & 0 deletions
Large diffs are not rendered by default.

iTerm2.xcodeproj/project.pbxproj

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -659,6 +659,7 @@
659659
4887C7B3CAB5465296225860 /* MenuTips.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 904AE312A0E0489899D9ECB8 /* MenuTips.xcassets */; };
660660
49A6E4091211CC6000D9AD6F /* Compatability.h in Headers */ = {isa = PBXBuildFile; fileRef = 49A6E4081211CC6000D9AD6F /* Compatability.h */; };
661661
4BC1E97CEA4441CF9390DD52 /* MenuTips.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 904AE312A0E0489899D9ECB8 /* MenuTips.xcassets */; };
662+
4C4BE71D1C8948BD8CD672BC /* iTermLayoutCalculator.m in Sources */ = {isa = PBXBuildFile; fileRef = AF19E441FF824539A275D8DA /* iTermLayoutCalculator.m */; };
662663
5300984C2259365B00A69348 /* iTermHapticActuator.h in Headers */ = {isa = PBXBuildFile; fileRef = 5300984A2259365B00A69348 /* iTermHapticActuator.h */; };
663664
5300984D2259365B00A69348 /* iTermHapticActuator.m in Sources */ = {isa = PBXBuildFile; fileRef = 5300984B2259365B00A69348 /* iTermHapticActuator.m */; };
664665
530098502259A44D00A69348 /* iTermSoundPlayer.h in Headers */ = {isa = PBXBuildFile; fileRef = 5300984E2259A44D00A69348 /* iTermSoundPlayer.h */; };
@@ -949,6 +950,7 @@
949950
9DB3D6F0176CCABE0071CCF8 /* PrefsArrangements.png in Resources */ = {isa = PBXBuildFile; fileRef = 9DB3D6EE176CCABE0071CCF8 /* PrefsArrangements.png */; };
950951
9DB3D6F1176CCABE0071CCF8 /* PrefsArrangements@2x.png in Resources */ = {isa = PBXBuildFile; fileRef = 9DB3D6EF176CCABE0071CCF8 /* PrefsArrangements@2x.png */; };
951952
A073973E14C768E400786414 /* ColorsMenuItemView.h in Headers */ = {isa = PBXBuildFile; fileRef = A073973D14C768E400786414 /* ColorsMenuItemView.h */; };
953+
A445EEDC85D346B39596FDC3 /* iTermLayoutCalculator.h in Headers */ = {isa = PBXBuildFile; fileRef = 93EDFB853885460AA504C6B9 /* iTermLayoutCalculator.h */; };
952954
A60014F618550F3900CE38D8 /* NSMutableAttributedString+iTerm.h in Headers */ = {isa = PBXBuildFile; fileRef = A60014F418550F3900CE38D8 /* NSMutableAttributedString+iTerm.h */; };
953955
A60014FB18552BDF00CE38D8 /* iTermNSKeyBindingEmulator.h in Headers */ = {isa = PBXBuildFile; fileRef = A60014F918552BDF00CE38D8 /* iTermNSKeyBindingEmulator.h */; };
954956
A600A93B2E3EDD5200D05EAD /* iTermBrowserTriggerHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = A67238B72E3EDC8E003227C3 /* iTermBrowserTriggerHandler.swift */; };
@@ -5127,6 +5129,7 @@
51275129
C6675EBC1C4FE96B0041173B /* iTermSelectorSwizzler.m in Sources */ = {isa = PBXBuildFile; fileRef = C6675EBB1C4FE96B0041173B /* iTermSelectorSwizzler.m */; };
51285130
CDCD9FF08FCE42F7A6963859 /* PillBackgroundGenerator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7EA289FFE2BF4090A65931BA /* PillBackgroundGenerator.swift */; };
51295131
D3CE2D4E1A00936F0098ED99 /* PSMDarkTabStyle.h in Headers */ = {isa = PBXBuildFile; fileRef = D3CE2D4C1A00936F0098ED99 /* PSMDarkTabStyle.h */; };
5132+
D5EF7F4036204859817200E0 /* iTermLayoutCalculator.h in Headers */ = {isa = PBXBuildFile; fileRef = 93EDFB853885460AA504C6B9 /* iTermLayoutCalculator.h */; };
51305133
DDF0FD65062916F70080EF74 /* iTermApplication.h in Headers */ = {isa = PBXBuildFile; fileRef = DDF0FD63062916F70080EF74 /* iTermApplication.h */; };
51315134
FB4CEC973C7E9235362E3F26 /* iTermRemotePreferences.h in Headers */ = {isa = PBXBuildFile; fileRef = FB4CECF4AC4392B21E87A07B /* iTermRemotePreferences.h */; };
51325135
/* End PBXBuildFile section */
@@ -6202,6 +6205,7 @@
62026205
904AE312A0E0489899D9ECB8 /* MenuTips.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; path = MenuTips.xcassets; sourceTree = "<group>"; };
62036206
90A1E139186F9EA4003EC3E8 /* AppleScriptTest.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = AppleScriptTest.h; sourceTree = "<group>"; };
62046207
90A1E13A186F9EA4003EC3E8 /* AppleScriptTest.m */ = {isa = PBXFileReference; fileEncoding = 4; indentWidth = 4; lastKnownFileType = sourcecode.c.objc; path = AppleScriptTest.m; sourceTree = "<group>"; };
6208+
93EDFB853885460AA504C6B9 /* iTermLayoutCalculator.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = iTermLayoutCalculator.h; sourceTree = "<group>"; };
62056209
9D737D5D17495B4100B3334D /* PrefsGeneral.png */ = {isa = PBXFileReference; lastKnownFileType = image.jpeg; name = PrefsGeneral.png; path = images/PrefsGeneral.png; sourceTree = "<group>"; };
62066210
9D737D5E17495B4100B3334D /* PrefsGeneral@2x.png */ = {isa = PBXFileReference; lastKnownFileType = image.jpeg; name = "PrefsGeneral@2x.png"; path = "images/PrefsGeneral@2x.png"; sourceTree = "<group>"; };
62076211
9DB3D6DE176C8F0B0071CCF8 /* PrefsAppearance.png */ = {isa = PBXFileReference; lastKnownFileType = image.jpeg; name = PrefsAppearance.png; path = images/PrefsAppearance.png; sourceTree = "<group>"; };
@@ -9090,6 +9094,7 @@
90909094
A6FFAE702CF123CC0031FB21 /* CoreTextLineRenderingHelper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CoreTextLineRenderingHelper.swift; sourceTree = "<group>"; };
90919095
A6FFAE722CF682B60031FB21 /* iTermDoubleWidthCharacterCache.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = iTermDoubleWidthCharacterCache.h; sourceTree = "<group>"; };
90929096
A6FFAE742CF682D40031FB21 /* iTermDoubleWidthCharacterCache.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = iTermDoubleWidthCharacterCache.m; sourceTree = "<group>"; };
9097+
AF19E441FF824539A275D8DA /* iTermLayoutCalculator.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = iTermLayoutCalculator.m; sourceTree = "<group>"; };
90939098
C6675EBA1C4FE96B0041173B /* iTermSelectorSwizzler.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = iTermSelectorSwizzler.h; sourceTree = "<group>"; };
90949099
C6675EBB1C4FE96B0041173B /* iTermSelectorSwizzler.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = iTermSelectorSwizzler.m; sourceTree = "<group>"; };
90959100
D3CE2D4C1A00936F0098ED99 /* PSMDarkTabStyle.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = PSMDarkTabStyle.h; sourceTree = "<group>"; };
@@ -9537,6 +9542,7 @@
95379542
FB4CECF4AC4392B21E87A07B /* iTermRemotePreferences.h */,
95389543
A6C537C11939507100A08C18 /* iTermRestorableSession.h */,
95399544
A6BDB0B91B470CBE00F511E6 /* iTermRootTerminalView.h */,
9545+
93EDFB853885460AA504C6B9 /* iTermLayoutCalculator.h */,
95409546
1D8CE03B195A143100FE1BEE /* iTermRule.h */,
95419547
A63BA39318A9CB43002BE075 /* iTermSelection.h */,
95429548
A6E77F791A23D1A5009B1CB6 /* iTermSelectionScrollHelper.h */,
@@ -10290,6 +10296,7 @@
1029010296
1DB40AA01B221028005B83C7 /* iTermNoColorAccessoryButton.m */,
1029110297
A682DE95190C5E7000BE8758 /* iTermProgressIndicator.m */,
1029210298
A6BDB0BA1B470CBE00F511E6 /* iTermRootTerminalView.m */,
10299+
AF19E441FF824539A275D8DA /* iTermLayoutCalculator.m */,
1029310300
1DDC093D1B4DB97500B1A910 /* iTermRoundedCornerScrollView.h */,
1029410301
1DDC093E1B4DB97500B1A910 /* iTermRoundedCornerScrollView.m */,
1029510302
A667F3951B4F71AE00705186 /* iTermSavePanel.h */,
@@ -15219,6 +15226,7 @@
1521915226
files = (
1522015227
A626DF0B217D3034005600F9 /* iTermTabBarAccessoryViewController.h in Headers */,
1522115228
A667190A1DCE36C3000CE608 /* iTermRootTerminalView.h in Headers */,
15229+
D5EF7F4036204859817200E0 /* iTermLayoutCalculator.h in Headers */,
1522215230
A667190B1DCE36C3000CE608 /* iTermCarbonHotKeyController.h in Headers */,
1522315231
A6E7352A20EBF3270034FCFD /* iTermSetCurrentTerminalHelper.h in Headers */,
1522415232
A6F66BA32C2E2C0700DC3569 /* iTermMutableOrderedSet.h in Headers */,
@@ -15770,6 +15778,7 @@
1577015778
buildActionMask = 2147483647;
1577115779
files = (
1577215780
A6BDB0BB1B470CBE00F511E6 /* iTermRootTerminalView.h in Headers */,
15781+
A445EEDC85D346B39596FDC3 /* iTermLayoutCalculator.h in Headers */,
1577315782
A6CEC11B1DCE8146009F4FD2 /* GPBCodedOutputStream.h in Headers */,
1577415783
A6CEC1251DCE8146009F4FD2 /* GPBProtocolBuffers.h in Headers */,
1577515784
A6CC16541CF012E300E8C148 /* iTermCarbonHotKeyController.h in Headers */,
@@ -19056,6 +19065,7 @@
1905619065
A69FF3CD2E1467D800AFF7A8 /* iTermFileProviderServiceError.swift in Sources */,
1905719066
A619DC4E2CD5A1D0004B8996 /* iTermCoreTextLineRenderingHelper.m in Sources */,
1905819067
A606CBF42145AF4800B3A97E /* iTermRootTerminalView.m in Sources */,
19068+
4C4BE71D1C8948BD8CD672BC /* iTermLayoutCalculator.m in Sources */,
1905919069
A605A1DB2EB292530072ED7E /* NSButton+iTerm.swift in Sources */,
1906019070
A65660D52372A4A600DC6744 /* iTermCache.m in Sources */,
1906119071
A6EC937524E78A5100EEADEF /* iTermEditSnippetWindowController.m in Sources */,

sources/PseudoTerminal.m

Lines changed: 40 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,10 +1156,6 @@ - (void)rootTerminalViewDidResizeContentArea {
11561156
[self fitTabsToWindow];
11571157
}
11581158

1159-
- (CGFloat)tabviewWidth {
1160-
return _contentView.tabviewWidth;
1161-
}
1162-
11631159
- (void)toggleBroadcastingToCurrentSession:(id)sender {
11641160
[_broadcastInputHelper toggleSession:self.currentSession.guid];
11651161
}
@@ -10456,10 +10452,29 @@ - (iTermStatusBarViewController *)rootTerminalViewSharedStatusBarViewController
1045610452
return self.currentSession.statusBarViewController;
1045710453
}
1045810454

10459-
- (BOOL)rootTerminalViewWindowHasFullSizeContentView {
10460-
return [PseudoTerminal windowTypeHasFullSizeContentView:self.windowType];
10461-
}
10462-
10455+
// Returns YES when the tab bar accessory in fullscreen overlaps the content area.
10456+
// This happens when the window uses NSWindowStyleMaskFullSizeContentView, which
10457+
// causes the content view to extend under the title bar area.
10458+
//
10459+
// This is used by tabViewFrameByShrinkingForFullScreenTabBar to determine if
10460+
// frame shrinking is needed. It is mutually exclusive with
10461+
// rootTerminalViewShouldLeaveEmptyAreaAtTop, which handles the transitional state.
10462+
- (BOOL)rootTerminalViewFullScreenTabBarAccessoryOverlapsContent {
10463+
// Check the actual window's styleMask rather than savedWindowType, because
10464+
// during window transitions the styleMask is the authoritative source.
10465+
return (self.window.styleMask & NSWindowStyleMaskFullSizeContentView) == NSWindowStyleMaskFullSizeContentView;
10466+
}
10467+
10468+
// Returns YES when empty space should be reserved at the top of the content area
10469+
// for a tab bar that is "on loan" to the titlebar accessory system but is not
10470+
// yet positioned as an accessory.
10471+
//
10472+
// This handles the transitional state during fullscreen entry/exit when:
10473+
// 1. The tab bar control has been borrowed (tabBarControlOnLoan == YES)
10474+
// 2. But it's not yet (or no longer) being shown as a titlebar accessory
10475+
//
10476+
// This is mutually exclusive with tabViewFrameByShrinkingForFullScreenTabBar,
10477+
// which handles the case when the tab bar IS a functioning accessory.
1046310478
- (BOOL)rootTerminalViewShouldLeaveEmptyAreaAtTop {
1046410479
if ([PseudoTerminal windowTypeHasFullSizeContentView:self.windowType]) {
1046510480
DLog(@"YES because window type %@ has full size content view", @(self.windowType));
@@ -10732,7 +10747,23 @@ - (BOOL)tabBarIsVisibleInTitleBarAccessory {
1073210747
return _contentView.tabBarControlOnLoan;
1073310748
}
1073410749

10735-
// See the note in windowDecorationSize about this hack.
10750+
// Returns YES during the transitional state when the tab bar is about to stop
10751+
// being a titlebar accessory.
10752+
//
10753+
// This happens when:
10754+
// 1. tabBarShouldBeAccessory returns NO (tab bar should NOT be an accessory)
10755+
// 2. BUT tabBarControlOnLoan is YES (tab bar IS currently loaned to accessory system)
10756+
//
10757+
// This transitional state occurs when:
10758+
// - Going from 2+ tabs to 1 tab in fullscreen (tab bar will hide)
10759+
// - Exiting fullscreen (tab bar will return to normal position)
10760+
//
10761+
// During this state, we need to compensate in window size calculations because
10762+
// the tab bar is still physically present as an accessory but should not be
10763+
// counted in the window's decoration size.
10764+
//
10765+
// See also: rootTerminalViewShouldLeaveEmptyAreaAtTop (handles layout during transition)
10766+
// See also: windowDecorationSize (uses this to adjust decoration height)
1073610767
- (BOOL)shouldCompensateForDisappearingTabBarAccessory {
1073710768
return (!self.tabBarShouldBeAccessory && _contentView.tabBarControlOnLoan);
1073810769
}

sources/iTerm2SharedARC-Bridging-Header.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
#import "iTermKeyBindingAction.h"
5656
#import "iTermKeystroke.h"
5757
#import "iTermKeystrokeFormatter.h"
58+
#import "iTermLayoutCalculator.h"
5859
#import "iTermLogoGenerator.h"
5960
#import "iTermMenuBarObserver.h"
6061
#import "iTermMetalBufferPool.h"

sources/iTermLayoutCalculator.h

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
//
2+
// iTermLayoutCalculator.h
3+
// iTerm2
4+
//
5+
// Created by George Nachman on 2/25/26.
6+
//
7+
// Pure layout calculation logic extracted from iTermRootTerminalView for testability.
8+
// This class takes layout inputs (dimensions, flags) and returns frame rects
9+
// with no direct AppKit dependencies.
10+
//
11+
12+
#import <Foundation/Foundation.h>
13+
#import <CoreGraphics/CoreGraphics.h>
14+
15+
NS_ASSUME_NONNULL_BEGIN
16+
17+
/// Input parameters for layout calculation.
18+
/// All dimensions are in points.
19+
typedef struct {
20+
/// Size of the window's content view
21+
CGFloat contentViewWidth;
22+
CGFloat contentViewHeight;
23+
24+
/// Tab bar dimensions
25+
CGFloat tabBarHeight;
26+
CGFloat leftTabBarWidth;
27+
28+
/// Toolbelt dimensions
29+
CGFloat toolbeltWidth;
30+
BOOL shouldShowToolbelt;
31+
32+
/// Status bar
33+
CGFloat statusBarHeight;
34+
BOOL hasStatusBar;
35+
BOOL statusBarOnTop; // YES for top, NO for bottom
36+
37+
/// Tab bar state flags
38+
BOOL tabBarVisible;
39+
BOOL tabBarOnLoan;
40+
BOOL tabBarFlashing;
41+
BOOL tabBarShouldBeAccessory;
42+
BOOL tabBarAccessoryOverlapsContent;
43+
44+
/// Fullscreen state
45+
BOOL enteringFullscreen;
46+
BOOL inFullscreen;
47+
48+
/// Tab position (PSMTab_TopTab, PSMTab_BottomTab, PSMTab_LeftTab)
49+
int tabPosition;
50+
51+
/// Division view
52+
BOOL divisionViewVisible;
53+
CGFloat divisionViewHeight;
54+
55+
/// Notch inset (for MacBooks with notch in fullscreen)
56+
CGFloat notchInset;
57+
58+
/// Whether to leave empty area at top for transitional tab bar state
59+
BOOL shouldLeaveEmptyAreaAtTop;
60+
61+
/// Whether to draw window title in place of tab bar
62+
BOOL drawWindowTitleInPlaceOfTabBar;
63+
} iTermLayoutInputs;
64+
65+
/// Output frame rects from layout calculation.
66+
typedef struct {
67+
/// Frame for the tab view (main content area)
68+
CGRect tabViewFrame;
69+
70+
/// Frame for the status bar container
71+
CGRect statusBarFrame;
72+
73+
/// Frame for the toolbelt
74+
CGRect toolbeltFrame;
75+
76+
/// Frame for the tab bar
77+
CGRect tabBarFrame;
78+
79+
/// Decoration heights (space consumed by decorations at top/bottom)
80+
CGFloat decorationHeightTop;
81+
CGFloat decorationHeightBottom;
82+
} iTermLayoutOutputs;
83+
84+
/// Pure layout calculator with no AppKit dependencies.
85+
/// All layout logic is encapsulated here for easy unit testing.
86+
@interface iTermLayoutCalculator : NSObject
87+
88+
/// Calculate layout frames given input parameters.
89+
/// This is a pure function with no side effects.
90+
+ (iTermLayoutOutputs)calculateLayoutWithInputs:(iTermLayoutInputs)inputs;
91+
92+
/// Calculate the tab view frame, optionally shrinking for fullscreen tab bar.
93+
/// This handles the case when the tab bar is a titlebar accessory that overlaps content.
94+
+ (CGRect)tabViewFrameByShrinkingForFullScreenTabBar:(CGRect)frame
95+
withInputs:(iTermLayoutInputs)inputs
96+
NS_SWIFT_NAME(tabViewFrame(byShrinkingForFullScreenTabBar:with:));
97+
98+
/// Calculate the toolbelt frame.
99+
+ (CGRect)toolbeltFrameWithInputs:(iTermLayoutInputs)inputs;
100+
101+
/// Calculate layout for hidden tab bar case.
102+
+ (iTermLayoutOutputs)calculateLayoutWithHiddenTabBarInputs:(iTermLayoutInputs)inputs;
103+
104+
/// Calculate layout for visible top tab bar.
105+
+ (iTermLayoutOutputs)calculateLayoutWithVisibleTopTabBarInputs:(iTermLayoutInputs)inputs;
106+
107+
/// Calculate layout for visible bottom tab bar.
108+
+ (iTermLayoutOutputs)calculateLayoutWithVisibleBottomTabBarInputs:(iTermLayoutInputs)inputs;
109+
110+
/// Calculate layout for visible left tab bar.
111+
+ (iTermLayoutOutputs)calculateLayoutWithVisibleLeftTabBarInputs:(iTermLayoutInputs)inputs;
112+
113+
@end
114+
115+
// Tab position constants (matching PSMTabBarControl)
116+
extern const int kLayoutTabPositionTop;
117+
extern const int kLayoutTabPositionBottom;
118+
extern const int kLayoutTabPositionLeft;
119+
120+
NS_ASSUME_NONNULL_END

0 commit comments

Comments
 (0)