diff --git a/.pi/orksorksorks/alanvardy-var-1135-refine-swipe-buttons/done.md b/.pi/orksorksorks/alanvardy-var-1135-refine-swipe-buttons/done.md new file mode 100644 index 00000000..04c696d2 --- /dev/null +++ b/.pi/orksorksorks/alanvardy-var-1135-refine-swipe-buttons/done.md @@ -0,0 +1,8 @@ +# Done + +- **What was built**: Yellow `moreHintColor` (adaptive: darkened gold in light, `.yellow` in dark) used by both the up-guide text and the `.more` reveal plate; dismiss button renamed to "Dismiss hints" with accessibility label "Dismiss swipe hints" and localized catalog entry; two black hint panels (More above, side hints + Dismiss below) rendered **inside the main card plate**, separated by the reminder content, with the Dismiss pill made white with black text for contrast. Follow-up per user request: hints moved from floating above/below the card to inside the card, so the chrome swipes as one unit with the plate. +- **Commit SHA(s)**: `01426651` (implementation + tests), `9e385667` (test nit fix), `0fb6dbbe` (remove bootstrap DELETEME marker), `bbd64e48` (render hint panels inside the card plate) +- **Verification**: `make format`, `make lint` (0 violations), `scripts/test-one.sh SingleThreadTests/SwipePromptTests` (7 cases), `SingleThreadTests/CardPlateModifierTests` (4 cases), `SingleThreadTests/SwipeToActModifierTests` (14 cases) — all pass. `SwipePromptTests` re-run green after the inside-card refactor. +- **Reviewer findings**: no blockers. Nits: the two panel-layout tests are constants-based tautologies (deferred; matches the repo's headless-assertion convention); tautological colour asserts fixed in `9e385667`; a11y label string remains unverified headlessly (pre-existing pattern, deferred); a11y label is not localized (pre-existing pattern, deferred); light-mode `.more` reveal tint on off-white is ~2.7:1, consistent with existing skip/complete reveals (observation only). +- **Remaining manual items**: Full CI gate (`./scripts/test.sh`) not run in this SMALL pass; run it before merge. Device/simulator visual check of the in-card panels and Dynamic Type is worth a quick look. No PR opened yet. +- **Branch note**: local `feb95c39` (your `1` command) diverged from the remote and swept the step artifacts into a source commit; it was rebased onto `0fb6dbbe` (the remote DELETEME removal) and the artifacts were re-committed separately, so no force-push was needed. diff --git a/.pi/orksorksorks/alanvardy-var-1135-refine-swipe-buttons/small.md b/.pi/orksorksorks/alanvardy-var-1135-refine-swipe-buttons/small.md new file mode 100644 index 00000000..b341ae3b --- /dev/null +++ b/.pi/orksorksorks/alanvardy-var-1135-refine-swipe-buttons/small.md @@ -0,0 +1,52 @@ +# Task + +Refine the swipe hint UI in the SingleThread iOS app (Linear VAR-1135). Three +changes: + +1. **"More" should be yellow** — both the swipe reveal guide (the tinted hint + plate shown while dragging up, in `SwipeToActModifier`/`CardPlate`) and the + textual hint, currently sharing the skip colour. Introduce/use a distinct + yellow "more" hint colour. +2. **Rename the dismiss button** — the swipe-hint prompt's dismiss button text + should read **"Dismiss hints"** (currently "Dismiss"), in + `ReminderCardView`. Keep it accessible (update the accessibility label / + identifier expectations accordingly). +3. **Add black panels behind the hints** — one black panel behind the "More" + hint (above), and one black panel behind all the remaining hints plus the + dismiss button (below). The two panels must have a visible gap so they do + not touch each other. Ensure contrast/accessibility and Dynamic Type still + work. + +The existing swipe-hint prompt is gated by the `showSwipePrompt` preference and +has unit coverage (`SingleThreadTests/SwipePromptTests.swift`, +`SwipeToActModifierTests.swift`, `CardPlateModifierTests.swift`). Update and add +tests for the new colour, the new button label, and the panel layout constants, +following existing patterns. + +# Why SMALL + +Localized UI change across an existing pattern: a handful of files in the iOS +app (`ReminderCardView.swift`, `SwipeToActModifier.swift`, `CardPlate.swift`) +plus their existing tests. No schema/migration, no new subsystem/integration, +no shared/convention risk, and no design trade-off requiring sign-off. + +# Key files (recon) + +- `SingleThread/ReminderCardView.swift` — the dismissible swipe-instruction + prompt; `dismissButton` around line 280 currently renders `Text("Dismiss")` + with accessibility label "Dismiss swipe prompt" and identifier + `swipePromptDismissButton`. +- `SingleThread/SwipeToActModifier.swift` — `SwipeRevealPanel`/hint plates; + `.more` currently uses `CardPlate.skipHintColor` (line ~104) and a bottom + edge icon `ellipsis.circle.fill`. +- `SingleThread/CardPlate.swift` — hint colour helpers + (`completeHintColor`, `skipHintColor`, `refreshHintColor`); add a yellow + "more" colour here. +- `SingleThreadTests/SwipePromptTests.swift` — asserts the prompt description + contains "Dismiss" and the dismiss button accessibility label. +- `SingleThreadTests/SwipeToActModifierTests.swift` and + `SingleThreadTests/CardPlateModifierTests.swift` — existing coverage for the + reveal panel and hint colours; extend for the yellow More tint. +- `SingleThreadCore/Sources/SingleThreadCore/LocalizedString+Shared.swift` — + shared strings (`moreAction`); check whether the new "Dismiss hints" label + should be localized there. diff --git a/SingleThread/CardPlate.swift b/SingleThread/CardPlate.swift index a156cb8c..1ec56977 100644 --- a/SingleThread/CardPlate.swift +++ b/SingleThread/CardPlate.swift @@ -40,6 +40,15 @@ enum CardPlate { colorScheme == .dark ? .green : Color(red: 0.10, green: 0.58, blue: 0.24) } + /// Adaptive tint for the "Swipe up for more" hint (and the revealed more + /// panel). A darkened gold in light mode keeps the caption legible on the + /// off-white plate; dark mode uses semantic `.yellow` — same rationale as + /// `skipHintColor(for:)`. Yellow sets the More action apart from the + /// complete/skip/refresh reveals. + static func moreHintColor(for colorScheme: ColorScheme) -> Color { + colorScheme == .dark ? .yellow : Color(red: 0.72, green: 0.56, blue: 0.05) + } + /// Adaptive tint for the pull-to-refresh panel (darkened blue in light /// mode, semantic `.blue` in dark mode — same rationale as /// `skipHintColor(for:)`). Blue keeps refresh distinct from the diff --git a/SingleThread/ReminderCardView.swift b/SingleThread/ReminderCardView.swift index 0a471af4..29cb4a53 100644 --- a/SingleThread/ReminderCardView.swift +++ b/SingleThread/ReminderCardView.swift @@ -39,21 +39,20 @@ struct ReminderCardView: View { // MARK: Internal + /// Vertical gap (in points) used inside the swipe-prompt chrome — between + /// the side-hint row and the Dismiss button, and as the card's own row + /// spacing. Both hint panels sit inside the card plate and are separated by + /// the reminder content between them, so they never touch. Extracted so the + /// gap can be asserted headlessly. + static let promptPanelSpacing: CGFloat = 12 + + /// Fill behind both swipe-hint panels: solid black so the tinted hints stay + /// legible over any wallpaper in light and dark schemes. + static let promptPanelFill = Color.black + var body: some View { - VStack(spacing: 12) { - if showSwipePrompt { - upGuide - .offset(cardOffset) - } - cardPlate - if showSwipePrompt { - horizontalGuide - .offset(cardOffset) - dismissButton - .offset(cardOffset) - } - } - .frame(maxWidth: maxWidth) + cardPlate + .frame(maxWidth: maxWidth) } // MARK: Private @@ -69,9 +68,9 @@ struct ReminderCardView: View { /// button writes `false` through this binding. @Binding private var showSwipePrompt: Bool - /// The plate's live travel, mirrored onto the prompt chrome so the guides - /// travel with the card without being pulled into the reveal container — - /// which would anchor the revealed plate to the chrome instead of the card. + /// The plate's live travel, owned here so `cardPlate`'s swipe gesture can + /// drive it while the plate (now including the hint panels) stays inside the + /// reveal container and moves as one unit. @State private var swipeOffset: CGSize = .zero private let display: ReminderDisplay @@ -103,12 +102,6 @@ struct ReminderCardView: View { /// Fired when the plate's own swipe passes its release threshold. private let onSwipe: (SwipeGestureOutcome) -> Void - /// Downward drags belong to the pull-to-refresh surface, so the plate only - /// travels up from its own gesture; the pull adds downward travel on top. - private var cardOffset: CGSize { - CGSize(width: swipeOffset.width, height: min(swipeOffset.height, 0) + max(pullOffset, 0)) - } - /// Every link in the card's title and notes, in reading order. private var linkURLs: [URL] { var urls = LinkFormatter.links(in: display.title) @@ -120,13 +113,22 @@ struct ReminderCardView: View { /// The reminder content on its plate — and the swipe surface. Only this part /// is wrapped by the reveal container, so complete / skip / more / refresh - /// all emerge from under the card itself. + /// all emerge from under the card itself. When the prompt is shown the two + /// black hint panels live inside this same plate — the More panel above the + /// content, the side hints + Dismiss panel below it — so the hints sit + /// within the card rather than floating above and below it. private var cardPlate: some View { - VStack(alignment: .leading, spacing: 4) { + VStack(alignment: .leading, spacing: Self.promptPanelSpacing) { + if showSwipePrompt { + upGuidePanel + } content if showNudge { nudgeBanner } + if showSwipePrompt { + sideGuidePanel + } } .frame(maxWidth: maxWidth, alignment: .leading) .cardPlate(fill: CardPlate.plateFill(for: colorScheme), padding: 12, restoresGeometry: true) @@ -239,7 +241,27 @@ struct ReminderCardView: View { .accessibilityIdentifier("skipNudgeBanner") } - /// `↑ Swipe up for more`, centred above the card. Visual-only: the copy is + /// The up-guide badge on its black panel: `↑ Swipe up for more` centred, + /// visual-only, at the top of the card. + private var upGuidePanel: some View { + upGuide + .frame(maxWidth: .infinity) + .cardPlate(fill: Self.promptPanelFill) + } + + /// The bottom black panel carrying the horizontal guide row and the Dismiss + /// button, at the foot of the card. Both side hints share one plate so the + /// two labels read as a single dismissible group. + private var sideGuidePanel: some View { + VStack(spacing: Self.promptPanelSpacing) { + horizontalGuide + dismissButton + } + .frame(maxWidth: .infinity) + .cardPlate(fill: Self.promptPanelFill) + } + + /// `↑ Swipe up for more`, centred. Visual-only: the copy is /// hidden from accessibility (VoiceOver users get the card's swipe actions). private var upGuide: some View { VStack(spacing: 2) { @@ -247,13 +269,13 @@ struct ReminderCardView: View { Text("Swipe up for more") } .font(.caption) - .foregroundStyle(.secondary) + .foregroundStyle(CardPlate.moreHintColor(for: colorScheme)) .accessibilityHidden(true) } - /// `← Swipe left to skip` and `Swipe right to complete →`, one row below the - /// card, arrow on the outer edge of each side and tinted like its revealed - /// panel. Visual-only. + /// `← Swipe left to skip` and `Swipe right to complete →`, one row within + /// the card, arrow on the outer edge of each side and tinted like its + /// revealed panel. Visual-only. private var horizontalGuide: some View { HStack { HStack(spacing: 4) { @@ -275,20 +297,21 @@ struct ReminderCardView: View { } /// Hides the swipe guide: writes `false` through the persisted - /// `showSwipePrompt` binding. Caption-sized, bordered-prominent, black/white - /// adaptive tint so it contrasts on the surrounding backdrop. + /// `showSwipePrompt` binding. Caption-sized, bordered-prominent, white pill + /// so it always contrasts against the black hint panel behind it — a black + /// pill would vanish into that panel in light mode. private var dismissButton: some View { Button { showSwipePrompt = false } label: { - Text("Dismiss") + Text("Dismiss hints") .font(.caption.bold()) - .foregroundStyle(colorScheme == .dark ? Color.black : Color.white) + .foregroundStyle(Color.black) .padding(.horizontal, 24) .padding(.vertical, 8) } .buttonStyle(.borderedProminent) - .tint(colorScheme == .dark ? Color.white : Color.black) + .tint(Color.white) // Caption-sized text alone falls below the 44pt accessibility // hit-region minimum; stretch the button's frame vertically so // the hit area passes the audit. Padding on the label content @@ -296,7 +319,7 @@ struct ReminderCardView: View { // button itself. .padding(.vertical, 8) .contentShape(Rectangle()) - .accessibilityLabel("Dismiss swipe prompt") + .accessibilityLabel("Dismiss swipe hints") .accessibilityIdentifier("swipePromptDismissButton") } diff --git a/SingleThread/Resources/Localizable.xcstrings b/SingleThread/Resources/Localizable.xcstrings index 0e3e61aa..cc82548d 100644 --- a/SingleThread/Resources/Localizable.xcstrings +++ b/SingleThread/Resources/Localizable.xcstrings @@ -649,43 +649,43 @@ } } }, - "Dismiss": { + "Dismiss hints": { "extractionState": "manual", "localizations": { "en": { "stringUnit": { "state": "translated", - "value": "Dismiss" + "value": "Dismiss hints" } }, "zh-Hans": { "stringUnit": { "state": "translated", - "value": "关闭" + "value": "关闭提示" } }, "es": { "stringUnit": { "state": "translated", - "value": "Cerrar" + "value": "Cerrar sugerencias" } }, "ja": { "stringUnit": { "state": "translated", - "value": "閉じる" + "value": "ヒントを閉じる" } }, "de": { "stringUnit": { "state": "translated", - "value": "Schließen" + "value": "Hinweise schließen" } }, "fr": { "stringUnit": { "state": "translated", - "value": "Fermer" + "value": "Fermer les astuces" } } } diff --git a/SingleThread/SwipeToActModifier.swift b/SingleThread/SwipeToActModifier.swift index eeb81342..79c51b63 100644 --- a/SingleThread/SwipeToActModifier.swift +++ b/SingleThread/SwipeToActModifier.swift @@ -101,7 +101,7 @@ struct SwipeRevealPanel: View { Presentation( title: SharedStrings.moreAction, systemImage: Self.systemImage(for: outcome), - tint: CardPlate.skipHintColor(for: colorScheme)) + tint: CardPlate.moreHintColor(for: colorScheme)) case .refresh: Presentation( title: LocalizedStringResource("Refresh", table: "Localizable", bundle: .main), diff --git a/SingleThreadTests/CardPlateModifierTests.swift b/SingleThreadTests/CardPlateModifierTests.swift index 2449970a..30c7e3d5 100644 --- a/SingleThreadTests/CardPlateModifierTests.swift +++ b/SingleThreadTests/CardPlateModifierTests.swift @@ -26,4 +26,21 @@ struct CardPlateModifierTests { let withoutRestore = Text("test").cardPlate(fill: .blue, restoresGeometry: false) #expect(String(describing: withRestore) != String(describing: withoutRestore)) } + + /// The More hint must be its own yellow decision — distinct from the skip + /// hint in both colour schemes so the upcoming reveal reads as "more", not + /// as a second skip action. + @Test + func moreHintColorDiffersFromSkipInBothSchemes() { + #expect(CardPlate.moreHintColor(for: .light) != CardPlate.skipHintColor(for: .light)) + #expect(CardPlate.moreHintColor(for: .dark) != CardPlate.skipHintColor(for: .dark)) + } + + /// The yellow hint adapts per scheme: light mode is the darkened gold that + /// stays legible on the off-white plate, dark mode is semantic `.yellow`. + @Test + func moreHintColorIsDistinctPerScheme() { + #expect(CardPlate.moreHintColor(for: .light) != CardPlate.moreHintColor(for: .dark)) + #expect(CardPlate.moreHintColor(for: .dark) == .yellow) + } } diff --git a/SingleThreadTests/SwipePromptTests.swift b/SingleThreadTests/SwipePromptTests.swift index 703e30f0..83122f59 100644 --- a/SingleThreadTests/SwipePromptTests.swift +++ b/SingleThreadTests/SwipePromptTests.swift @@ -34,6 +34,40 @@ struct SwipePromptTests { #expect(description.contains("Dismiss")) } + /// The renamed dismiss button copy still surfaces as a literal on the + /// button label, and the corrected accessibility label is reflected as the + /// attachment modifier (its string value is covered by the doc comment on + /// `dismissButtonHasAccessibilityLabel`). + @Test + func dismissButtonShowsRenamedLabel() { + let description = String(describing: makeCard(showSwipePrompt: true).body) + #expect(description.contains("Dismiss hints")) + } + + /// The Dismiss button's accessibility label reflects the renamed copy + /// ("Dismiss swipe hints" instead of the old "Dismiss swipe prompt"). + @Test + func dismissButtonAccessibilityLabelUsesHintsWording() { + let description = String(describing: makeCard(showSwipePrompt: true).body) + #expect(!description.contains("Dismiss swipe prompt")) + #expect(description.contains("AccessibilityAttachmentModifier")) + } + + /// The two black hint panels are separated by the reminder card itself, and + /// the interstitial gap is the shared prompt spacing constant — extracted so + /// the panels provably never touch. + @Test + func promptPanelGapIsNonZeroConstant() { + #expect(ReminderCardView.promptPanelSpacing > 0) + } + + /// The hint panels use solid black so the tinted hints read over any + /// wallpaper in both colour schemes. + @Test + func promptPanelsUseBlackFill() { + #expect(ReminderCardView.promptPanelFill == .black) + } + @Test func promptHiddenWhenDisabled() { let description = String(describing: makeCard(showSwipePrompt: false).body) @@ -49,7 +83,7 @@ struct SwipePromptTests { // `String(describing:)` (see ActionButtonTests), but it does reflect the // `AccessibilityAttachmentModifier` on the Dismiss button. The label // value itself is asserted by the Phase 4 UI test via - // `app.buttons["Dismiss swipe prompt"]`. The bordered-prominent style + // `app.buttons["Dismiss swipe hints"]`. The bordered-prominent style // pins the rounded-button treatment. #expect(description.contains("Button<")) #expect(description.contains("BorderedProminentButtonStyle"))