Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 23 additions & 0 deletions docs/plans/synced-skills-add-button-light-mode.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Synced Skills Suggested Add Button Light Mode Fix

## Goal
Make the Settings > Synced Skills > Suggested row Add button match the About settings page's standard bordered text-button treatment in light mode, instead of rendering as a dark/black prominent fill.

## Current state
`SuggestedSyncFolderRow` uses a small `.borderedProminent` button with a custom tint derived from `SettingsManager`. When the app accent preference is the default value, this can render as a black filled control in light mode. Existing UI coverage only asserts the sampled button is not near-black, which does not lock in the desired About-style bordered control.

## Proposed changes
- Remove the custom Suggested Add button tint helper and the `.borderedProminent` style.
- Render Suggested Add as `.buttonStyle(.bordered)` with `.controlSize(.small)`, matching About settings text buttons while preserving the label, click behavior, and accessibility identifier.
- Update the Synced Skills UI test to assert the button remains visually non-prominent in light mode by checking the screenshot does not have a dark/opaque filled background, then verify the add action still removes the suggested row.

## If we do not change it
The button can continue to look like a black filled control in light mode, inconsistent with other Settings text buttons and visually heavier than intended.

## Expected result after the change
The Add button appears as a small bordered text button in Suggested rows, consistent with About settings, while adding suggested sync folders still works.

## Acceptance criteria
- Suggested Add button uses bordered, not prominent, styling in code.
- UI test covers light-mode visual regression and add behavior.
- Targeted macOS validation confirms Settings > Synced Skills > Suggested shows the corrected button and clicking Add works.
4 changes: 4 additions & 0 deletions macos/OpenBridge/Application/main.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ private var isPreviewMode: Bool {
UserDefaults.standard.set(SystemAccentColor.default.rawValue, forKey: SettingsKeyName.accentColorName.key)
}

if arguments.contains("-e2eLightAppearance") {
UserDefaults.standard.set(Appearance.light.rawValue, forKey: SettingsKeyName.appearance.key)
}

SettingsManager.shared.enabledFeatures = SettingsManager.Defaults.enabledFeatures
}
#endif
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,8 +293,6 @@ private extension SyncFolderSection {
}

private struct SuggestedSyncFolderRow: View {
@Environment(SettingsManager.self) private var settings

let source: SkillDirectories.SyncSkillSource
let url: URL
let onAdd: () -> Void
Expand All @@ -304,10 +302,6 @@ private struct SuggestedSyncFolderRow: View {
return url.path.hasPrefix(home) ? "~" + url.path.dropFirst(home.count) : url.path
}

private var addButtonTint: Color {
settings.accentColorName == .default ? settings.systemAccentColor : settings.accentColor
}

var body: some View {
HStack(spacing: 8) {
Label {
Expand Down Expand Up @@ -346,8 +340,7 @@ private struct SuggestedSyncFolderRow: View {
Button(String(localized: "Add")) {
onAdd()
}
.tint(addButtonTint)
.buttonStyle(.borderedProminent)
.buttonStyle(.bordered)
.controlSize(.small)
.accessibilityIdentifier(AccessibilityID.Settings.syncedSkillsSuggestedAddButton(source.key))
}
Expand Down
65 changes: 36 additions & 29 deletions macos/OpenBridgeUITests/Suites/SyncedSkillsSettingsUITests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ final class SyncedSkillsSettingsUITests: XCTestCase {
try createSuggestedSkillFolders(in: e2eHomeDirectory)

app = XCUIApplication()
app.launchArguments = ["-e2eMode", "-e2eOpenSettings", "-e2eResetAccentColor"]
app.launchArguments = ["-e2eMode", "-e2eOpenSettings", "-e2eResetAccentColor", "-e2eLightAppearance"]
app.launchEnvironment["OPENBRIDGE_E2E_HOME_DIRECTORY"] = e2eHomeDirectory.path
app.launch()
XCTAssertTrue(app.wait(for: .runningForeground, timeout: 8))
Expand All @@ -26,7 +26,7 @@ final class SyncedSkillsSettingsUITests: XCTestCase {
}
}

func testSuggestedAddButtonUsesVisibleAccentTint() throws {
func testSuggestedAddButtonUsesAboutStyleBorderedButtonInLightMode() throws {
openSyncedSkillsSettings()

let addButton = app.buttons["settings.syncedSkills.suggested.add.claude"].firstMatch
Expand All @@ -38,9 +38,17 @@ final class SyncedSkillsSettingsUITests: XCTestCase {
attachment.lifetime = .keepAlways
add(attachment)

let color = try averageOpaqueColor(in: screenshot.pngRepresentation)
XCTAssertGreaterThan(color.maxComponent, 60, "Suggested Add button should not render as a near-black block.")
XCTAssertGreaterThan(color.luminance, 40, "Suggested Add button should keep enough visible fill contrast.")
let colorStats = try buttonColorStats(in: screenshot.pngRepresentation)
XCTAssertLessThan(
colorStats.nonLightPixelCoverage,
0.45,
"Suggested Add button should render as a bordered text button, not a filled prominent block."
)
XCTAssertGreaterThan(
colorStats.averageCompositedLuminance,
170,
"Suggested Add button should keep a light bordered appearance in light mode."
)

addButton.click()
XCTAssertFalse(addButton.waitForExistence(timeout: 3))
Expand Down Expand Up @@ -69,7 +77,7 @@ final class SyncedSkillsSettingsUITests: XCTestCase {
tabByTitle.click()
}

private func averageOpaqueColor(in pngData: Data) throws -> AverageColor {
private func buttonColorStats(in pngData: Data) throws -> ButtonColorStats {
guard let image = NSImage(data: pngData) else {
throw ColorSamplingError.invalidImage
}
Expand Down Expand Up @@ -99,39 +107,38 @@ final class SyncedSkillsSettingsUITests: XCTestCase {

context.draw(cgImage, in: CGRect(x: 0, y: 0, width: width, height: height))

var red = 0.0
var green = 0.0
var blue = 0.0
var count = 0.0
var luminanceTotal = 0.0
var nonLightPixelCount = 0.0
var totalPixelCount = 0.0

for offset in stride(from: 0, to: pixels.count, by: bytesPerPixel) {
guard pixels[offset + 3] > 32 else { continue }
red += Double(pixels[offset])
green += Double(pixels[offset + 1])
blue += Double(pixels[offset + 2])
count += 1
let alpha = Double(pixels[offset + 3]) / 255.0
let red = Double(pixels[offset]) * alpha + 255.0 * (1.0 - alpha)
let green = Double(pixels[offset + 1]) * alpha + 255.0 * (1.0 - alpha)
let blue = Double(pixels[offset + 2]) * alpha + 255.0 * (1.0 - alpha)
let luminance = 0.2126 * red + 0.7152 * green + 0.0722 * blue

luminanceTotal += luminance
if luminance < 210 {
nonLightPixelCount += 1
}
totalPixelCount += 1
}

guard count > 0 else {
guard totalPixelCount > 0 else {
throw ColorSamplingError.noOpaquePixels
}

return AverageColor(red: red / count, green: green / count, blue: blue / count)
return ButtonColorStats(
averageCompositedLuminance: luminanceTotal / totalPixelCount,
nonLightPixelCoverage: nonLightPixelCount / totalPixelCount
)
}
}

private struct AverageColor {
let red: Double
let green: Double
let blue: Double

var maxComponent: Double {
max(red, green, blue)
}

var luminance: Double {
0.2126 * red + 0.7152 * green + 0.0722 * blue
}
private struct ButtonColorStats {
let averageCompositedLuminance: Double
let nonLightPixelCoverage: Double
}

private enum ColorSamplingError: Error {
Expand Down
Loading