diff --git a/FuelBoard/AlertsView.swift b/FuelBoard/AlertsView.swift index 150b713..4307e0d 100644 --- a/FuelBoard/AlertsView.swift +++ b/FuelBoard/AlertsView.swift @@ -13,7 +13,10 @@ struct AlertsView: View { @Binding var radius: Double // stored in km (monitor + storage) @Binding var fuel: FuelType // the fuel alerts monitor for @Binding var alertMode: AlertMode - @Binding var bestValueThresholdGBP: Double + /// Read-only here: the threshold is edited in Settings → Fuel Savings, where + /// the fill size and MPG that decide the saving live. Read back so the + /// alert's own rule is visible without leaving the tab. + let bestValueThresholdGBP: Double let bestValueConfigured: Bool @Binding var favouriteDropEnabled: Bool @Binding var favouriteDropFuel: FuelType @@ -178,16 +181,13 @@ struct AlertsView: View { .font(.caption) .foregroundStyle(.secondary) if alertMode == .bestValue { - Picker(String(localized: "Minimum saving", comment: "Best Value alert threshold picker title"), selection: $bestValueThresholdGBP) { - ForEach(FuelStore.bestValueAlertThresholdOptionsGBP, id: \.self) { value in - Text(FuelSavingsFormat.money(value)).tag(value) - } - } - if !bestValueConfigured { - Text(String(localized: "Set up Best Value first so FuelBoard can account for your fill size, MPG and travel cost.", comment: "Best Value alert setup hint")) - .font(.caption) - .foregroundStyle(.secondary) - } + // The threshold is edited in Settings → Fuel Savings: it + // labels station rows BEST VALUE as well as gating this + // alert, so it lives with the fill size and MPG that + // produce the saving. Read back here, not offered twice. + Text(String(format: String(localized: "Minimum saving %@ — change in Settings, Fuel Savings.", comment: "Best Value alert threshold read-back"), FuelSavingsFormat.money(bestValueThresholdGBP))) + .font(.caption) + .foregroundStyle(.secondary) } if enabled && monitoredCount == 0 { Text(String(localized: "No stations monitored yet — open the Stations tab to load prices first.", comment: "No monitored stations hint")) diff --git a/FuelBoard/ContentView.swift b/FuelBoard/ContentView.swift index 027701d..30a6cc9 100644 --- a/FuelBoard/ContentView.swift +++ b/FuelBoard/ContentView.swift @@ -29,7 +29,7 @@ struct ContentView: View { @State private var sortMode: SortMode = ContentView.qaSortModeOverride(default: FuelStore.loadSortMode()) @State private var stationLimit: Int = ContentView.qaStationRadiusOverride(default: FuelStore.loadStationLimit()) @State private var distanceUnit: DistanceUnit = FuelStore.loadDistanceUnit() - @State private var priceDisplayStyle: PriceDisplayStyle = FuelStore.loadPriceDisplayStyle() + @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 @@ -114,6 +114,21 @@ struct ContentView: View { return fallback } + /// QA hook: `-priceStyle poundsPence` renders prices in the wide pounds & + /// pence form for ONE launch. Same reason as `-sortMode`: simctl has no tap + /// command, so the wide price has to be measured against the row's column + /// widths without walking Settings → Units for every check. Never persisted. + private static func qaPriceStyleOverride(default fallback: PriceDisplayStyle) -> PriceDisplayStyle { +#if DEBUG + let args = ProcessInfo.processInfo.arguments + if let i = args.firstIndex(of: "-priceStyle"), i + 1 < args.count, + let style = PriceDisplayStyle(rawValue: args[i + 1]) { + return style + } +#endif + return fallback + } + private static func qaSortModeOverride(default fallback: SortMode) -> SortMode { #if DEBUG let args = ProcessInfo.processInfo.arguments @@ -1564,7 +1579,7 @@ struct ContentView: View { radius: $alertsRadius, fuel: $alertsFuel, alertMode: $alertsMode, - bestValueThresholdGBP: $alertsBestValueThresholdGBP, + bestValueThresholdGBP: alertsBestValueThresholdGBP, bestValueConfigured: fuelSavingsSettings.isConfigured, favouriteDropEnabled: $favouriteDropAlertsEnabled, favouriteDropFuel: $favouriteDropAlertsFuel, @@ -1592,6 +1607,7 @@ struct ContentView: View { priceDisplayStyle: $priceDisplayStyle, stationTapAction: $stationTapAction, fuelSavingsSettings: $fuelSavingsSettings, + bestValueThresholdGBP: $alertsBestValueThresholdGBP, dataRefreshMode: $dataRefreshMode, alertsFuel: alertsFuel, alertsRadiusKM: alertsRadius, @@ -1775,20 +1791,47 @@ struct StationRow: View { let distanceUnit: DistanceUnit let priceDisplayStyle: PriceDisplayStyle let baselinePrice: Double? + /// This row is the cheapest price in range — the caller's existing + /// `topStationID` test. Feeds the CHEAPEST badge. let isTopResult: Bool let isFavourite: Bool - /// Label for the cheapest-pump-price badge ("TOP" in price/distance order, - /// "CHEAPEST" when the list is ranked by net saving). - var topBadgeText: String = "TOP" var tapAction: StationTapAction = .openMap var fuelSavings: FuelSavingsAvailability? - /// The session fill-up figure (the one the adjustment control edits). The - /// compact banner does not print it today, but a row that carried a hard - /// zero would print "for £0" the moment it did — pass the real figure. - var fuelSavingsAmount: FuelSavingsFillAmount = FuelSavingsSettings.defaultPreferredAmount + /// The user's Best Value alert threshold — the app's existing gate for + /// calling a saving worth the drive, so a row labels itself BEST VALUE by + /// exactly the rule the alerts already use. Read per row (the alert path + /// reads it per evaluation too); pass explicitly to inject a value. + var bestValueThresholdGBP: Double = FuelStore.loadAlertsBestValueThresholdGBP() var onToggleFavourite: () -> Void = {} var onShowDetails: (FuelStation) -> Void = { _ in } + /// The price's point size, scaled with Dynamic Type: the price is built from + /// an explicit size (its "p"/"/L" suffixes are sized relative to it inside + /// `priceTextAttributed`), so it needs a metric rather than a font style. + /// 22pt keeps the price the row's loudest element after the station name. + @ScaledMetric(relativeTo: .title2) private var priceSize: CGFloat = 22 + + /// The identity column's share of the ROW, for every row and both price + /// styles. The mockup's divider sits at ~53% of its row; this is that ratio, + /// held per row rather than derived from the row's text — the point is that + /// the hairline, the price axis and the star cannot move as the list scrolls. + /// The price column takes the remainder, and its savings capsule scales down + /// instead of widening the column and dragging the hairline with it. + /// + /// One value for both price styles: the star used to sit at the end of the + /// price line and hold ~28pt of the price column, so the wide form `£1.58⁷/L` + /// — ~20pt wider than the bare `158.7` at the same point size — did not fit on + /// a 393pt screen. With the star now under the station logo the price column + /// has that width back, so the identity column no longer has to give width up + /// in pounds & pence, and names keep the same room in both styles. + private static let leftSectionShare: CGFloat = 0.52 + + /// The comparison label sits beside the capsule only where the row is wide + /// enough for both (regular width). On an iPhone the mockup's canvas is + /// wider than the whole screen, so the label drops under the capsule rather + /// than starving the station name. + @Environment(\.horizontalSizeClass) private var sizeClass + private var ragColor: Color { guard let price = station.prices[fuel], let baselinePrice else { return .gray } switch RAGRating.rating(price: price, cheapest: baselinePrice) { @@ -1814,6 +1857,26 @@ struct StationRow: View { return abs(price - baselinePrice) <= 0.05 } + /// The row's single recommendation badge, if any. BEST VALUE (worth the + /// drive by the alert threshold) outranks CHEAPEST; NEAREST never gets one, + /// being context rather than a recommendation. + private var recommendationBadge: StationRecommendationBadge? { + StationRecommendationBadge.badge( + netSavingGBP: fuelSavings?.result?.netSavingGBP, + isCheapest: isTopResult, + bestValueThresholdGBP: bestValueThresholdGBP + ) + } + + /// VoiceOver: the price, then the delta the redesign no longer prints — the + /// listing shows a savings capsule in its place, so the comparison against + /// the cheapest price in range would otherwise be visible only as a colour. + private func priceSpokenLabel(_ price: Double) -> String { + guard let deltaText else { return FuelStore.priceTextSpoken(price) } + let comparison = String(localized: "vs cheapest", comment: "Price delta comparison caption") + return "\(FuelStore.priceTextSpoken(price)), \(isCheapestPrice ? deltaText : "\(deltaText) \(comparison)")" + } + private func handleTap() { switch tapAction { case .openMap: @@ -1830,131 +1893,236 @@ struct StationRow: View { } var body: some View { - VStack(alignment: .leading, spacing: 6) { - HStack(spacing: 12) { - // LEFT — round brand logo (or generic fuel pump fallback) - Group { - if let asset = station.brandImageName { - Image(asset) - .resizable() - .scaledToFit() - .padding(5) - } else { - Image(systemName: "fuelpump.fill") - .font(.system(size: 22, weight: .semibold)) - .foregroundStyle(Color.black.opacity(0.82)) - } - } - .frame(width: 46, height: 46) - .background(Circle().fill(Color.white)) - .clipShape(Circle()) - .overlay(Circle().stroke(Color.primary.opacity(0.10), lineWidth: 1)) - .shadow(color: .black.opacity(0.08), radius: 2, y: 1) + // Two sections, one hairline: station identity on the left, price and + // savings on the right. No wrapper VStack and no full-width verdict + // banner under the row — the savings capsule carries the verdict inside + // the right-hand column, and the extra travel behind it is explained in + // the detail sheet instead of being repeated on every row. + HStack(alignment: .center, spacing: 12) { + // LEFT SECTION — station identity: brand logo, then name, address and + // distance. Given a FIXED SHARE of the row (~52%, the mockup's ratio) + // rather than sizing to its content: sized to content, this column + // grew and shrank with each row's badge and savings copy, which moved + // the hairline 23pt between adjacent rows (x767 on a BEST VALUE row, + // x699 on a "costs more" row) — so the divider and the price axis + // both jumped sideways as the list scrolled. One share for every row + // puts them on a single vertical line down the whole list. + HStack(alignment: .top, spacing: 12) { + // The logo, with the favourite star directly beneath it. Moving + // the star out of the price line is what gives the price column + // its width back: sat at the end of that line it held ~28pt of + // the column, which is exactly the space the wide pounds & + // pence price needs for its own digits on a 393pt screen. + // 10pt between logo and star: at 4pt the star's glyph sat almost + // against the logo's bottom edge and read as part of it rather + // than as the control it is. + VStack(spacing: 10) { + // Round brand logo (or generic fuel pump fallback) + Group { + if let asset = station.brandImageName { + Image(asset) + .resizable() + .scaledToFit() + .padding(5) + } else { + Image(systemName: "fuelpump.fill") + .font(.system(size: 22, weight: .semibold)) + .foregroundStyle(Color.black.opacity(0.82)) + } + } + .frame(width: 46, height: 46) + .background(Circle().fill(Color.white)) + .clipShape(Circle()) + .overlay(Circle().stroke(Color.primary.opacity(0.10), lineWidth: 1)) + .shadow(color: .black.opacity(0.08), radius: 2, y: 1) - // MIDDLE — name, full address, distance - VStack(alignment: .leading, spacing: 2) { - HStack(spacing: 6) { - Text(station.name) - .font(.headline) - .lineLimit(1) - .truncationMode(.tail) - ForEach(StationNotice.notices(for: station)) { notice in - Image(systemName: notice.symbolName) - .font(.caption2.weight(.semibold)) - .foregroundStyle(.secondary) - .accessibilityLabel(notice.title) + favouriteButton + } + + // Name, full address, distance + VStack(alignment: .leading, spacing: 2) { + // The name WRAPS instead of truncating: the row is tall + // enough now, and a name cut to "Z A Service Stat…" cannot + // be told apart from the station next door. Baseline + // alignment keeps the notice symbols on the name's FIRST + // line instead of floating beside a two-line name. + HStack(alignment: .firstTextBaseline, spacing: 6) { + Text(station.name) + .font(.headline) + .multilineTextAlignment(.leading) + .fixedSize(horizontal: false, vertical: true) + ForEach(StationNotice.notices(for: station)) { notice in + Image(systemName: notice.symbolName) + .font(.caption2.weight(.semibold)) + .foregroundStyle(.secondary) + .accessibilityLabel(notice.title) + } + } + // Wraps for the same reason as the name — addresses here run + // to 70 characters at the 95th percentile. + Text(StationAddressLine.display(address: station.address, + postcode: station.postcode, + name: station.name)) + .font(.caption) + .foregroundStyle(.secondary) + .multilineTextAlignment(.leading) + .fixedSize(horizontal: false, vertical: true) + if let location { + let distance = FuelStore.stationDistanceDisplay( + station: station, userLat: location.lat, userLng: location.lng + ) + // ONE-WAY distance: the listing shows how far the station + // is. The return leg stays in the savings explanation, where + // the extra travel it drives is actually costed. + HStack(spacing: 3) { + Image(systemName: "location.fill") + .font(.caption2) + .accessibilityHidden(true) + Text(distance.label(unit: distanceUnit)) + .font(.caption2) + .monospacedDigit() + } + .foregroundStyle(.secondary) + .accessibilityElement(children: .ignore) + .accessibilityLabel(distance.spokenLabel(unit: distanceUnit)) } } - Text(StationAddressLine.display(address: station.address, - postcode: station.postcode, - name: station.name)) - .font(.caption) - .foregroundStyle(.secondary) - .lineLimit(1) - .truncationMode(.tail) - if let location { - let distance = FuelStore.stationDistanceDisplay( - station: station, userLat: location.lat, userLng: location.lng - ) - Text(distance.labelWithReturn(unit: distanceUnit)) - .font(.caption2) - .foregroundStyle(.secondary) - .monospacedDigit() - .accessibilityLabel(distance.spokenLabelWithReturn(unit: distanceUnit)) - } + .frame(maxWidth: .infinity, alignment: .leading) } .frame(maxWidth: .infinity, alignment: .leading) + .containerRelativeFrame(.horizontal, alignment: .leading) { length, _ in + length * Self.leftSectionShare + } - // RIGHT — price in £ (no p) + difference vs cheapest - VStack(alignment: .trailing, spacing: 2) { - if isTopResult { - // The badge always marks the CHEAPEST PUMP PRICE (never - // "best saving"), so in Best Value order — where the - // top row is the best net saving — it may sit lower down - // the list, and says so. It sits ABOVE the price it - // annotates rather than beside the name, so it reads as - // part of the price and the name keeps its width. - Text(topBadgeText) - .font(.caption2.bold()) - .padding(.horizontal, 5) - .padding(.vertical, 1) - .background(Capsule().fill(.blue.opacity(0.15))) - .foregroundStyle(.blue) - .lineLimit(1) - .fixedSize() + // 1pt hairline between the two sections. Inset top and bottom so it + // spans the row's content without touching the row edges, and + // decorative: the columns are told apart by their content, and two + // hairs per row would just be noise to VoiceOver. + // + // `secondary`, not `separator`: over a dark row fill the system + // separator measures 7/255 brighter than the fill — measurably + // invisible — while the mockup's rule sits ~112 above it. Secondary + // lands on the mockup's own value in both appearances. + Rectangle() + .fill(Color.secondary) + .frame(width: 1) + .frame(maxHeight: .infinity) + .padding(.vertical, 6) + .accessibilityHidden(true) + + // RIGHT SECTION — price and savings, every element leading-aligned on + // ONE axis. The 12pt row spacing plus this column's leading padding + // puts that axis ~22pt right of the hairline. (The mockup centres the + // price against the badge so its status dot hangs into the gutter; the + // redesign was explicitly asked not to do that.) + VStack(alignment: .leading, spacing: 12) { + if let badge = recommendationBadge { + badgeCapsule(badge) } + // Price line: the status dot and the price, leading-aligned on + // the column's one axis. The star no longer ends this line — it + // sits under the station logo — so this whole column width now + // belongs to the price, which is what lets the wide pounds & + // pence form (£1.58⁷/L) render at full size rather than being + // scaled down or truncated. if let price = station.prices[fuel] { HStack(spacing: 5) { Circle() .fill(ragColor) .frame(width: 8, height: 8) - FuelStore.priceTextAttributed(price, style: priceDisplayStyle, size: 20, weight: .bold) - } - if let deltaText { - Text(deltaText) - .font(.caption2.bold().monospaced()) - .foregroundStyle(ragColor) - if !isCheapestPrice { - // "+8.0p" on its own never said what it measured - // against; the delta is against the cheapest price - // in range in EVERY sort mode. - Text("vs cheapest") - .font(.caption2) - .foregroundStyle(.secondary) - .lineLimit(1) - .minimumScaleFactor(0.8) - } + FuelStore.priceTextAttributed(price, style: priceDisplayStyle, + size: priceSize, weight: .bold) + .lineLimit(1) + .minimumScaleFactor(0.8) } + // The visible "+1.8p vs cheapest" line is gone with the + // redesign, so the comparison rides on the price's label. + .accessibilityElement(children: .ignore) + .accessibilityLabel(priceSpokenLabel(price)) + } + if let result = fuelSavings?.result { + savingsCapsule(result) } } - - // FAR RIGHT — favourite star - Button { - onToggleFavourite() - } label: { - Image(systemName: isFavourite ? "star.fill" : "star") - .foregroundStyle(isFavourite ? .yellow : .secondary) - } - .buttonStyle(.borderless) - } - if fuelSavings != nil { - // Full-width verdict banner under the row. Inline in the middle - // column it truncated ("Approx. £2.75 better off after tra…") - // because the - // price block squeezes that column — and the verdict is the - // point of this sort mode, so it gets its own line. - FuelSavingsSummaryView( - availability: fuelSavings, - amount: fuelSavingsAmount, - compact: true, - distanceUnit: distanceUnit - ) - } + .frame(maxWidth: .infinity, alignment: .leading) + // The spec's ~20-24pt of air between the divider and the price + // column: the row's own 12pt spacing plus this 10pt. + .padding(.leading, 10) } .contentShape(Rectangle()) .onTapGesture(perform: handleTap) .onLongPressGesture(perform: showDetails) } + + /// The favourite toggle, sitting under the station logo. Kept as its own view + /// so it stays a separate VoiceOver element — the price beside it is a single + /// grouped label — and keeps its own tap target inside the row's tap and + /// long-press gestures. + private var favouriteButton: some View { + Button { + onToggleFavourite() + } label: { + Image(systemName: isFavourite ? "star.fill" : "star") + .foregroundStyle(isFavourite ? .yellow : .secondary) + } + .buttonStyle(.borderless) + } + + /// The savings capsule plus the comparison it is measured against. The + /// capsule itself is the shared verdict view, so a row, the detail sheet and + /// the compact summary cannot drift apart. The comparison label sits beside + /// the capsule only where there is width for both. + @ViewBuilder + private func savingsCapsule(_ result: FuelSavingsResult) -> some View { + let comparison = Text(FuelSavingsFormat.comparisonLabel(status: result.status)) + .font(.caption2) + .foregroundStyle(.secondary) + .lineLimit(1) + // Same narrow column as the capsule above it, so the same floor: + // scaled copy, not a trimmed "vs closest st…". + .minimumScaleFactor(0.7) + if sizeClass == .regular { + HStack(spacing: 8) { + FuelSavingsCapsule(result: result) + comparison + } + } else { + VStack(alignment: .leading, spacing: 2) { + FuelSavingsCapsule(result: result) + comparison + } + } + } + + /// One recommendation badge, above the price it annotates. Tinted rather + /// than the mockup's solid fill, like every other badge and capsule in the + /// app: coloured text keeps its contrast in both appearances, at every + /// Dynamic Type size, without introducing a colour outside the palette. + private func badgeCapsule(_ badge: StationRecommendationBadge) -> some View { + Text(badge.title) + .font(.caption2.bold()) + .foregroundStyle(badge.tint) + .padding(.horizontal, 7) + .padding(.vertical, 3) + .background(Capsule().fill(badge.tint.opacity(0.15))) + .lineLimit(1) + .fixedSize() + } +} + +/// Badge copy and colour for `StationRecommendationBadge` — view concerns, so +/// they stay out of the shared model the tests cover. +private extension StationRecommendationBadge { + var title: String { + switch self { + case .bestValue: + return String(localized: "BEST VALUE", comment: "Station badge: saving clears the Best Value threshold") + case .cheapest: + return String(localized: "CHEAPEST", comment: "Station badge: cheapest price in range") + } + } + + var tint: Color { self == .bestValue ? .green : .blue } } // MARK: - Location manager diff --git a/FuelBoard/FavouritesView.swift b/FuelBoard/FavouritesView.swift index a9f79b0..9c0886d 100644 --- a/FuelBoard/FavouritesView.swift +++ b/FuelBoard/FavouritesView.swift @@ -161,10 +161,8 @@ struct FavouritesView: View { baselinePrice: cheapestPrice, isTopResult: station.id == cheapest?.id, isFavourite: activeFuelFavouriteIDs.contains(station.id), - topBadgeText: "CHEAPEST", tapAction: stationTapAction, fuelSavings: savingsForStation(station), - fuelSavingsAmount: fuelSavingsAmount, onToggleFavourite: { onToggleFavourite(station, activeFuel) }, onShowDetails: { detailStation = $0 } ) diff --git a/FuelBoard/FuelSavingsViews.swift b/FuelBoard/FuelSavingsViews.swift index ba25e5d..77bf605 100644 --- a/FuelBoard/FuelSavingsViews.swift +++ b/FuelBoard/FuelSavingsViews.swift @@ -257,36 +257,60 @@ struct FuelSavingsSummaryView: View { result.spokenHeadline() } - /// The colour-coded verdict capsule — ONE implementation for the listing rows - /// and the detail sheet, so the verdict reads identically in both; the sheet - /// just gets the larger font and padding. - /// - /// An HStack with an explicit spacing, NOT a Label: given the width to spare, - /// a Label lays its icon and title out across the row and leaves a visible - /// gap between them (22pt in the listing row), which read as the text having - /// drifted away from its symbol. The icon is decorative here — the verdict - /// is in the text — so VoiceOver reads the capsule as one phrase. + /// 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. +/// +/// An HStack with an explicit spacing, NOT a Label: given the width to spare, +/// a Label lays its icon and title out across the row and leaves a visible gap +/// between them (22pt in the listing row), which read as the text having +/// drifted away from its symbol. The icon is decorative here — the verdict is +/// in the text — so VoiceOver reads the capsule as one phrase. +struct FuelSavingsCapsule: View { + let result: FuelSavingsResult + var compact: Bool = true + + var body: some View { HStack(spacing: compact ? 3 : 5) { - Image(systemName: symbol(for: result.status)) + Image(systemName: Self.symbol(for: result.status)) .accessibilityHidden(true) Text(result.capsuleHeadline()) .lineLimit(1) - // Belt and braces: now that the capsule owns its line it has no - // rival for width, and shrinking beats truncating at large - // Dynamic Type sizes. - .minimumScaleFactor(0.8) + // Measured on a 393pt screen: the capsule is handed ~101pt, of + // which the icon, its spacing and the 8pt padding either side + // take 32pt, leaving ~69pt for the text. "Closest station" needs + // ~90pt at full size, so a 0.8 floor — 72pt — could not reach it + // and the row showed "Closest stati…" instead. 0.7 leaves ~63pt, + // which clears it at ~10pt type: scaled, never truncated. + .minimumScaleFactor(0.7) } .font(compact ? .caption.weight(.semibold) : .headline) - .foregroundStyle(capsuleColor(for: result.status)) + .foregroundStyle(Self.color(for: result.status)) .padding(.horizontal, compact ? 8 : 12) .padding(.vertical, compact ? 4 : 7) - .background(Capsule().fill(capsuleColor(for: result.status).opacity(0.15))) + .background(Capsule().fill(Self.color(for: result.status).opacity(0.15))) } /// Verdict colour: green save, red worse off, grey break even, blue /// baseline — the same green/amber/red family the price dot already uses. - private func capsuleColor(for status: FuelSavingsStatus) -> Color { + static func color(for status: FuelSavingsStatus) -> Color { switch status { case .worthTheDrive: return .green case .notWorthTheDrive: return .red @@ -295,7 +319,7 @@ struct FuelSavingsSummaryView: View { } } - private func symbol(for status: FuelSavingsStatus) -> String { + static func symbol(for status: FuelSavingsStatus) -> String { switch status { case .worthTheDrive: return "checkmark.circle.fill" case .breakEven: return "equal.circle.fill" diff --git a/FuelBoard/SettingsView.swift b/FuelBoard/SettingsView.swift index 2439f6e..b8b6804 100644 --- a/FuelBoard/SettingsView.swift +++ b/FuelBoard/SettingsView.swift @@ -20,6 +20,12 @@ struct SettingsView: View { @Binding var priceDisplayStyle: PriceDisplayStyle @Binding var stationTapAction: StationTapAction @Binding var fuelSavingsSettings: FuelSavingsSettings + /// The Best Value gate — the net saving that makes a station worth the trip. + /// Edited here, with the fill size and MPG that PRODUCE that saving, rather + /// than in the Alerts tab: the same figure also decides which station rows + /// carry the BEST VALUE badge, so it is a fuel-savings setting first and an + /// alert setting second. Stored under the alerts key either way. + @Binding var bestValueThresholdGBP: Double @Binding var dataRefreshMode: DataRefreshMode /// The fuel + radius currently configured for alerts (mirrors the Alerts /// tab) so the test notification matches what real alerts will say. @@ -149,6 +155,20 @@ struct SettingsView: View { step: FuelSavingsCalculator.fillAmountSliderStep(for: fuelSavingsSettings.preferredAmount.unit)) .accessibilityLabel("Typical fill-up") .accessibilityValue(fillUpText) + Picker("Minimum saving", selection: $bestValueThresholdGBP) { + ForEach(FuelStore.bestValueAlertThresholdOptionsGBP, id: \.self) { value in + Text(FuelSavingsFormat.money(value)).tag(value) + } + } + // Persisted by ContentView's single + // `.onChange(of: alertsBestValueThresholdGBP)`, which + // also refreshes the alert monitor — so this picker + // needs no handler of its own. The store snaps to the + // nearest option, so the picker and the stored value + // cannot disagree. + Text("A saving worth driving for: the net saving left after the extra travel cost. Stations above it are labelled BEST VALUE in the station list, and Best Value alerts use the same figure.") + .font(.caption) + .foregroundStyle(.secondary) if let settingsUnitChangeNotice { Text(settingsUnitChangeNotice) .font(.caption2) diff --git a/FuelBoard/StationsView.swift b/FuelBoard/StationsView.swift index 4db56e5..60a5dfd 100644 --- a/FuelBoard/StationsView.swift +++ b/FuelBoard/StationsView.swift @@ -356,10 +356,8 @@ struct StationsView: View { baselinePrice: baselinePrice, isTopResult: station.id == topStationID, isFavourite: favouriteIDs.contains(station.id), - topBadgeText: sortMode == .worthTheDrive ? "CHEAPEST" : "TOP", tapAction: stationTapAction, fuelSavings: savingsForStation(station), - fuelSavingsAmount: fuelSavingsAmount, onToggleFavourite: { onToggleFavourite(station, selectedFuel) }, onShowDetails: { detailStation = $0 } ) diff --git a/FuelBoard/en.lproj/Localizable.strings b/FuelBoard/en.lproj/Localizable.strings index 4a65a3e..245f287 100644 --- a/FuelBoard/en.lproj/Localizable.strings +++ b/FuelBoard/en.lproj/Localizable.strings @@ -10,6 +10,8 @@ "Settings" = "Settings"; "FuelBoard" = "FuelBoard"; "TOP" = "TOP"; +"BEST VALUE" = "BEST VALUE"; +"CHEAPEST" = "CHEAPEST"; /* Stations tab */ "Closest %@ stations — closest first, lowest price within %lld %@ · %lld stations" = "Closest %@ stations — closest first, lowest price within %lld %@ · %lld stations"; @@ -207,6 +209,7 @@ "Best Value" = "Best Value"; "Total" = "Total"; "Minimum saving" = "Minimum saving"; +"Minimum saving %@ — change in Settings, Fuel Savings." = "Minimum saving %@ — change in Settings, Fuel Savings."; "Set up Best Value first so FuelBoard can account for your fill size, MPG and travel cost." = "Set up Best Value first so FuelBoard can account for your fill size, MPG and travel cost."; "Set up Best Value first so Best Value alerts can account for your fill size, MPG and travel cost." = "Set up Best Value first so Best Value alerts can account for your fill size, MPG and travel cost."; "When you approach the Best Value station within the radius, FuelBoard sends a notification if it could leave you at least %@ better off after extra return travel. Nearby stations selling %@ are watched — favourites get priority — and each station alerts at most once per hour." = "When you approach the Best Value station within the radius, FuelBoard sends a notification if it could leave you at least %@ better off after extra return travel. Nearby stations selling %@ are watched — favourites get priority — and each station alerts at most once per hour."; diff --git a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsTests.swift b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsTests.swift index 7ccac71..37e806b 100644 --- a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsTests.swift +++ b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsTests.swift @@ -159,3 +159,101 @@ final class FuelSavingsSettingsTests: XCTestCase { XCTAssertTrue(FuelSavingsSettings(enabled: true, mpg: 45, preferredAmount: .init(value: 40, unit: .pounds)).isConfigured) } } + +/// The row's single recommendation badge. Presentation only, but the rules are +/// worth pinning: they decide which of two true statements a row makes, and +/// "no badge" is itself an answer. +final class StationRecommendationBadgeTests: XCTestCase { + private let threshold = 1.0 + + func testSavingAtTheThresholdEarnsBestValue() { + XCTAssertEqual( + StationRecommendationBadge.badge(netSavingGBP: 1.0, isCheapest: false, + bestValueThresholdGBP: threshold), + .bestValue + ) + } + + func testSavingBelowTheThresholdDoesNot() { + XCTAssertNil( + StationRecommendationBadge.badge(netSavingGBP: 0.42, isCheapest: false, + bestValueThresholdGBP: threshold) + ) + } + + func testCheapestWithoutAWorthwhileSavingStillSaysCheapest() { + XCTAssertEqual( + StationRecommendationBadge.badge(netSavingGBP: 0.42, isCheapest: true, + bestValueThresholdGBP: threshold), + .cheapest + ) + } + + /// The cheapest station keeps its badge even when the drive costs more than + /// the saving buys — it IS the cheapest price; the capsule underneath is + /// what says the trip is not worth it. + func testCheapestKeepsItsBadgeWhenTheDriveCostsMoreThanItSaves() { + XCTAssertEqual( + StationRecommendationBadge.badge(netSavingGBP: -14.30, isCheapest: true, + bestValueThresholdGBP: threshold), + .cheapest + ) + } + + /// One badge per row: the stronger statement wins. + func testBestValueOutranksCheapestWhenBothApply() { + XCTAssertEqual( + StationRecommendationBadge.badge(netSavingGBP: 3.15, isCheapest: true, + bestValueThresholdGBP: threshold), + .bestValue + ) + } + + /// A near-by station with no saving to speak of is context, not a + /// recommendation — the mockup gives NEAREST no capsule, and nothing else + /// here may invent one. + func testNoSavingAndNotCheapestMeansNoBadge() { + XCTAssertNil( + StationRecommendationBadge.badge(netSavingGBP: 0.0, isCheapest: false, + bestValueThresholdGBP: threshold) + ) + XCTAssertNil( + StationRecommendationBadge.badge(netSavingGBP: -2.50, isCheapest: false, + bestValueThresholdGBP: threshold) + ) + } + + func testUnavailableSavingsLeaveOnlyTheCheapestBadge() { + XCTAssertEqual( + StationRecommendationBadge.badge(netSavingGBP: nil, isCheapest: true, + bestValueThresholdGBP: threshold), + .cheapest + ) + XCTAssertNil( + StationRecommendationBadge.badge(netSavingGBP: nil, isCheapest: false, + bestValueThresholdGBP: threshold) + ) + } + + /// A rubbish figure must not become a recommendation. + func testNonFiniteFiguresProduceNoBadge() { + XCTAssertNil( + StationRecommendationBadge.badge(netSavingGBP: .nan, isCheapest: false, + bestValueThresholdGBP: threshold) + ) + XCTAssertNil( + StationRecommendationBadge.badge(netSavingGBP: 5.0, isCheapest: false, + bestValueThresholdGBP: .nan) + ) + } + + /// The threshold is the user's alert setting: set it to zero and any saving + /// that is not a loss counts. + func testZeroThresholdAwardsBestValueToAnyNonNegativeSaving() { + XCTAssertEqual( + StationRecommendationBadge.badge(netSavingGBP: 0, isCheapest: false, + bestValueThresholdGBP: 0), + .bestValue + ) + } +} diff --git a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelStoreTests.swift b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelStoreTests.swift new file mode 100644 index 0000000..fdcffc3 --- /dev/null +++ b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelStoreTests.swift @@ -0,0 +1,33 @@ +import XCTest +#if SWIFT_PACKAGE +@testable import FuelBoardShared +#endif + +/// The Best Value gate's option list. +/// +/// This one array is the figure behind two user-facing things — the BEST VALUE +/// badge on a station row and the proximity alert — so its granularity is a +/// setting, not an implementation detail. It was four coarse values (50p, £1, +/// £2, £5); these tests pin the 50p step and the £5 ceiling so a later edit +/// cannot quietly coarsen it back. +final class FuelStoreBestValueThresholdTests: XCTestCase { + func testOptionsStepByFiftyPenceUpToFivePounds() { + let options = FuelStore.bestValueAlertThresholdOptionsGBP + XCTAssertEqual(options, [0.5, 1.0, 1.5, 2.0, 2.5, 3.0, 3.5, 4.0, 4.5, 5.0]) + XCTAssertEqual(options.count, 10) + XCTAssertEqual(options.first, 0.5, "the floor is the cheapest worth-the-drive saving offered") + XCTAssertEqual(options.last, 5.0, "£5 is the ceiling") + } + + /// The store writes the threshold as a string and, on load, rejects any + /// value that is not one of these options — falling back to the £1 default. + /// So an option that does not survive `String` → `Double` exactly would + /// silently discard the user's choice. Multiples of 0.5 are exactly + /// representable in binary, and this keeps them that way. + func testEveryOptionSurvivesTheStoresStringRoundTrip() { + for option in FuelStore.bestValueAlertThresholdOptionsGBP { + XCTAssertEqual(Double(String(option)), option, + "£\(option) does not round-trip through the store's string encoding") + } + } +} diff --git a/Shared/FuelSavings.swift b/Shared/FuelSavings.swift index 27d5dc2..81ad3f3 100644 --- a/Shared/FuelSavings.swift +++ b/Shared/FuelSavings.swift @@ -262,6 +262,29 @@ struct FuelSavingsRankedStation: Equatable { let pricePencePerLitre: Double } +/// Which recommendation badge a station listing row shows, if any. +/// +/// Presentation only — no new rules: the BEST VALUE qualification reuses the +/// app's existing Best Value gate (the user's alert threshold, the same figure +/// `FuelSavingsCalculator.bestValueAlertCandidate` is given) and CHEAPEST +/// reuses the caller's existing "cheapest in range" flag (the station behind +/// `topStationID`). One badge per row: a station that is both the cheapest and +/// worth the drive shows BEST VALUE, the stronger of the two. +enum StationRecommendationBadge: Equatable { + case bestValue + case cheapest + + static func badge(netSavingGBP: Double?, + isCheapest: Bool, + bestValueThresholdGBP: Double) -> StationRecommendationBadge? { + if let netSavingGBP, netSavingGBP.isFinite, bestValueThresholdGBP.isFinite, + netSavingGBP >= bestValueThresholdGBP { + return .bestValue + } + return isCheapest ? .cheapest : nil + } +} + struct BestValueAlertCandidate: Equatable { let station: FuelStation let result: FuelSavingsResult diff --git a/Shared/FuelStore.swift b/Shared/FuelStore.swift index 2edb484..d556de9 100644 --- a/Shared/FuelStore.swift +++ b/Shared/FuelStore.swift @@ -711,7 +711,16 @@ struct FuelStore { saveString(mode.rawValue, service: alertsModeKey) } - static let bestValueAlertThresholdOptionsGBP: [Double] = [0.5, 1.0, 2.0, 5.0] + /// The Best Value gate's options, in 50p steps from 50p to £5. The gate is + /// the same figure for the listing's BEST VALUE badges and for the alert, so + /// its granularity decides how finely the two can be tuned: the old four + /// values (50p, £1, £2, £5) made £2.50 and £3 unreachable, and a whole extra + /// detour's worth of saving sat between £1 and £2. Multiples of 0.5 are + /// exactly representable, so the stored value and the picker's tags cannot + /// drift apart. + static let bestValueAlertThresholdOptionsGBP: [Double] = [ + 0.5, 1.0, 1.5, 2.0, 2.5, 3.0, 3.5, 4.0, 4.5, 5.0 + ] static func loadAlertsBestValueThresholdGBP() -> Double { if let raw = loadString(service: alertsBestValueThresholdKey),