diff --git a/docs/plans/synced-skills-add-button-light-mode.md b/docs/plans/synced-skills-add-button-light-mode.md new file mode 100644 index 0000000..6320ba1 --- /dev/null +++ b/docs/plans/synced-skills-add-button-light-mode.md @@ -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. diff --git a/macos/OpenBridge/Application/main.swift b/macos/OpenBridge/Application/main.swift index 1a57915..3a9da68 100644 --- a/macos/OpenBridge/Application/main.swift +++ b/macos/OpenBridge/Application/main.swift @@ -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 diff --git a/macos/OpenBridge/Interface/Settings/Skills/Sections/SyncFolderSection.swift b/macos/OpenBridge/Interface/Settings/Skills/Sections/SyncFolderSection.swift index c132ba5..141f2f9 100644 --- a/macos/OpenBridge/Interface/Settings/Skills/Sections/SyncFolderSection.swift +++ b/macos/OpenBridge/Interface/Settings/Skills/Sections/SyncFolderSection.swift @@ -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 @@ -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 { @@ -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)) } diff --git a/macos/OpenBridgeUITests/Suites/SyncedSkillsSettingsUITests.swift b/macos/OpenBridgeUITests/Suites/SyncedSkillsSettingsUITests.swift index e3c4bfa..4a37757 100644 --- a/macos/OpenBridgeUITests/Suites/SyncedSkillsSettingsUITests.swift +++ b/macos/OpenBridgeUITests/Suites/SyncedSkillsSettingsUITests.swift @@ -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)) @@ -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 @@ -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)) @@ -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 } @@ -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 {