diff --git a/FuelBoard/ContentView.swift b/FuelBoard/ContentView.swift index 30a6cc9..e0e8e1b 100644 --- a/FuelBoard/ContentView.swift +++ b/FuelBoard/ContentView.swift @@ -31,8 +31,8 @@ struct ContentView: View { @State private var distanceUnit: DistanceUnit = FuelStore.loadDistanceUnit() @State private var priceDisplayStyle: PriceDisplayStyle = ContentView.qaPriceStyleOverride(default: FuelStore.loadPriceDisplayStyle()) @State private var stationTapAction: StationTapAction = FuelStore.loadStationTapAction() - @State private var fuelSavingsSettings: FuelSavingsSettings = FuelStore.loadFuelSavingsSettings() - @State private var sessionFuelSavingsAmount: FuelSavingsFillAmount = FuelStore.loadFuelSavingsSettings().preferredAmount + @State private var fuelSavingsSettings: FuelSavingsSettings = ContentView.qaSavingsSettingsOverride(default: FuelStore.loadFuelSavingsSettings()) + @State private var sessionFuelSavingsAmount: FuelSavingsFillAmount = ContentView.qaSavingsSettingsOverride(default: FuelStore.loadFuelSavingsSettings()).preferredAmount @State private var favourites: [FavouriteEntry] = FuelStore.loadFavourites() @State private var alertsEnabled: Bool = FuelStore.loadAlertsEnabled() @State private var alertsRadius: Double = FuelStore.loadAlertsRadius() @@ -175,6 +175,27 @@ struct ContentView: View { ) } + /// QA hook: `-qaConfigureSavings` configures the Fuel Savings feature for the + /// session (the sheet's Best Value card, the Worth the Drive sort and the + /// Key sheet's Best Value row are ALL gated on `settings.isConfigured`), so + /// they can be screenshotted without walking Settings. It writes through the + /// store exactly as the Settings screen does and returns what it wrote, so + /// the app runs on the same stored values — not a stubbed, parallel one. + static func qaSavingsSettingsOverride(default value: FuelSavingsSettings) -> FuelSavingsSettings { +#if DEBUG + guard ProcessInfo.processInfo.arguments.contains("-qaConfigureSavings") else { return value } + let seeded = FuelSavingsSettings( + enabled: true, + mpg: value.mpg, + preferredAmount: FuelSavingsFillAmount(value: 60, unit: .pounds) + ) + FuelStore.saveFuelSavingsSettings(seeded) + return seeded +#else + return value +#endif + } + private func installLocationUpdateHook() { locationManager.onLocationUpdate = { self.updateAlertMonitor() diff --git a/FuelBoard/FuelSavingsViews.swift b/FuelBoard/FuelSavingsViews.swift index 77bf605..86b4029 100644 --- a/FuelBoard/FuelSavingsViews.swift +++ b/FuelBoard/FuelSavingsViews.swift @@ -77,204 +77,6 @@ struct FuelSavingsAdjustmentView: View { } } -struct FuelSavingsSummaryView: View { - let availability: FuelSavingsAvailability? - let amount: FuelSavingsFillAmount - var baselineStationName: String? = nil - var compact: Bool = false - /// Sizes the "extra return travel" caption in the user's own unit (the - /// stored figure is miles) — used by both the row banner and the detail sheet. - var distanceUnit: DistanceUnit = .miles - var showInfoAction: (() -> Void)? = nil - - private func comparisonLabel(for result: FuelSavingsResult) -> String { - FuelSavingsFormat.comparisonLabel(status: result.status) - } - - var body: some View { - switch availability { - case .available(let result): - if compact { - // FULL-WIDTH banner under the row. Inline inside the middle - // column the capsule truncated ("Approx. £2.75 better off after - // tra…") because - // the column is squeezed by the price block — the verdict is the - // point of this sort mode, so it gets its own line with room. - // Colour carries the verdict: green save, red worse off, grey - // break even, blue baseline. lineLimit + minimumScaleFactor keep - // it to one line, so rows stay even. - VStack(alignment: .leading, spacing: 2) { - // The capsule gets the WHOLE line and the comparison caption - // sits beneath it. Side by side, the caption's layoutPriority - // squeezed the verdict until it truncated ("Approx. £2.75 - // better off after tra…"), and in this sort mode the verdict - // is the point of the row. - capsule(for: result, compact: true) - .accessibilityLabel(accessibilityHeadline(for: result)) - Text(comparisonLabel(for: result)) - .font(.caption2) - .foregroundStyle(.secondary) - .lineLimit(1) - .minimumScaleFactor(0.85) - .accessibilityLabel("Comparison: \(comparisonLabel(for: result))") - if let extraTravel = FuelSavingsFormat.extraTravelLabel( - extraDistanceMiles: result.extraDistanceMiles, - unit: distanceUnit, - confidence: result.journeyConfidence - ) { - Text(extraTravel) - .font(.caption2) - .foregroundStyle(.secondary) - .lineLimit(1) - .minimumScaleFactor(0.85) - .accessibilityLabel(spokenExtraTravel(for: result, fallback: extraTravel)) - } - } - // Aligns the banner with the row's text column (46 pt logo + 12 - // pt spacing) so it reads as part of the row, not the cell. - .padding(.leading, 58) - } else { - // Detail sheet: the SAME verdict capsule and the SAME caption as - // the listing rows (one implementation, so wording and colour - // cannot drift), followed by the breakdown only this view has - // room for. The old shape led with a status word + headline that - // matched neither the listing nor each other. - VStack(alignment: .leading, spacing: 10) { - // Same shape as the listing rows: the capsule takes the line - // and the comparison caption sits directly beneath it, so the - // verdict never competes for width with its own caption. - VStack(alignment: .leading, spacing: 4) { - capsule(for: result, compact: false) - .accessibilityLabel(accessibilityHeadline(for: result)) - Text(comparisonLabel(for: result)) - .font(.subheadline) - .foregroundStyle(.secondary) - .lineLimit(1) - .minimumScaleFactor(0.85) - .accessibilityLabel("Comparison: \(comparisonLabel(for: result))") - } - LabeledContent("Your fill-up", value: amountText) - LabeledContent("Fuel you'll receive", value: String(format: "%.2f litres", result.fillLitres)) - .monospacedDigit() - if amount.unit == .pounds { - Text("You pay the selected budget here. The saving compares the same litres at your closest station; litres differ between stations, and this is not a refund.") - .font(.caption2) - .foregroundStyle(.secondary) - } - if let extraTravel = FuelSavingsFormat.extraTravelLabel( - extraDistanceMiles: result.extraDistanceMiles, - unit: distanceUnit, - confidence: result.journeyConfidence - ) { - Text(extraTravel) - .font(.subheadline) - .foregroundStyle(.secondary) - .accessibilityLabel(spokenExtraTravel(for: result, fallback: extraTravel)) - } - if result.status == .baseline { - Text("Your closest station by journey distance is the baseline — every other saving is measured against it.") - .font(.caption) - .foregroundStyle(.secondary) - } else { - Text(baselineStationName.map { "Compared with \($0)." } ?? "Compared with your closest station.") - .font(.caption) - .foregroundStyle(.secondary) - LabeledContent( - result.grossSavingGBP >= 0 ? "Fuel-price saving" : "Fuel-price difference", - value: result.grossSavingGBP >= 0 - ? FuelSavingsFormat.money(result.grossSavingGBP) - : "Costs \(FuelSavingsFormat.money(abs(result.grossSavingGBP))) more" - ) - LabeledContent("Extra driving cost", value: FuelSavingsFormat.money(result.extraDrivingCostGBP)) - Divider() - LabeledContent { - Text(result.headline()) - .fontWeight(.semibold) - .foregroundStyle(capsuleColor(for: result.status)) - .monospacedDigit() - } label: { - Text("Total") - .fontWeight(.semibold) - } - if result.journeyConfidence.isEstimated { - Text("Estimated from straight-line or incomplete route data — refined once both routes are measured.") - .font(.caption2) - .foregroundStyle(.secondary) - } - Text("Estimate covers fuel cost only. It excludes time, parking, tolls and wear.") - .font(.caption2) - .foregroundStyle(.secondary) - } - if let showInfoAction { - Rectangle() - .fill(Color(.separator)) - .frame(height: 0.5) - .padding(.vertical, 4) - Button(action: showInfoAction) { - HStack(spacing: 8) { - Text("How Best Value works") - Spacer(minLength: 8) - Image(systemName: "chevron.forward") - .font(.footnote.weight(.semibold)) - .foregroundStyle(.tertiary) - } - } - .buttonStyle(.plain) - } - } - } - case .unavailable(let reason): - Text(compact ? "Savings unavailable" : "Best Value unavailable: \(reason)") - .font(compact ? .caption2 : .caption) - .foregroundStyle(.secondary) - case nil: - EmptyView() - } - } - - private var amountText: String { - switch amount.unit { - case .pounds: return String(format: "£%.0f", amount.value) - case .litres: return String(format: "%.0f litres", amount.value) - } - } - - /// VoiceOver form of the extra-travel caption ("Approximately 6.4 miles extra - /// return travel"): the abbreviation is spelled out and the unit is a word. - /// Falls back to the visible string if no spoken form exists. - private func spokenExtraTravel(for result: FuelSavingsResult, fallback: String) -> String { - FuelSavingsFormat.extraTravelSpokenLabel( - extraDistanceMiles: result.extraDistanceMiles, - unit: distanceUnit, - confidence: result.journeyConfidence - ) ?? fallback - } - - /// VoiceOver form of the verdict. `spokenHeadline()` spells "Approx." out for - /// and leaves the plain words ("Break even", "Closest station") alone — - /// one source, so visible and spoken copy cannot drift. - private func accessibilityHeadline(for result: FuelSavingsResult) -> String { - result.spokenHeadline() - } - - /// The colour-coded verdict capsule lives in `FuelSavingsCapsule` — ONE - /// implementation for the listing rows, the detail sheet and this compact - /// summary, so the verdict reads identically everywhere; the sheet just - /// gets the larger font and padding. - private func capsule(for result: FuelSavingsResult, compact: Bool) -> some View { - FuelSavingsCapsule(result: result, compact: compact) - } - - /// Verdict colour lives in `FuelSavingsCapsule.color(for:)`. - private func capsuleColor(for status: FuelSavingsStatus) -> Color { - FuelSavingsCapsule.color(for: status) - } - - private func symbol(for status: FuelSavingsStatus) -> String { - FuelSavingsCapsule.symbol(for: status) - } -} - /// The colour-coded verdict capsule shared by the station listing rows, the /// detail sheet and the compact summary. /// @@ -327,4 +129,4 @@ struct FuelSavingsCapsule: View { case .baseline: return "location.circle.fill" } } -} +} \ No newline at end of file diff --git a/FuelBoard/StationDetailSections.swift b/FuelBoard/StationDetailSections.swift new file mode 100644 index 0000000..812674a --- /dev/null +++ b/FuelBoard/StationDetailSections.swift @@ -0,0 +1,263 @@ +import SwiftUI + +/// The More info sheet's facts row: the selected fuel's price on the left, the +/// distance on the right, separated by a hairline down the middle. +/// +/// Every figure comes from the sheet's existing sources — the price through +/// `FuelStore.priceText` and the distance through +/// `FuelStore.stationDistanceDisplay(...)` — so this row can never disagree with +/// the listing row, the widget or the Live Activity, and both columns keep the +/// shared "Approx." confidence marking. +struct StationFactsRow: View { + let station: FuelStation + let selectedFuel: FuelType + let priceAge: FuelPriceFreshness.Assessment + let distance: StationDistanceDisplay? + let distanceUnit: DistanceUnit + + var body: some View { + HStack(alignment: .top, spacing: 12) { + fuelColumn + StationFactsDivider() + distanceColumn + } + .padding(.vertical, 2) + } + + private var fuelColumn: some View { + VStack(alignment: .leading, spacing: 3) { + Text(selectedFuel.displayName) + .font(.caption) + .foregroundStyle(.secondary) + Text(priceText) + .font(.title2.weight(.bold)) + .monospacedDigit() + StationPriceAgeView(assessment: priceAge, compact: true) + } + .frame(maxWidth: .infinity, alignment: .leading) + .accessibilityElement(children: .combine) + } + + private var distanceColumn: some View { + VStack(alignment: .leading, spacing: 3) { + Text("Distance") + .font(.caption) + .foregroundStyle(.secondary) + Text(distanceText) + .font(.title2.weight(.bold)) + .monospacedDigit() + Text(returnText) + .font(.caption2) + .foregroundStyle(.secondary) + .monospacedDigit() + } + .frame(maxWidth: .infinity, alignment: .leading) + .accessibilityElement(children: .combine) + } + + /// The selected grade's price, exactly as the listing row prints it. A + /// missing grade shows an em dash rather than hiding the column, so the row + /// keeps its two-column shape. + private var priceText: String { + guard let price = station.prices[selectedFuel] else { return "—" } + return FuelStore.priceText(price, style: .stationSign) + } + + /// One-way distance, with its confidence marker, from the shared display. + private var distanceText: String { + distance?.label(unit: distanceUnit) ?? "—" + } + + /// Out-and-back distance, same source and same marker rule. + private var returnText: String { + distance?.returnLabel(unit: distanceUnit) ?? "" + } +} + +/// A one-pixel vertical hairline between the facts row's two columns, drawn in +/// the List's own separator colour so it matches every other line on the sheet. +/// Decorative: VoiceOver reads each column as one phrase instead. +private struct StationFactsDivider: View { + var body: some View { + Rectangle() + .fill(Color(.separator)) + .frame(width: 0.5) + .frame(maxHeight: .infinity) + .accessibilityHidden(true) + } +} + +/// The More info sheet's Best Value card: the verdict, then the figures it is +/// built from. +/// +/// Nothing is recomputed here — every number comes from the shared +/// `FuelSavingsResult`, the verdict capsule is the same `FuelSavingsCapsule` the +/// listing rows use, and the total is the shared `headline()`. Wording and +/// colour therefore cannot drift between the sheet and the list. +struct BestValueDetailCard: View { + let availability: FuelSavingsAvailability + let amount: FuelSavingsFillAmount + let baselineStationName: String? + let distanceUnit: DistanceUnit + + var body: some View { + switch availability { + case .available(let result): + availableBody(result) + case .unavailable(let reason): + Text("Best Value unavailable: \(reason)") + .font(.caption) + .foregroundStyle(.secondary) + } + } + + private func availableBody(_ result: FuelSavingsResult) -> some View { + VStack(alignment: .leading, spacing: 14) { + header(result) + verdict(result) + panels(result) + workings(result) + } + } + + /// Card title on the left, the verdict badge on the right — the same capsule + /// the listing rows carry, so the badge and the large capsule below it can + /// never disagree. + private func header(_ result: FuelSavingsResult) -> some View { + HStack(alignment: .firstTextBaseline, spacing: 10) { + Text("Best Value") + .font(.headline) + Spacer(minLength: 8) + FuelSavingsCapsule(result: result) + } + } + + /// The verdict capsule keeps its own line above the working, as before. + private func verdict(_ result: FuelSavingsResult) -> some View { + VStack(alignment: .leading, spacing: 4) { + FuelSavingsCapsule(result: result, compact: false) + .accessibilityLabel(result.spokenHeadline()) + Text(FuelSavingsFormat.comparisonLabel(status: result.status)) + .font(.subheadline) + .foregroundStyle(.secondary) + } + } + + /// The two figures the estimate is made OF, side by side: what the user + /// spends, and how much fuel that buys at this station. + private func panels(_ result: FuelSavingsResult) -> some View { + HStack(spacing: 10) { + panel(symbol: "fuelpump.fill", + title: "Your fill-up", + value: Self.amountText(amount)) + panel(symbol: "drop.fill", + title: "Fuel you'll receive", + value: String(format: "%.2f litres", result.fillLitres)) + } + } + + private func panel(symbol: String, title: String, value: String) -> some View { + VStack(alignment: .leading, spacing: 6) { + HStack(spacing: 5) { + Image(systemName: symbol) + .font(.caption) + .foregroundStyle(.secondary) + .accessibilityHidden(true) + Text(title) + .font(.caption) + .foregroundStyle(.secondary) + .lineLimit(1) + .minimumScaleFactor(0.8) + } + Text(value) + .font(.title3.weight(.semibold)) + .monospacedDigit() + .lineLimit(1) + .minimumScaleFactor(0.8) + } + .frame(maxWidth: .infinity, alignment: .leading) + .padding(.horizontal, 12) + .padding(.vertical, 10) + .background(RoundedRectangle(cornerRadius: 10).fill(Color.primary.opacity(0.06))) + .accessibilityElement(children: .combine) + } + + /// The working: what the extra travel adds, what the pump saves, and the + /// total. A baseline station has nothing to save against itself, so it stops + /// after the panels — the capsule above already says "Closest station". + @ViewBuilder + private func workings(_ result: FuelSavingsResult) -> some View { + if result.status != .baseline { + Divider() + extraTravelRow(result) + savingRows(result) + Divider() + totalRow(result) + if let baselineStationName { + Text("Compared with \(baselineStationName).") + .font(.caption) + .foregroundStyle(.secondary) + } + } + if result.journeyConfidence.isEstimated { + Text("Estimated from straight-line or incomplete route data — refined once both routes are measured.") + .font(.caption2) + .foregroundStyle(.secondary) + } + } + + /// The extra driving a saving has to beat, named in the row label and + /// measured in the value — same figure and same "Approx." rule as the shared + /// caption, from the shared formatter. + @ViewBuilder + private func extraTravelRow(_ result: FuelSavingsResult) -> some View { + if let extraTravel = FuelSavingsFormat.extraTravelDistanceLabel( + extraDistanceMiles: result.extraDistanceMiles, + unit: distanceUnit, + confidence: result.journeyConfidence + ) { + LabeledContent("Extra return travel", value: extraTravel) + .monospacedDigit() + } + } + + @ViewBuilder + private func savingRows(_ result: FuelSavingsResult) -> some View { + LabeledContent( + result.grossSavingGBP >= 0 ? "Fuel-price saving" : "Fuel-price difference", + value: result.grossSavingGBP >= 0 + ? FuelSavingsFormat.money(result.grossSavingGBP) + : "Costs \(FuelSavingsFormat.money(abs(result.grossSavingGBP))) more" + ) + .monospacedDigit() + LabeledContent("Extra driving cost", value: FuelSavingsFormat.money(result.extraDrivingCostGBP)) + .monospacedDigit() + } + + /// The Total row is the WORKINGS; the capsule above is the quick verdict. + /// Both take their wording and colour from the shared result. + private func totalRow(_ result: FuelSavingsResult) -> some View { + LabeledContent { + Text(result.headline()) + .fontWeight(.semibold) + .foregroundStyle(FuelSavingsCapsule.color(for: result.status)) + .monospacedDigit() + } label: { + Text("Total") + .fontWeight(.semibold) + } + } + + /// "£60" or "60 litres" — the same strings the sheet printed before the + /// redesign, so the fill-up reads identically in both units. + static func amountText(_ amount: FuelSavingsFillAmount) -> String { + switch amount.unit { + case .pounds: return String(format: "£%.0f", amount.value) + case .litres: return String(format: "%.0f litres", amount.value) + } + } +} + +/// "How Best Value works" lives in the shared `HowBestValueRow` +/// (HowBestValueSheet.swift) — the same row the Key sheet uses, so the sheet and +/// the Key cannot drift. diff --git a/FuelBoard/StationDetailView.swift b/FuelBoard/StationDetailView.swift index 901f4b6..7efd2fb 100644 --- a/FuelBoard/StationDetailView.swift +++ b/FuelBoard/StationDetailView.swift @@ -63,12 +63,78 @@ struct StationDetailView: View { FuelPriceFreshness.assessment(for: station) } - /// The selected grade's current price, when present. Kept separate from - /// the price-age row so an old price remains visible rather than being - /// silently hidden. - private var selectedFuelPriceText: String { - guard let price = station.prices[selectedFuel] else { return "—" } - return FuelStore.priceText(price, style: .stationSign) + /// One-way and out-and-back distance from the current location, from the ONE + /// shared display helper — so this sheet's figures carry the same confidence + /// marker as the listing row, the widget and the Live Activity. + private var distanceDisplay: StationDistanceDisplay? { + guard let location else { return nil } + return FuelStore.stationDistanceDisplay( + station: station, userLat: location.lat, userLng: location.lng + ) + } + + /// Header title: the station name over its address, as the mockup shows. + /// Tapping copies the postcode, and the "Postcode copied" capsule floats over + /// the address line it confirms. + /// + /// The CARD'S FIRST ROW, above the map — deliberately not a toolbar + /// principal item: a principal item is clamped to one line, so a long + /// forecourt name ("Brighouse Bradford Rd Superstore - Petrol") showed an + /// ellipsis and `minimumScaleFactor` was not honoured. Inside the card it + /// wraps to two lines, and the confirmation capsule cannot be clipped by the + /// nav bar. + private var headerTitle: some View { + Button { + copyPostcode() + } label: { + VStack(alignment: .leading, spacing: 1) { + Text(station.name) + .font(.title3.weight(.semibold)) + .multilineTextAlignment(.leading) + .lineLimit(2) + Text(addressLine) + .font(.caption) + .foregroundStyle(.secondary) + .multilineTextAlignment(.leading) + .lineLimit(2) + } + .frame(maxWidth: .infinity, alignment: .leading) + } + .buttonStyle(.plain) + .disabled(postcodeForCopying.isEmpty) + .accessibilityLabel(String(localized: "Address: \(addressLine)", + comment: "Accessibility label for the address in the More info sheet header")) + .accessibilityHint(postcodeForCopying.isEmpty + ? "" + : String(localized: "Copies the postcode", + comment: "Accessibility hint for the address in the More info sheet header")) + .overlay(alignment: .center) { copiedConfirmation } + } + + /// The transient "Postcode copied" capsule. An overlay, so nothing moves + /// while it is up, and hit testing stays off so the header keeps its tap. + @ViewBuilder + private var copiedConfirmation: some View { + if showingCopied { + HStack(spacing: 4) { + Image(systemName: "checkmark") + Text("Postcode copied") + } + .font(.caption.weight(.semibold)) + .foregroundStyle(.green) + .padding(.horizontal, 8) + .padding(.vertical, 4) + // A material base keeps it legible where the tint alone (the + // notice-capsule recipe) washes out. + .background { + Capsule() + .fill(.regularMaterial) + .overlay(Capsule().fill(Color.green.opacity(0.22))) + } + .allowsHitTesting(false) + .accessibilityHidden(true) + .transition(.opacity) + } } /// Opens Apple Maps driving directions — the same destination (and the same @@ -90,8 +156,18 @@ struct StationDetailView: View { UINotificationFeedbackGenerator().notificationOccurred(.success) withAnimation(.easeInOut(duration: 0.2)) { showingCopied = true } copyResetTask?.cancel() +#if DEBUG + // QA hook: `-qaCopyPostcode` holds the confirmation up long enough for a + // screenshot. The production window (2s) cannot be hit reliably — the app + // takes several seconds to render, and simctl cannot tap. + let hold: UInt64 = ProcessInfo.processInfo.arguments.contains("-qaCopyPostcode") + ? 25_000_000_000 + : 2_000_000_000 +#else + let hold: UInt64 = 2_000_000_000 +#endif copyResetTask = Task { - try? await Task.sleep(nanoseconds: 2_000_000_000) + try? await Task.sleep(nanoseconds: hold) guard !Task.isCancelled else { return } withAnimation(.easeInOut(duration: 0.3)) { showingCopied = false } } @@ -99,8 +175,18 @@ struct StationDetailView: View { var body: some View { NavigationStack { + // Wrapper exists only so the DEBUG scroll hook below can reach a + // section by id; it has no effect on layout or behaviour. + ScrollViewReader { proxy in List { Section { + // The sheet header is the CARD'S FIRST ROW, above the map: + // the station name over its address, both wrapping rather + // than truncating, on the card's own insets so it lines up + // with the map and the rows below. Tapping it copies the + // postcode. No hairline under it — the map follows. + headerTitle + .listRowSeparator(.hidden, edges: .bottom) // Tapping anywhere on the map opens the same Apple Maps // directions the Directions button below opens. // @@ -140,80 +226,26 @@ struct StationDetailView: View { // clutter. The Address/Distance and Distance/actions lines // stay. .listRowSeparator(.hidden, edges: .bottom) - // The map, the address, the distance, any notices and the two - // station actions all share ONE card (the list's own row - // hairlines separate them), so no hand-rolled Divider here — - // adding one would double the line. + // The map, the facts row, any notices and the two station + // actions all share ONE card (the list's own row hairlines + // separate them), so no hand-rolled Divider here — adding one + // would double the line. // - // The station name is NOT repeated: the sheet header already - // shows it. - // - // The whole row is a copy control: it puts JUST the postcode - // on the clipboard so it can be pasted into another - // navigation app. The copy glyph sits ON the address line, - // and the "Postcode copied" confirmation is a capsule that floats OVER - // THIS ROW — it confirms the address, so it must not hover - // over the map. It is an overlay, so nothing moves while it is - // up, and `allowsHitTesting(false)` keeps the row tappable - // through the capsule. - Button { - copyPostcode() - } label: { - HStack(alignment: .firstTextBaseline, spacing: 8) { - Text("Address") - Spacer(minLength: 8) - Text(addressLine) - .multilineTextAlignment(.trailing) - .foregroundStyle(.secondary) - // Stays the plain copy glyph — the capsule carries - // the confirmation, so swapping this to a checkmark - // would say it twice. - Image(systemName: "doc.on.doc") - .font(.footnote.weight(.semibold)) - .foregroundStyle(.secondary) - } - } - .buttonStyle(.plain) - .disabled(postcodeForCopying.isEmpty) - .accessibilityLabel(String(localized: "Address: \(addressLine)", - comment: "Accessibility label for the address row in the More info sheet")) - .accessibilityHint(postcodeForCopying.isEmpty - ? "" - : String(localized: "Copies the postcode", - comment: "Accessibility hint for the address row in the More info sheet")) - // Horizontally centred over the address line itself. - .overlay(alignment: .center) { - if showingCopied { - HStack(spacing: 4) { - Image(systemName: "checkmark") - Text("Postcode copied") - } - .font(.footnote.weight(.semibold)) - .foregroundStyle(.green) - .padding(.horizontal, 10) - .padding(.vertical, 6) - // A material base keeps it legible where the tint - // alone (the notice-capsule recipe) washes out. - .background { - Capsule() - .fill(.regularMaterial) - .overlay(Capsule().fill(Color.green.opacity(0.22))) - } - .allowsHitTesting(false) - .accessibilityHidden(true) - .transition(.opacity) - } - } - if let location { - let distance = FuelStore.stationDistanceDisplay( - station: station, userLat: location.lat, userLng: location.lng - ) - LabeledContent("Distance", value: distance.labelWithReturn(unit: distanceUnit)) - .monospacedDigit() - } - LabeledContent("\(selectedFuel.displayName) price", value: selectedFuelPriceText) - .monospacedDigit() - StationPriceAgeView(assessment: priceFreshness) + // The station name AND the address live in the sheet header, + // which is also where a tap copies the postcode — see the + // principal toolbar item below. + // The price and the distance share one two-column row: each + // figure comes from the same source it did before (the price + // from FuelStore, the distance from the one shared display + // helper), so the row cannot disagree with the listing row, + // the widget or the Live Activity. + StationFactsRow( + station: station, + selectedFuel: selectedFuel, + priceAge: priceFreshness, + distance: distanceDisplay, + distanceUnit: distanceUnit + ) ForEach(StationNotice.notices(for: station)) { notice in Label(notice.title, systemImage: notice.symbolName) .font(.subheadline.weight(.semibold)) @@ -222,15 +254,19 @@ struct StationDetailView: View { .padding(.vertical, 6) .background(Capsule().fill(Color.orange.opacity(0.14))) } - // The two station actions are the LAST row of the card: half - // the width each, on one line, below Address and Distance. - HStack(spacing: 12) { + // Half the width each, on one line, with the list's hairline + // running between them. + HStack(spacing: 0) { Button { openDirections() } label: { Label("Directions", systemImage: "arrow.triangle.turn.up.right.diamond.fill") .frame(maxWidth: .infinity) } + Rectangle() + .fill(Color(.separator)) + .frame(width: 0.5, height: 22) + .accessibilityHidden(true) Button(role: .destructive) { showingReportIssueConfirmation = true } label: { @@ -250,16 +286,23 @@ struct StationDetailView: View { // is not configured — so the heading cannot be left dangling // over an empty section. if let fuelSavings { + // The card's own header row says "Best Value", so a section + // header would repeat it; the green tint is what marks this + // block as the savings card. Section { - FuelSavingsSummaryView( + BestValueDetailCard( availability: fuelSavings, amount: fuelSavingsAmount, baselineStationName: savingsBaselineStationName, - distanceUnit: distanceUnit, - showInfoAction: { showBestValueInfo = true } + distanceUnit: distanceUnit ) - } header: { - Text("Best Value") + } + .listRowBackground(Color.green.opacity(0.08)) + .id("bestValueCard") + + Section { + // The shared row — same component the Key sheet shows. + HowBestValueRow { showBestValueInfo = true } } } @@ -276,8 +319,19 @@ struct StationDetailView: View { #endif } - .navigationTitle(station.name) - .navigationBarTitleDisplayMode(.inline) + // No navigationTitle: the sheet's header row carries the name and + // address, and an inline title would print the name a second time. +#if DEBUG + // QA hook: `-qaDetailBottom` scrolls to the Best Value card so its + // lower half (Extra driving cost, Total, the estimate caveat) can be + // screenshotted. `simctl` cannot scroll, and `.defaultScrollAnchor` + // is a no-op on a List — only scrollTo moves it. + .task { + guard ProcessInfo.processInfo.arguments.contains("-qaDetailBottom") else { return } + try? await Task.sleep(nanoseconds: 3_000_000_000) + withAnimation { proxy.scrollTo("bestValueCard", anchor: .bottom) } + } +#endif .alert("Report an error?", isPresented: $showingReportIssueConfirmation) { Button("Cancel", role: .cancel) { } Button("Report") { @@ -319,6 +373,9 @@ struct StationDetailView: View { copyPostcode() } #endif + } + // Closes the ScrollViewReader wrapped around the List: kept last so + // every List modifier above stays inside `proxy`'s scope. } } } diff --git a/FuelBoard/StationPriceAgeView.swift b/FuelBoard/StationPriceAgeView.swift index af28646..3a0907e 100644 --- a/FuelBoard/StationPriceAgeView.swift +++ b/FuelBoard/StationPriceAgeView.swift @@ -16,10 +16,25 @@ import SwiftUI struct StationPriceAgeView: View { let assessment: FuelPriceFreshness.Assessment + /// The More info sheet's facts row prints this under the fuel price inside a + /// half-width column, where a `LabeledContent`'s label + value pair cannot + /// fit on one line. Compact keeps the SAME words in one caption line. + var compact: Bool = false + var body: some View { VStack(alignment: .leading, spacing: 2) { - LabeledContent("Price updated", value: assessment.updateAgeLabel) - .monospacedDigit() + if compact { + Text("Price updated \(assessment.updateAgeLabel)") + .font(.caption2) + .foregroundStyle(.secondary) + .monospacedDigit() + .lineLimit(1) + .minimumScaleFactor(0.75) + .accessibilityLabel("Price updated \(assessment.updateAgeLabel)") + } else { + LabeledContent("Price updated", value: assessment.updateAgeLabel) + .monospacedDigit() + } if let notice = assessment.outdatedNotice { Text(notice) .font(.caption2) diff --git a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelBoardTests.swift b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelBoardTests.swift index c7918c8..ac232a7 100644 --- a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelBoardTests.swift +++ b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelBoardTests.swift @@ -787,6 +787,12 @@ final class RoadDistanceCacheTests: XCTestCase { XCTAssertEqual(confirmed.labelWithReturn(unit: .kilometers), "3.2 km (6.8 km return)") XCTAssertEqual(confirmed.spokenLabel(unit: .miles), "2.0 miles") XCTAssertEqual(confirmed.spokenLabelWithReturn(unit: .miles), "2.0 miles, 4.2 miles return") + // The return figure split out on its own — a surface that prints the + // one-way and the return distance on separate lines still gets BOTH from + // this type, marker included. + XCTAssertEqual(confirmed.returnLabel(unit: .miles), "4.2 mi return") + XCTAssertEqual(confirmed.returnLabel(unit: .kilometers), "6.8 km return") + XCTAssertEqual(confirmed.spokenReturnLabel(unit: .miles), "4.2 miles return") let approximate = FuelStore.stationDistanceDisplay(station: unrouted, userLat: 51.6, userLng: -0.1) XCTAssertTrue(approximate.isApproximate, "the straight-line fallback must be identified as one") @@ -828,6 +834,10 @@ final class RoadDistanceCacheTests: XCTestCase { XCTAssertFalse(display.returnIsApproximate) XCTAssertEqual(display.labelWithReturn(unit: .miles), "2.9 mi (5.9 mi return)") XCTAssertEqual(display.spokenLabelWithReturn(unit: .miles), "2.9 miles, 5.9 miles return") + // The split-out return figure must use the INDEPENDENT return route + // (9.47 km), not a doubled one-way (2 × 4.65 = 9.30 km → 5.8 mi). + XCTAssertEqual(display.returnLabel(unit: .miles), "5.9 mi return") + XCTAssertEqual(display.spokenReturnLabel(unit: .miles), "5.9 miles return") } /// The marker tracks the cache on its own: same station, bare figure once its diff --git a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsEstimateTests.swift b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsEstimateTests.swift index c380e8c..cd8be6a 100644 --- a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsEstimateTests.swift +++ b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsEstimateTests.swift @@ -640,6 +640,29 @@ final class FuelSavingsEstimateTests: XCTestCase { ) } + /// The same figure on its own, for the sheet's row form ("Extra return + /// travel | 6.4 mi"): identical value and identical marker rule, so the row + /// and the caption can never disagree about being an estimate. Below the + /// 0.05-mile floor both forms return nil together. + func testExtraTravelDistanceLabelMatchesTheCaption() { + XCTAssertEqual( + FuelSavingsFormat.extraTravelDistanceLabel(extraDistanceMiles: 6.4, unit: .miles), + "6.4 mi" + ) + XCTAssertEqual( + FuelSavingsFormat.extraTravelDistanceLabel(extraDistanceMiles: 6.4, unit: .miles, confidence: .estimated), + "Approx. 6.4 mi" + ) + XCTAssertEqual( + FuelSavingsFormat.extraTravelDistanceLabel(extraDistanceMiles: 6.4, unit: .kilometers), + "10.3 km" + ) + XCTAssertNil( + FuelSavingsFormat.extraTravelDistanceLabel(extraDistanceMiles: 0.04, unit: .miles), + "below the floor there is no extra travel to name" + ) + } + /// VoiceOver gets words, not abbreviations: "Approx." → "Approximately" and /// "mi"/"km" → "miles"/"kilometres". func testSpokenLabelsSpellOutTheAbbreviations() { diff --git a/Shared/FuelSavings.swift b/Shared/FuelSavings.swift index 81ad3f3..2fb89e7 100644 --- a/Shared/FuelSavings.swift +++ b/Shared/FuelSavings.swift @@ -136,11 +136,25 @@ enum FuelSavingsFormat { static func extraTravelLabel(extraDistanceMiles: Double, unit: DistanceUnit, confidence: JourneyConfidence = .confirmed) -> String? { + guard let distanceText = extraTravelDistanceLabel(extraDistanceMiles: extraDistanceMiles, + unit: unit, + confidence: confidence) else { return nil } + return "\(distanceText) extra return travel" + } + + /// The extra-travel FIGURE on its own, with its confidence marker: "4.3 mi" + /// or "Approx. 4.3 mi". + /// + /// A row form ("Extra return travel | 4.3 mi") names the journey in its own + /// label, so this is the same figure and the same "Approx." rule as + /// `extraTravelLabel`, from the same inputs — never rebuilt at a call site, + /// or one surface marks an estimate while another drops the marker. + static func extraTravelDistanceLabel(extraDistanceMiles: Double, + unit: DistanceUnit, + confidence: JourneyConfidence = .confirmed) -> String? { guard extraDistanceMiles > 0.05 else { return nil } let distanceText = distance(extraDistanceMiles, unit: unit) - return confidence == .estimated - ? "Approx. \(distanceText) extra return travel" - : "\(distanceText) extra return travel" + return confidence == .estimated ? "Approx. \(distanceText)" : distanceText } /// Compact comparison copy kept outside the coloured verdict capsule. diff --git a/Shared/FuelStore.swift b/Shared/FuelStore.swift index d556de9..a8ae9bc 100644 --- a/Shared/FuelStore.swift +++ b/Shared/FuelStore.swift @@ -1806,9 +1806,24 @@ func label(unit: DistanceUnit) -> String { /// Row/sheet label that keeps the one-way station distance primary, then /// adds the independently measured or estimated out-and-back distance. func labelWithReturn(unit: DistanceUnit) -> String { - let returnText = unit.format(returnKM) - let returnLabel = returnIsApproximate ? "Approx. \(returnText)" : returnText - return "\(label(unit: unit)) (\(returnLabel) return)" + "\(label(unit: unit)) (\(returnLabel(unit: unit)))" +} + +/// The out-and-back figure on its own: "5.0 mi return" / "Approx. 5.0 mi return". +/// +/// Split out of `labelWithReturn` for surfaces that print the one-way and the +/// return figure on SEPARATE lines (the More info sheet's distance column). Both +/// figures still come from this one type, so a routed value can never be marked +/// "Approx." while its return figure is not — or the other way round. +func returnLabel(unit: DistanceUnit) -> String { + let text = unit.format(returnKM) + return returnIsApproximate ? "Approx. \(text) return" : "\(text) return" +} + +/// VoiceOver form of `returnLabel(unit:)`. +func spokenReturnLabel(unit: DistanceUnit) -> String { + let text = unit.spoken(returnKM) + return returnIsApproximate ? "Approximately \(text) return" : "\(text) return" } /// VoiceOver form of `label(unit:)`.