Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Sources/SessionPersistence.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ enum SessionSnapshotSchema {
enum SessionPersistencePolicy {
static let sidebarMinimumWidthKey = "sidebarMinimumWidth"
static let defaultSidebarWidth: Double = 220
static let defaultMinimumSidebarWidth: Double = 190
static let minimumSidebarWidth: Double = 190
static let defaultMinimumSidebarWidth: Double = 180
static let minimumSidebarWidth: Double = 180
static let sidebarMinimumWidthRange: ClosedRange<Double> = 120...260
static let maximumSidebarWidth: Double = 600
static let minimumWindowWidth: Double = 300
Expand Down
17 changes: 11 additions & 6 deletions Sources/Update/UpdateTitlebarAccessory.swift
Original file line number Diff line number Diff line change
Expand Up @@ -468,15 +468,19 @@ enum TitlebarControlsLayoutMetrics {
return (buttonCount * config.buttonSize) + (gapCount * config.spacing)
}

static func visibleContentWidth(config: TitlebarControlsStyleConfig) -> CGFloat {
outerLeadingPadding
+ config.groupPadding.leading
+ buttonRowWidth(config: config)
+ config.groupPadding.trailing
}

static func contentSize(
config: TitlebarControlsStyleConfig,
titlebarShortcutHintXOffset: Double = ShortcutHintDebugSettings.defaultTitlebarHintX
) -> NSSize {
NSSize(
width: outerLeadingPadding
+ config.groupPadding.leading
+ buttonRowWidth(config: config)
+ config.groupPadding.trailing
width: visibleContentWidth(config: config)
+ hintTrailingInset(titlebarShortcutHintXOffset: titlebarShortcutHintXOffset),
height: max(
WindowChromeMetrics.appTitlebarHeight,
Expand Down Expand Up @@ -514,7 +518,7 @@ enum TitlebarControlsLayoutMetrics {
static func minimumSidebarWidth(config: TitlebarControlsStyleConfig) -> CGFloat {
trafficLightClusterWidth
+ trafficLightGap
+ contentSize(config: config).width
+ visibleContentWidth(config: config)
+ sidebarTrailingPadding
}

Expand Down Expand Up @@ -2010,6 +2014,7 @@ final class TitlebarControlsAccessoryViewController: NSTitlebarAccessoryViewCont
let styleRawValue = UserDefaults.standard.integer(forKey: "titlebarControlsStyle")
let style = TitlebarControlsStyle(rawValue: styleRawValue) ?? .classic
let contentSize = TitlebarControlsLayoutMetrics.contentSize(config: style.config)
let visibleContentWidth = TitlebarControlsLayoutMetrics.visibleContentWidth(config: style.config)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 visibleContentWidth is computed twice per updateSize() call — once inside contentSize(config:) (which calls visibleContentWidth internally) and once again as a separate local. Since this is not a hot path the impact is negligible, but the redundancy is easy to eliminate by computing visibleContentWidth first and threading it into contentSize.

Suggested change
let contentSize = TitlebarControlsLayoutMetrics.contentSize(config: style.config)
let visibleContentWidth = TitlebarControlsLayoutMetrics.visibleContentWidth(config: style.config)
let visibleContentWidth = TitlebarControlsLayoutMetrics.visibleContentWidth(config: style.config)
let contentSize = TitlebarControlsLayoutMetrics.contentSize(config: style.config)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

if intrinsicSizeNeedsRefresh {
hostingView.invalidateIntrinsicContentSize()
intrinsicSizeNeedsRefresh = false
Expand Down Expand Up @@ -2038,7 +2043,7 @@ final class TitlebarControlsAccessoryViewController: NSTitlebarAccessoryViewCont
let accessoryOriginXInWindow = view.convert(view.bounds, to: nil).minX
let sidebarTrailingEdgeInAccessory = max(0, sidebarTrailingEdge - accessoryOriginXInWindow)
let xOffset = TitlebarControlsLayoutMetrics.leadingOffset(
contentWidth: contentSize.width,
contentWidth: visibleContentWidth,
trafficLightFrame: trafficLightFrame,
debugSnapshot: debugSnapshot,
sidebarTrailingEdge: sidebarTrailingEdgeInAccessory
Expand Down
6 changes: 3 additions & 3 deletions cmuxTests/SidebarWidthPolicyTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,19 @@ final class SidebarWidthPolicyTests: XCTestCase {

XCTAssertEqual(
SessionPersistencePolicy.defaultMinimumSidebarWidth,
190,
180,
accuracy: 0.001
)
XCTAssertEqual(
SessionPersistencePolicy.resolvedMinimumSidebarWidth(defaults: defaults),
190,
180,
accuracy: 0.001
)
}

func testContentViewClampKeepsMinimumSidebarWidth() {
XCTAssertEqual(
ContentView.clampedSidebarWidth(184, maximumWidth: 600),
ContentView.clampedSidebarWidth(174, maximumWidth: 600),
CGFloat(SessionPersistencePolicy.minimumSidebarWidth),
accuracy: 0.001
)
Expand Down
Loading