From 2a9dd88ee0408899e5cadedfb9b84fac31da5079 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 13:27:31 +0000 Subject: [PATCH] fix(suiteui): sidebar row toggles keep one column whatever their symbol --- Sources/SuiteUI/SidebarRow.swift | 21 +++++++- .../SuiteSidebarToggleColumnSnapshots.swift | 49 +++++++++++++++++++ .../SidebarRenameGestureTests.swift | 15 +++++- 3 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 Tests/AppSnapshotTests/SuiteSidebarToggleColumnSnapshots.swift diff --git a/Sources/SuiteUI/SidebarRow.swift b/Sources/SuiteUI/SidebarRow.swift index 6711631a..2921bfbc 100644 --- a/Sources/SuiteUI/SidebarRow.swift +++ b/Sources/SuiteUI/SidebarRow.swift @@ -3,7 +3,8 @@ // item. Row height follows the system sidebar size setting, so nothing here // fixes a height. The row owns only presentation state (the rename draft); // toggles and renames go out through closures, and a nil closure turns that -// gesture off. +// gesture off. Each toggle icon lays out both of its symbols and draws one, +// so it is as wide in every state and the toggles form a column (#53). import SwiftUI public struct SidebarItem: Identifiable, Hashable, Sendable { @@ -95,7 +96,7 @@ public struct SidebarRow: View { Spacer(minLength: SuiteSpacing.s) ForEach(item.toggles) { toggle in Toggle(isOn: Binding(get: { toggle.isOn }, set: { _ in onToggle?(toggle.id) })) { - Label(toggle.title, systemImage: toggle.systemImage) + Label { Text(toggle.title) } icon: { SidebarToggleIcon(toggle: toggle) } } .toggleStyle(.button) .buttonStyle(.plain) @@ -146,3 +147,19 @@ public struct SidebarRow: View { if !name.isEmpty, name != item.title { onRename?(name) } } } + +/// A toggle's icon: its on and off symbols stacked, only the current one +/// drawn. The unseen symbol still takes part in layout, so the icon is as +/// wide as the wider of the two whatever the state, and a toggle with the +/// same symbols sits at the same x in every row (#53). No hand-sized frame. +private struct SidebarToggleIcon: View { + let toggle: SidebarToggle + + var body: some View { + ZStack { + Image(systemName: toggle.onSymbol).opacity(toggle.isOn ? 1 : 0) + Image(systemName: toggle.offSymbol).opacity(toggle.isOn ? 0 : 1) + } + .accessibilityHidden(true) + } +} diff --git a/Tests/AppSnapshotTests/SuiteSidebarToggleColumnSnapshots.swift b/Tests/AppSnapshotTests/SuiteSidebarToggleColumnSnapshots.swift new file mode 100644 index 00000000..ebd57114 --- /dev/null +++ b/Tests/AppSnapshotTests/SuiteSidebarToggleColumnSnapshots.swift @@ -0,0 +1,49 @@ +// SuiteSidebarToggleColumnSnapshots.swift: #53's regression shot. Four +// SidebarRows with an eye and a lock toggle in every on/off mix, so the +// narrow and wide symbols (eye / eye.slash, lock / lock.open) sit in rows +// next to each other. The eyes must form one column and the locks another; +// before #53 a row with an open lock drew its eye further left. Light and +// Dark. macOS only. +import SuiteUI +import SwiftUI +import Testing + +@MainActor +@Suite struct SuiteSidebarToggleColumnSnapshots { + @Test func toggleColumn() throws { + for scheme in [ColorScheme.light, .dark] { + try snapshot( + ToggleColumnSample(), name: "suiteui-sidebar-toggles-\(scheme == .dark ? "dark" : "light")", + scheme: scheme) + } + } +} + +private struct ToggleColumnSample: View { + private static let rows: [(title: String, visible: Bool, locked: Bool)] = [ + ("Open Lock", true, false), + ("Locked", true, true), + ("Hidden, Open Lock", false, false), + ("Hidden, Locked", false, true), + ] + + var body: some View { + VStack(alignment: .leading, spacing: SuiteSpacing.s) { + ForEach(Self.rows, id: \.title) { row in + SidebarRow( + SidebarItem( + id: row.title, title: row.title, systemImage: "square", + toggles: [ + SidebarToggle(id: "visible", title: "Visible", onSymbol: "eye", offSymbol: "eye.slash", isOn: row.visible), + SidebarToggle(id: "locked", title: "Locked", onSymbol: "lock", offSymbol: "lock.open", isOn: row.locked), + ]), + onToggle: { _ in }) + } + } + .padding(SuiteSpacing.m) + .frame(width: SuiteSize.sidebarWidth.ideal + SuiteSpacing.xxl) + .background(SuiteColor.surface, in: RoundedRectangle(cornerRadius: SuiteRadius.medium)) + .padding(SuiteSpacing.xxl) + .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading) + } +} diff --git a/Tests/SuiteUITests/SidebarRenameGestureTests.swift b/Tests/SuiteUITests/SidebarRenameGestureTests.swift index f79da88d..699ad06c 100644 --- a/Tests/SuiteUITests/SidebarRenameGestureTests.swift +++ b/Tests/SuiteUITests/SidebarRenameGestureTests.swift @@ -100,7 +100,7 @@ private func callNames(in text: some StringProtocol) -> (modifiers: Set, let modifiers: Set = [ "toggleStyle", "buttonStyle", "labelStyle", "foregroundStyle", "disabled", "help", "accessibilityLabel", "badge", ] - let calls: Set = ["View", "HStack", "Spacer", "ForEach", "Toggle", "Binding", "Label"] + let calls: Set = ["View", "HStack", "Spacer", "ForEach", "Toggle", "Binding", "Label", "Text", "SidebarToggleIcon"] #expect(names.modifiers.subtracting(modifiers).isEmpty, "new modifier in body: can it rename from a toggle? (#54)") #expect(names.calls.subtracting(calls).isEmpty, "new view or call in body: can it rename from a toggle? (#54)") } @@ -129,3 +129,16 @@ private func callNames(in text: some StringProtocol) -> (modifiers: Set, "new modifier on the title: does it grow the rename hit area over the toggles or blank space? (#308)") #expect(names.calls.subtracting(calls).isEmpty, "new view or call in the title: can it rename elsewhere? (#308)") } + +/// #53: a toggle's icon lays out both its symbols, so flipping a toggle can't +/// change its width and shift the toggles before it. +@Test func toggleIconLaysOutBothSymbols() throws { + let source = try sidebarRowSource() + let body = try #require(region(of: source, from: "public var body: some View {", to: "private var title: some View")) + #expect(squeezed(body).contains("icon:{SidebarToggleIcon(toggle:toggle)}")) + #expect(!body.contains("toggle.systemImage"), "an icon drawn from the current symbol alone changes width (#53)") + let icon = try #require(region(of: source, from: "private struct SidebarToggleIcon", to: ".accessibilityHidden")) + let flat = squeezed(icon) + #expect(flat.contains("Image(systemName:toggle.onSymbol)") && flat.contains("Image(systemName:toggle.offSymbol)")) + #expect(!flat.contains(".frame("), "no hand-sized frame: both symbols set the width") +}