Skip to content

fix: keep selection highlight uniform across colored and uncolored workspaces (#3308) #3310

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
austinywang merged 2 commits into from
Apr 30, 2026
Merged

fix: keep selection highlight uniform across colored and uncolored workspaces (#3308) #3310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4cb20a8
Expose regression for uniform sidebar selection color
austinywang Apr 29, 2026
64e098d
Keep colored workspace selection highlights uniform
austinywang Apr 29, 2026
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
69 changes: 0 additions & 69 deletions Sources/Sidebar/SidebarAppearanceSupport.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,66 +70,6 @@ func cmuxAccentColor() -> Color {
Color(nsColor: cmuxAccentNSColor())
}

private func sidebarSelectedWorkspaceRelativeLuminance(_ color: NSColor) -> CGFloat {
guard let rgbColor = color.usingColorSpace(.sRGB) else { return 0 }
var red: CGFloat = 0
var green: CGFloat = 0
var blue: CGFloat = 0
var alpha: CGFloat = 0
rgbColor.getRed(&red, green: &green, blue: &blue, alpha: &alpha)

func linearized(_ component: CGFloat) -> CGFloat {
if component <= 0.03928 {
return component / 12.92
}
return pow((component + 0.055) / 1.055, 2.4)
}

let linearRed = linearized(red)
let linearGreen = linearized(green)
let linearBlue = linearized(blue)
return (0.2126 * linearRed) + (0.7152 * linearGreen) + (0.0722 * linearBlue)
}

private func sidebarSelectedWorkspaceContrastRatio(
between first: NSColor,
and second: NSColor
) -> CGFloat {
let firstLuminance = sidebarSelectedWorkspaceRelativeLuminance(first)
let secondLuminance = sidebarSelectedWorkspaceRelativeLuminance(second)
let lighter = max(firstLuminance, secondLuminance)
let darker = min(firstLuminance, secondLuminance)
return (lighter + 0.05) / (darker + 0.05)
}

private func sidebarSelectedWorkspaceReadableBackgroundNSColor(_ color: NSColor) -> NSColor {
let minimumContrast: CGFloat = 4.5
var adjusted = color.usingColorSpace(.sRGB) ?? color
var iteration = 0

while sidebarSelectedWorkspaceContrastRatio(between: adjusted, and: NSColor.white) < minimumContrast,
iteration < 12 {
guard let darkened = adjusted.blended(withFraction: 0.12, of: .black) else { break }
adjusted = darkened.usingColorSpace(.sRGB) ?? darkened
iteration += 1
}

return adjusted
}

private func sidebarSelectedWorkspaceCustomBackgroundNSColor(
hex: String,
colorScheme: ColorScheme
) -> NSColor? {
guard let color = WorkspaceTabColorSettings.displayNSColor(
hex: hex,
colorScheme: colorScheme
) else {
return nil
}
return sidebarSelectedWorkspaceReadableBackgroundNSColor(color)
}

struct SidebarRemoteErrorCopyEntry: Equatable {
let workspaceTitle: String
let target: String
Expand Down Expand Up @@ -169,16 +109,8 @@ enum SidebarRemoteErrorCopySupport {

func sidebarSelectedWorkspaceBackgroundNSColor(
for colorScheme: ColorScheme,
customHex: String? = nil,
sidebarSelectionColorHex: String? = UserDefaults.standard.string(forKey: "sidebarSelectionColorHex")
) -> NSColor {
if let customHex,
let customColor = sidebarSelectedWorkspaceCustomBackgroundNSColor(
hex: customHex,
colorScheme: colorScheme
) {
return customColor
}
if let hex = sidebarSelectionColorHex,
let parsed = NSColor(hex: hex) {
return parsed
Expand Down Expand Up @@ -224,7 +156,6 @@ func sidebarWorkspaceRowBackgroundStyle(
) -> SidebarWorkspaceRowBackgroundStyle {
let selectedBackground = sidebarSelectedWorkspaceBackgroundNSColor(
for: colorScheme,
customHex: customColorHex,
sidebarSelectionColorHex: sidebarSelectionColorHex
)
let accentBackground = cmuxAccentNSColor(for: colorScheme)
Expand Down
131 changes: 131 additions & 0 deletions cmuxTests/SidebarWidthPolicyTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import AppKit
import SwiftUI
import XCTest

#if canImport(cmux_DEV)
Expand Down Expand Up @@ -61,3 +63,132 @@ final class SidebarWidthPolicyTests: XCTestCase {
XCTAssertFalse(range.contains(686.1))
}
}

final class SidebarWorkspaceSelectionColorTests: XCTestCase {

@greptile-apps greptile-apps Bot Apr 29, 2026

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 New test class placed in the wrong file

SidebarWorkspaceSelectionColorTests is added to SidebarWidthPolicyTests.swift, a file that tests resize-range arithmetic and clamping logic. The existing parallel coverage lives in WorkspaceUnitTests.swift alongside SidebarSelectedWorkspaceColorTests. Moving the new class there (or into a new SidebarSelectionColorTests.swift) would keep test organisation consistent and make it easier to find all selection-colour tests in one place.

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!

func testSelectedColoredWorkspaceUsesStandardSelectionBackgroundInLightAndDark() {
for colorScheme in [ColorScheme.light, .dark] {
let coloredSelected = sidebarWorkspaceRowBackgroundStyle(
activeTabIndicatorStyle: .solidFill,
isActive: true,
isMultiSelected: false,
customColorHex: "#E85D75",
colorScheme: colorScheme,
sidebarSelectionColorHex: nil
)
let standardSelected = sidebarWorkspaceRowBackgroundStyle(
activeTabIndicatorStyle: .solidFill,
isActive: true,
isMultiSelected: false,
customColorHex: nil,
colorScheme: colorScheme,
sidebarSelectionColorHex: nil
)

XCTAssertEqual(coloredSelected.opacity, standardSelected.opacity, accuracy: 0.001)
XCTAssertEqual(coloredSelected.opacity, 1, accuracy: 0.001)
assertColor(coloredSelected.color, equals: standardSelected.color)

let unselectedColored = sidebarWorkspaceRowBackgroundStyle(
activeTabIndicatorStyle: .solidFill,
isActive: false,
isMultiSelected: false,
customColorHex: "#E85D75",
colorScheme: colorScheme,
sidebarSelectionColorHex: nil
)
XCTAssertEqual(unselectedColored.opacity, 0.7, accuracy: 0.001)
XCTAssertFalse(
colorsAreEqual(coloredSelected.color, unselectedColored.color),
"Selected row should use the standard selection background, not the workspace tab color"
)
}
}

func testSelectedColoredWorkspaceUsesConfiguredSelectionBackground() {
let selectionHex = "#123456"
let coloredSelected = sidebarWorkspaceRowBackgroundStyle(
activeTabIndicatorStyle: .solidFill,
isActive: true,
isMultiSelected: false,
customColorHex: "#E85D75",
colorScheme: .light,
sidebarSelectionColorHex: selectionHex
)
let standardSelected = sidebarWorkspaceRowBackgroundStyle(
activeTabIndicatorStyle: .solidFill,
isActive: true,
isMultiSelected: false,
customColorHex: nil,
colorScheme: .light,
sidebarSelectionColorHex: selectionHex
)

XCTAssertEqual(coloredSelected.opacity, 1, accuracy: 0.001)
assertColor(coloredSelected.color, equals: standardSelected.color)
assertColor(coloredSelected.color, equals: NSColor(hex: selectionHex))
}
Comment on lines +107 to +129

@greptile-apps greptile-apps Bot Apr 29, 2026

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 Configured-selection test is light-only

testSelectedColoredWorkspaceUsesConfiguredSelectionBackground passes colorScheme: .light exclusively, whereas the test above it iterates over both schemes in a loop. Because sidebarSelectedWorkspaceBackgroundNSColor uses cmuxAccentNSColor(for:) — which returns different sRGB values in light vs. dark — a sidebarSelectionColorHex-override test in dark mode would exercise the same code path but confirm there's no colour-scheme-specific conditional that could accidentally re-introduce the custom-colour branch for dark mode. Worth adding .dark coverage here to match the companion test.


private func assertColor(
_ actual: NSColor?,
equals expected: NSColor?,
file: StaticString = #filePath,
line: UInt = #line
) {
guard let actual, let expected else {
XCTAssertNotNil(actual, file: file, line: line)
XCTAssertNotNil(expected, file: file, line: line)
return
}

XCTAssertTrue(
colorsAreEqual(actual, expected),
"Expected \(colorDescription(actual)) to equal \(colorDescription(expected))",
file: file,
line: line
)
}

private func colorsAreEqual(_ lhs: NSColor?, _ rhs: NSColor?) -> Bool {
guard let lhs, let rhs else {
return lhs == nil && rhs == nil
}
guard let lhsRGB = lhs.usingColorSpace(.sRGB),
let rhsRGB = rhs.usingColorSpace(.sRGB) else {
return false
}

var lhsRed: CGFloat = 0
var lhsGreen: CGFloat = 0
var lhsBlue: CGFloat = 0
var lhsAlpha: CGFloat = 0
var rhsRed: CGFloat = 0
var rhsGreen: CGFloat = 0
var rhsBlue: CGFloat = 0
var rhsAlpha: CGFloat = 0
lhsRGB.getRed(&lhsRed, green: &lhsGreen, blue: &lhsBlue, alpha: &lhsAlpha)
rhsRGB.getRed(&rhsRed, green: &rhsGreen, blue: &rhsBlue, alpha: &rhsAlpha)

return abs(lhsRed - rhsRed) <= 0.001 &&
abs(lhsGreen - rhsGreen) <= 0.001 &&
abs(lhsBlue - rhsBlue) <= 0.001 &&
abs(lhsAlpha - rhsAlpha) <= 0.001
}

private func colorDescription(_ color: NSColor) -> String {
guard let rgb = color.usingColorSpace(.sRGB) else {
return color.description
}
var red: CGFloat = 0
var green: CGFloat = 0
var blue: CGFloat = 0
var alpha: CGFloat = 0
rgb.getRed(&red, green: &green, blue: &blue, alpha: &alpha)
return String(
format: "rgba(%.3f, %.3f, %.3f, %.3f)",
red,
green,
blue,
alpha
)
}
}