diff --git a/apps/Mac/Components/RecordingCard.swift b/apps/Mac/Components/RecordingCard.swift index ffacbd7..93e1d7d 100644 --- a/apps/Mac/Components/RecordingCard.swift +++ b/apps/Mac/Components/RecordingCard.swift @@ -56,58 +56,44 @@ struct RecordingCard: View { var body: some View { VStack(alignment: .leading, spacing: 12) { - // Album art with canonical star overlay - ZStack(alignment: .topTrailing) { - Group { - if let frontUrl = frontCoverUrl { - AsyncImage(url: URL(string: frontUrl)) { phase in - switch phase { - case .empty: - Rectangle() - .fill(ApproachNoteTheme.surface) - .overlay { ProgressView() } - case .success(let image): - image - .resizable() - .aspectRatio(contentMode: .fill) - case .failure: - Rectangle() - .fill(ApproachNoteTheme.surface) - .overlay { - Image(systemName: "music.note") - .font(.system(size: 40)) - .foregroundColor(ApproachNoteTheme.textSecondary) - } - @unknown default: - EmptyView() - } + // Album art + Group { + if let frontUrl = frontCoverUrl { + AsyncImage(url: URL(string: frontUrl)) { phase in + switch phase { + case .empty: + Rectangle() + .fill(ApproachNoteTheme.surface) + .overlay { ProgressView() } + case .success(let image): + image + .resizable() + .aspectRatio(contentMode: .fill) + case .failure: + Rectangle() + .fill(ApproachNoteTheme.surface) + .overlay { + Image(systemName: "music.note") + .font(.system(size: 40)) + .foregroundColor(ApproachNoteTheme.textSecondary) + } + @unknown default: + EmptyView() } - } else { - Rectangle() - .fill(ApproachNoteTheme.surface) - .overlay { - Image(systemName: "music.note") - .font(.system(size: 40)) - .foregroundColor(ApproachNoteTheme.textSecondary) - } } - } - .frame(width: artworkSize, height: artworkSize) - .clipShape(RoundedRectangle(cornerRadius: 10)) - .shadow(color: .black.opacity(0.15), radius: 6, x: 0, y: 3) - - // Canonical star badge - if recording.isCanonical == true { - Image(systemName: "star.fill") - .foregroundColor(.yellow) - .font(ApproachNoteTheme.caption()) - .padding(6) - .background(Color.black.opacity(0.6)) - .clipShape(Circle()) - .padding(6) + } else { + Rectangle() + .fill(ApproachNoteTheme.surface) + .overlay { + Image(systemName: "music.note") + .font(.system(size: 40)) + .foregroundColor(ApproachNoteTheme.textSecondary) + } } } .frame(width: artworkSize, height: artworkSize) + .clipShape(RoundedRectangle(cornerRadius: 10)) + .shadow(color: .black.opacity(0.15), radius: 6, x: 0, y: 3) // Recording info below artwork — Year → Artist → Album → (Song Title) VStack(alignment: .leading, spacing: 4) { diff --git a/apps/Mac/Views/RecordingDetailView.swift b/apps/Mac/Views/RecordingDetailView.swift index 1e0e06f..06b2e45 100644 --- a/apps/Mac/Views/RecordingDetailView.swift +++ b/apps/Mac/Views/RecordingDetailView.swift @@ -250,10 +250,6 @@ struct RecordingDetailView: View { VStack(alignment: .leading, spacing: 8) { // Recording Name (Year) — matches SongDetailView title pattern HStack(alignment: .firstTextBaseline, spacing: 8) { - if recording.isCanonical == true { - Image(systemName: "star.fill") - .foregroundColor(ApproachNoteTheme.accent) - } if let songTitle = recording.songTitle { ( Text(songTitle) diff --git a/apps/Mac/Views/SongDetailView.swift b/apps/Mac/Views/SongDetailView.swift index d5bf625..bc1c0a5 100644 --- a/apps/Mac/Views/SongDetailView.swift +++ b/apps/Mac/Views/SongDetailView.swift @@ -23,6 +23,9 @@ struct SongDetailView: View { @State private var selectedVocalFilter: VocalFilter = .all @State private var selectedInstrument: InstrumentFamily? = nil @State private var showFilterPopover: Bool = false + // Per-group expansion state. Groups not in the set are collapsed, so all + // decade/artist shelves start collapsed (mirrors iOS). + @State private var expandedGroups: Set = [] @State private var showAddToRepertoire = false @State private var successMessage: String? @State private var errorMessage: String? @@ -170,6 +173,8 @@ struct SongDetailView: View { await viewModel.load(songId: songId) } .onChange(of: sortOrder) { _, _ in + // Collapse all shelves so the regrouped list reads cleanly. + expandedGroups.removeAll() Task { await viewModel.reloadRecordings(songId: songId) } } .onReceive(NotificationCenter.default.publisher(for: .transcriptionCreated)) { notification in @@ -368,7 +373,8 @@ struct SongDetailView: View { ApproachNoteButton( label, style: .secondary, - trailingSystemImage: "arrow.up.right.square" + trailingSystemImage: "arrow.up.right.square", + font: ApproachNoteTheme.callout(weight: .semibold) ) { openURL(url) } @@ -462,8 +468,8 @@ struct SongDetailView: View { VStack(alignment: .leading, spacing: 16) { // Heading HStack(alignment: .firstTextBaseline, spacing: 6) { - Text("MORE RECORDINGS") - .font(ApproachNoteTheme.title2()) + Text("ALL RECORDINGS") + .font(ApproachNoteTheme.title3()) .bold() .foregroundColor(ApproachNoteTheme.textPrimary) @@ -474,12 +480,12 @@ struct SongDetailView: View { Spacer() } - // Filter + Sort row + // Filter + Sort row: Filter on the left, Sort right-justified. HStack(spacing: 10) { Button(action: { showFilterPopover = true }) { HStack(spacing: 6) { Text("Filter") - .font(ApproachNoteTheme.subheadline()) + .font(ApproachNoteTheme.subheadline(weight: .bold)) Image(systemName: "slider.horizontal.3") .font(.caption) } @@ -488,6 +494,10 @@ struct SongDetailView: View { .padding(.vertical, 8) .background(ApproachNoteTheme.surface) .cornerRadius(8) + .overlay( + RoundedRectangle(cornerRadius: 8) + .stroke(ApproachNoteTheme.textSecondary.opacity(0.5), lineWidth: 1) + ) } .buttonStyle(.plain) .popover(isPresented: $showFilterPopover, arrowEdge: .bottom) { @@ -507,19 +517,31 @@ struct SongDetailView: View { } } label: { HStack(spacing: 6) { - Text("Sort: \(sortOrder.displayName)") - .font(ApproachNoteTheme.subheadline()) + ( + Text("Sort:") + .font(ApproachNoteTheme.subheadline(weight: .bold)) + + Text(" \(sortOrder.displayName)") + .font(ApproachNoteTheme.subheadline()) + ) Image(systemName: "chevron.down") .font(.caption) } .foregroundColor(ApproachNoteTheme.textPrimary) - .padding(.horizontal, 12) - .padding(.vertical, 8) - .background(ApproachNoteTheme.surface) - .cornerRadius(8) } .menuStyle(.borderlessButton) + .menuIndicator(.hidden) .fixedSize() + // Decorate the outer Menu, not the label: .borderlessButton + // strips a custom background/overlay applied inside `label:`, + // so the box has to live here to match the Filter button. + .padding(.horizontal, 12) + .padding(.vertical, 8) + .background(ApproachNoteTheme.surface) + .cornerRadius(8) + .overlay( + RoundedRectangle(cornerRadius: 8) + .stroke(ApproachNoteTheme.textSecondary.opacity(0.5), lineWidth: 1) + ) Spacer() } @@ -528,7 +550,7 @@ struct SongDetailView: View { Toggle(isOn: $playableOnly) { VStack(alignment: .leading, spacing: 2) { Text("Playable only?") - .font(ApproachNoteTheme.headline()) + .font(ApproachNoteTheme.callout(weight: .semibold)) .foregroundColor(ApproachNoteTheme.textPrimary) Text("Toggle On to hide versions of this song without a linked recording to listen to.") .font(ApproachNoteTheme.caption()) @@ -541,15 +563,10 @@ struct SongDetailView: View { // Performance Type segmented (always visible) VStack(alignment: .leading, spacing: 8) { Text("Performance Type") - .font(ApproachNoteTheme.headline()) + .font(ApproachNoteTheme.callout(weight: .semibold)) .foregroundColor(ApproachNoteTheme.textPrimary) - Picker("Performance Type", selection: $selectedVocalFilter) { - ForEach(VocalFilter.allCases) { filter in - Text(filter.displayName.uppercased()).tag(filter) - } - } - .pickerStyle(.segmented) + performanceTypePicker } // Recordings list @@ -582,39 +599,14 @@ struct SongDetailView: View { .frame(maxWidth: .infinity) .padding(.vertical, 40) } else { - // Grouped recordings + // Grouped recordings — each shelf is an expandable accordion + // (collapsed by default) mirroring the iOS layout. Zero spacing + // here so each header sits centered between its dividers rather + // than inheriting the section's 16pt gap below each row. let parentSongTitle = song?.title - ForEach(grouped, id: \.groupKey) { group in - let shelfHasAnyDistinctTitle = group.recordings.contains { recording in - recording.displayTitle(comparedTo: parentSongTitle) != nil - } - VStack(alignment: .leading, spacing: 8) { - // Group header - Text("\(group.groupKey) (\(group.recordings.count))") - .font(ApproachNoteTheme.headline()) - .foregroundColor(ApproachNoteTheme.brand) - .padding(.top, 8) - - // Horizontal scroll of recordings in this group - ScrollView(.horizontal, showsIndicators: false) { - HStack(alignment: .top, spacing: 16) { - ForEach(group.recordings) { recording in - RecordingCard( - recording: recording, - parentSongTitle: parentSongTitle, - shelfHasAnyDistinctTitle: shelfHasAnyDistinctTitle, - onVisible: { [weak viewModel] id in - viewModel?.requestHydration(for: id) - } - ) - .contentShape(Rectangle()) - .onTapGesture { - selectedRecordingId = recording.id - } - } - } - .padding(.horizontal, 4) - } + VStack(alignment: .leading, spacing: 0) { + ForEach(grouped, id: \.groupKey) { group in + groupAccordion(group: group, parentSongTitle: parentSongTitle) } } } @@ -630,6 +622,111 @@ struct SongDetailView: View { } } + // MARK: - Performance Type Picker (custom segmented control) + // Brand-outlined pill; the selected segment is filled with the brand color + // and white text, unselected segments are brand-colored on a clear + // background. Mirrors the iOS control so the selection uses the brand + // color rather than the system accent (issue #202). + @ViewBuilder + private var performanceTypePicker: some View { + HStack(spacing: 0) { + ForEach(Array(VocalFilter.allCases.enumerated()), id: \.element.id) { index, filter in + // Flexible spacers between segments distribute the bar width; + // each segment stays sized to its own text. + if index > 0 { + Spacer(minLength: 4) + } + let isSelected = selectedVocalFilter == filter + Button { + selectedVocalFilter = filter + } label: { + Text(filter.displayName.uppercased()) + .font(ApproachNoteTheme.footnote(weight: .semibold)) + .lineLimit(1) + .minimumScaleFactor(0.85) + .foregroundColor(isSelected ? ApproachNoteTheme.textOnAccent : ApproachNoteTheme.brand) + .padding(.horizontal, ApproachNoteTheme.spacingMD) + .padding(.vertical, ApproachNoteTheme.spacingXS) + .background( + Capsule().fill(isSelected ? ApproachNoteTheme.brand : Color.clear) + ) + .contentShape(Capsule()) + } + .buttonStyle(.plain) + } + } + .padding(.horizontal, ApproachNoteTheme.spacingXXS) + .padding(.vertical, ApproachNoteTheme.spacingXXS) + // Cap the width on Mac so the pill doesn't stretch across the whole + // window the way it fills the narrow iOS screen. + .frame(maxWidth: 480, alignment: .leading) + .overlay( + Capsule().stroke(ApproachNoteTheme.brand, lineWidth: 1.5) + ) + .animation(.easeInOut(duration: 0.15), value: selectedVocalFilter) + } + + // MARK: - Group Accordion Shelf + + @ViewBuilder + private func groupAccordion(group: (groupKey: String, recordings: [Recording]), parentSongTitle: String?) -> some View { + let isExpanded = expandedGroups.contains(group.groupKey) + + VStack(alignment: .leading, spacing: 0) { + Divider() + + Button(action: { + withAnimation(.easeInOut(duration: 0.2)) { + if isExpanded { + expandedGroups.remove(group.groupKey) + } else { + expandedGroups.insert(group.groupKey) + } + } + }) { + HStack { + Text("\(group.groupKey) (\(group.recordings.count))") + .font(ApproachNoteTheme.headline()) + .foregroundColor(ApproachNoteTheme.brand) + Spacer() + Image(systemName: isExpanded ? "minus" : "plus") + .font(ApproachNoteTheme.headline()) + .foregroundColor(ApproachNoteTheme.brand) + } + .padding(.vertical, 8) + .contentShape(Rectangle()) + } + .buttonStyle(.plain) + + if isExpanded { + let shelfHasAnyDistinctTitle = group.recordings.contains { recording in + recording.displayTitle(comparedTo: parentSongTitle) != nil + } + ScrollView(.horizontal, showsIndicators: false) { + HStack(alignment: .top, spacing: 16) { + ForEach(group.recordings) { recording in + RecordingCard( + recording: recording, + parentSongTitle: parentSongTitle, + shelfHasAnyDistinctTitle: shelfHasAnyDistinctTitle, + onVisible: { [weak viewModel] id in + viewModel?.requestHydration(for: id) + } + ) + .contentShape(Rectangle()) + .onTapGesture { + selectedRecordingId = recording.id + } + } + } + .padding(.horizontal, 4) + .padding(.vertical, 4) + } + .padding(.bottom, 8) + } + } + } + // MARK: - Filter helpers private func hasService(_ recording: Recording, _ service: StreamingService) -> Bool { diff --git a/apps/Shared/Support/ApproachNoteButton.swift b/apps/Shared/Support/ApproachNoteButton.swift index 22d09e4..c17f165 100644 --- a/apps/Shared/Support/ApproachNoteButton.swift +++ b/apps/Shared/Support/ApproachNoteButton.swift @@ -35,6 +35,9 @@ struct ApproachNoteButton: View { private let leadingSystemImage: String? private let trailingSystemImage: String? private let isLoading: Bool + /// Optional override for the label font. Defaults to `headline()` when nil. + /// Lets less-prominent buttons (e.g. the Learn More links) step down a size. + private let font: Font? private let action: () -> Void @State private var isHovered = false @@ -45,6 +48,7 @@ struct ApproachNoteButton: View { leadingSystemImage: String? = nil, trailingSystemImage: String? = nil, isLoading: Bool = false, + font: Font? = nil, action: @escaping () -> Void ) { self.title = title @@ -52,6 +56,7 @@ struct ApproachNoteButton: View { self.leadingSystemImage = leadingSystemImage self.trailingSystemImage = trailingSystemImage self.isLoading = isLoading + self.font = font self.action = action } @@ -73,7 +78,7 @@ struct ApproachNoteButton: View { } } } - .buttonStyle(ApproachNoteButtonStyle(style: style, isHovered: isHovered)) + .buttonStyle(ApproachNoteButtonStyle(style: style, isHovered: isHovered, font: font)) .disabled(isLoading) #if os(macOS) .onHover { hovering in isHovered = hovering } @@ -86,6 +91,7 @@ struct ApproachNoteButton: View { private struct ApproachNoteButtonStyle: ButtonStyle { let style: ApproachNoteButton.Style let isHovered: Bool + var font: Font? = nil @Environment(\.isEnabled) private var isEnabled @@ -94,10 +100,10 @@ private struct ApproachNoteButtonStyle: ButtonStyle { let appearance = appearance(active: active) configuration.label - .font(ApproachNoteTheme.headline()) + .font(font ?? ApproachNoteTheme.headline()) .foregroundColor(appearance.foreground) .frame(maxWidth: .infinity) - .padding(.vertical, 14) + .padding(.vertical, ApproachNoteTheme.controlVerticalPadding) .padding(.horizontal, ApproachNoteTheme.spacingXL) .background( RoundedRectangle(cornerRadius: 10, style: .continuous) diff --git a/apps/Shared/Support/ApproachNoteTheme.swift b/apps/Shared/Support/ApproachNoteTheme.swift index 1b771f6..196239d 100644 --- a/apps/Shared/Support/ApproachNoteTheme.swift +++ b/apps/Shared/Support/ApproachNoteTheme.swift @@ -376,6 +376,17 @@ extension ApproachNoteTheme { static let spacingXL: CGFloat = 24 } +// Component metrics that intentionally sit *outside* the spacing scale above. +// These are not inter-element spacing — they size the interior of a specific +// control to hit a comfortable tap target, and are deliberately named so the +// off-scale value reads as intentional rather than a stray literal. +extension ApproachNoteTheme { + /// Vertical padding inside the filled/outlined pill control + /// (`ApproachNoteButton`). Tuned for tap-target height, not section rhythm, + /// so it sits between `spacingSM` (12) and `spacingMD` (16). + static let controlVerticalPadding: CGFloat = 14 +} + // MARK: - Semantic Color Tokens // // Colors are organized by *role*, not pigment. Pick the token that matches diff --git a/apps/iOS/Components/RecordingRowView.swift b/apps/iOS/Components/RecordingRowView.swift index 7a192cf..461f921 100644 --- a/apps/iOS/Components/RecordingRowView.swift +++ b/apps/iOS/Components/RecordingRowView.swift @@ -89,17 +89,6 @@ struct RecordingRowView: View { .frame(width: 150, height: 150) .background(ApproachNoteTheme.surface) } - - // Canonical star badge - if recording.isCanonical == true { - Image(systemName: "star.fill") - .foregroundColor(.yellow) - .font(ApproachNoteTheme.caption()) - .padding(6) - .background(Color.black.opacity(0.6)) - .clipShape(Circle()) - .padding(6) - } } .cornerRadius(8) .frame(width: 150) diff --git a/apps/iOS/Support/ToastView.swift b/apps/iOS/Support/ToastView.swift index 503a9cb..b53afcc 100644 --- a/apps/iOS/Support/ToastView.swift +++ b/apps/iOS/Support/ToastView.swift @@ -71,7 +71,7 @@ struct ToastView: View { .buttonStyle(.plain) } .padding(.horizontal, ApproachNoteTheme.spacingMD) - .padding(.vertical, 14) + .padding(.vertical, ApproachNoteTheme.spacingMD) .background( RoundedRectangle(cornerRadius: 12) .fill(.white) diff --git a/apps/iOS/Views/ExternalReferencesPanel.swift b/apps/iOS/Views/ExternalReferencesPanel.swift index 728ea19..0331455 100644 --- a/apps/iOS/Views/ExternalReferencesPanel.swift +++ b/apps/iOS/Views/ExternalReferencesPanel.swift @@ -251,7 +251,8 @@ struct ExternalLinkButton: View { ApproachNoteButton( label, style: .secondary, - trailingSystemImage: "arrow.up.right.square" + trailingSystemImage: "arrow.up.right.square", + font: ApproachNoteTheme.callout(weight: .semibold) ) { if let url = URL(string: url) { openURL(url) diff --git a/apps/iOS/Views/RecordingDetailView.swift b/apps/iOS/Views/RecordingDetailView.swift index cb2079a..1e228fb 100644 --- a/apps/iOS/Views/RecordingDetailView.swift +++ b/apps/iOS/Views/RecordingDetailView.swift @@ -213,11 +213,6 @@ struct RecordingDetailView: View { // Recording Name (Year) — matches SongDetailView title pattern HStack(alignment: .firstTextBaseline, spacing: ApproachNoteTheme.spacingXS) { - if recording.isCanonical == true { - Image(systemName: "star.fill") - .foregroundColor(ApproachNoteTheme.accent) - .font(ApproachNoteTheme.title2()) - } if let songTitle = recording.songTitle { ( Text(songTitle) diff --git a/doc/design/layout.md b/doc/design/layout.md index cc70f36..4ac9673 100644 --- a/doc/design/layout.md +++ b/doc/design/layout.md @@ -136,3 +136,18 @@ extension ApproachNoteTheme { Reference these tokens in views instead of literals — `SongDetailView` is the adopted template. Treat any spacing value outside this set as a smell. Issue #199 tracks rolling adoption through the remaining screens. + +### Component metrics (intentionally off-scale) + +A few values size the *interior of a specific control* rather than the rhythm +between elements, so they live outside the spacing scale by design. They get +named tokens so the off-scale number reads as intentional, not a stray literal: + +```swift +static let controlVerticalPadding: CGFloat = 14 // ApproachNoteButton pill, tap-target height +``` + +`ApproachNoteButton` uses `controlVerticalPadding` (14) — kept off-scale to hold +its tap-target height; snapping to `sm`/`md` would shift the pill's proportions. +`ToastView`, by contrast, was snapped to `spacingMD` (16): its padding is card +breathing room, not a tap target, so it belongs on the scale. (Per #202.)