From 8153a36168c498d5180ee45fba2a70f16c2d86b2 Mon Sep 17 00:00:00 2001 From: FuelBoard Contributor Date: Mon, 19 Oct 2026 14:59:47 +0100 Subject: [PATCH] Make Closest a station label and retire Top MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Top and Cheapest were two words for one state: both meant "the lowest price among the stations on screen", differing only in the candidate set (radius vs Favourites). Which word a surface used had drifted by device — the phone's rows said Cheapest while the watch said Top for the identical state. Top is gone from the enum, the Key legend, the badge and the watch's hand-rolled label; Cheapest survives because it names the measure and is already the word the favourites copy uses. Closest is now a real station label rather than a verdict. StationRecommendationBadge gains .closest with a total precedence — Best value, then Cheapest, then Closest — because a row has room for one label and proximity is the weakest claim: a short drive says nothing about the price, and unlike the other two it reports no saving at all. The badge sits above the price, where the other two sit. The closest row's verdict capsule is suppressed: its only word for that state was "Closest", so printing it below as well would state the same fact twice, three lines apart. Other rows keep their Saves / Break even / Costs capsules unchanged. "Closest station" becomes "Closest", read from the single FuelSavingsResult.baselineHeadline constant so the badge, the verdict and the Key cannot word it differently. The Key legend renders all three station labels with the shared StationLabelBadge; the old KeyStationLabelIndicator, which special-cased the closest station to a wider verdict pill so it read as a different class of thing from the two above it, is deleted. One closest icon everywhere. The listing and detail verdict symbols used location.circle and location.circle.fill for the state the Key taught as location.north.fill; both now read StationLabelKind.closest.symbol, so the three surfaces resolve one glyph from one place. The detail sheet gains the Closest capsule below the map, ordered to match the Key. Favourites gains the nearest favourite as its Closest, measured on the same one-way distance its rows print and restricted to favourites with a price for the active fuel — matching the radius tab, where the closest station is by definition the savings baseline. closestStationID flows from the one scaffold baseline the savings lookup already compares against, so a row's badge and the reference station cannot disagree. No savings calculation, threshold, ranking, eligibility or station-selection logic changed. The Key sheet's detent is now bound rather than fixed, so dragging it up to read the Station labels section sticks instead of snapping back; a QA launch argument opens it at full height, since the 0.6 detent puts that section below the fold on every device. That argument is read unconditionally, unlike the other harness hooks which are #if DEBUG-gated. Tests: 278 pass, 9 new — the label precedence ladder across every combination, capsule order in the sheet, Closest alone, and the retired label pinned absent. --- FuelBoard/CapsuleStyles.swift | 16 +- FuelBoard/ContentView.swift | 63 ++++--- FuelBoard/FavouritesView.swift | 31 +++- FuelBoard/FuelSavingsViews.swift | 12 +- FuelBoard/StationDetailSections.swift | 2 +- FuelBoard/StationDetailView.swift | 14 +- FuelBoard/StationsView.swift | 74 ++++---- FuelBoard/en.lproj/Localizable.strings | 7 +- .../FuelSavingsCalculationAuditTests.swift | 2 +- .../FuelSavingsEstimateTests.swift | 12 +- .../FuelSavingsTests.swift | 171 ++++++++++++++++-- FuelBoardWatch Watch App/ContentView.swift | 8 +- Shared/FuelSavings.swift | 78 +++++--- 13 files changed, 341 insertions(+), 149 deletions(-) diff --git a/FuelBoard/CapsuleStyles.swift b/FuelBoard/CapsuleStyles.swift index 1430614..12d2ddf 100644 --- a/FuelBoard/CapsuleStyles.swift +++ b/FuelBoard/CapsuleStyles.swift @@ -64,17 +64,10 @@ extension CapsuleTone { /// Neutral grey: proximity. Same family as `breakEven` — both mean "no /// judgement" — but kept as its own pair so proximity and a break-even /// verdict stay independently tunable. - static let closestStation = CapsuleTone( + static let closest = CapsuleTone( lightForeground: Color(hex: 0x525967), lightBackground: Color(hex: 0xE8EAF0), darkForeground: Color(hex: 0xC0C3CC), darkBackground: Color(hex: 0x424349) ) - - /// Blue, like `cheapest`: the top favourite IS the cheapest favourite, so it - /// must not read as a different class of thing. - static let top = CapsuleTone( - lightForeground: Color(hex: 0x225F9B), lightBackground: Color(hex: 0xE0EFFB), - darkForeground: Color(hex: 0x8BC5FA), darkBackground: Color(hex: 0x293F53) - ) } /// The one shape rule every capsule in the app follows. @@ -83,9 +76,10 @@ extension CapsuleTone { /// wraps. A pill's semicircular ends crowd the text corners at two lines or /// more, while a 10pt radius does not. The shape follows what the content /// actually DREW, not how many lines it declared, because a label can wrap in a -/// narrow column without the data changing — "Closest station" is the case that -/// forced this: one declared line that wraps to two in the listing's price -/// column. +/// narrow column without the data changing — the baseline label was the case that +/// forced this: "Closest station" was one declared line that wrapped to two in the +/// listing's price column, which is also why that label is now the single word +/// "Closest". /// /// Measuring happens on the content BEFORE padding, so the drawn line count is /// never inflated by the capsule's own insets. diff --git a/FuelBoard/ContentView.swift b/FuelBoard/ContentView.swift index b246631..a5d2dc0 100644 --- a/FuelBoard/ContentView.swift +++ b/FuelBoard/ContentView.swift @@ -321,29 +321,29 @@ struct ContentView: View { return pool.compactMap { $0.prices[selectedFuel] }.min() } - /// The "best" station — the cheapest in the pool — gets the TOP badge. + /// The cheapest station in the pool — the one that wears the Cheapest badge. /// First occurrence in display order wins on price ties. - private var topStationID: String? { + private var cheapestStationID: String? { guard let baselinePrice else { return nil } - return topStationID(in: displayedStations, baselinePrice: baselinePrice) + return cheapestStationID(in: displayedStations, baselinePrice: baselinePrice) } - /// The TOP-badge station within an ALREADY-SORTED list. Split out so a caller - /// that has the sorted list (the Stations tab) does not sort the whole + /// The Cheapest-badge station within an ALREADY-SORTED list. Split out so a + /// caller that has the sorted list (the Stations tab) does not sort the whole /// country a second time just to find one id. - private func topStationID(in list: [FuelStation], baselinePrice: Double) -> String? { + private func cheapestStationID(in list: [FuelStation], baselinePrice: Double) -> String? { list.first { $0.prices[selectedFuel] == baselinePrice }?.id } - /// The app's own "cheapest in range" — the TOP-badge station — mirrored + /// The app's own "cheapest in range" — the Cheapest-badge station — mirrored /// into the Settings → Debug section so its "Cheapest in range" row /// matches the app list (and the widget when the widget's fuel + distance /// match the app's). The debug section previously showed the ALERT /// prediction (alertsFuel + alert radius), which is a different pool by /// design — that mismatch is what this fixes. private var appCheapestStatus: DebugAppCheapest? { - guard let topID = topStationID, - let station = stations.first(where: { $0.id == topID }), + guard let cheapestID = cheapestStationID, + let station = stations.first(where: { $0.id == cheapestID }), let location else { return nil } return DebugAppCheapest( station: station, @@ -428,7 +428,7 @@ struct ContentView: View { /// /// The baseline is chosen with the same journey guards as /// `FuelSavingsEstimator.baselineStation` when configured; with the feature - /// off, the ordinary Closest station still names that row. + /// off, the ordinary closest station still names that row. private func savingsScaffold() -> (baseline: FuelStation?, journeys: [String: FuelSavingsJourney]) { guard let location else { return (nil, [:]) } guard fuelSavingsSettings.isConfigured else { @@ -833,7 +833,10 @@ struct ContentView: View { // The same "cheapest in range" test this view gives its // rows, so the sheet's Cheapest capsule matches the badge on // the list it was opened from. - isCheapestInRange: station.id == topStationID, + isCheapestInRange: station.id == cheapestStationID, + // The same "closest" test the rows get, from the ONE + // scaffold baseline the savings lookup also compares against. + isClosestInRange: station.id == scaffold.baseline?.id, isFavourite: favouriteIDs.contains(station.id), onToggleFavourite: { toggleFavourite(station, fuel: selectedFuel) } ) @@ -1528,15 +1531,15 @@ struct ContentView: View { /// within the compiler's type-check budget. private var stationsTab: some View { // ONE scaffold (baseline + journeys) and ONE sort serve the list, its - // count, the TOP badge and every savings label. Asking for + // count, the Cheapest badge and every savings label. Asking for // `nearestSavingsBaseline`, `displayedStations`, `sortedStations.count`, - // `topStationID` and `savingsLookup` separately walked the full ~8k list - // six times per render — seconds of main-thread work in Closest mode, + // `cheapestStationID` and `savingsLookup` separately walked the full ~8k + // list six times per render — seconds of main-thread work in Closest mode, // where the pool is every station in the UK. let scaffold = savingsScaffold() let baselineStation = scaffold.baseline let displayed = displayedStations - let topID = baselinePrice.flatMap { topStationID(in: displayed, baselinePrice: $0) } + let cheapestID = baselinePrice.flatMap { cheapestStationID(in: displayed, baselinePrice: $0) } return StationsView( stations: displayed, totalCount: displayed.count, @@ -1547,7 +1550,8 @@ struct ContentView: View { distanceUnit: distanceUnit, priceDisplayStyle: priceDisplayStyle, baselinePrice: baselinePrice, - topStationID: topID, + cheapestStationID: cheapestID, + closestStationID: baselineStation?.id, location: location, favouriteIDs: favouriteIDs, stationTapAction: stationTapAction, @@ -1801,8 +1805,11 @@ struct StationRow: View { 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 + /// `cheapestStationID` test. Feeds the Cheapest badge. + let isCheapestResult: Bool + /// This row is the station savings are measured against — the caller's + /// `closestStationID` test. Feeds the Closest badge. + let isClosestResult: Bool let isFavourite: Bool var tapAction: StationTapAction = .openMap var fuelSavings: FuelSavingsAvailability? @@ -1862,12 +1869,13 @@ struct StationRow: View { } /// 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. + /// drive by the alert threshold) outranks CHEAPEST, which outranks CLOSEST — + /// see `StationRecommendationBadge.badge` for why proximity ranks last. private var recommendationBadge: StationRecommendationBadge? { StationRecommendationBadge.badge( netSavingGBP: fuelSavings?.result?.netSavingGBP, - isCheapest: isTopResult, + isCheapest: isCheapestResult, + isClosest: isClosestResult, bestValueThresholdGBP: bestValueThresholdGBP ) } @@ -2051,7 +2059,11 @@ struct StationRow: View { .accessibilityElement(children: .ignore) .accessibilityLabel(priceSpokenLabel(price)) } - if let result = fuelSavings?.result { + // The closest station wears the Closest badge above its price + // and gets no verdict capsule: the capsule's only word for that + // state was "Closest", so printing it here as well would say + // the same thing twice, three lines apart. + if let result = fuelSavings?.result, !isClosestResult { savingsCapsule(result) } } @@ -2080,9 +2092,9 @@ struct StationRow: View { } /// The listing's verdict capsule — ONE component for every status, so the - /// baseline's "Closest station" pill carries the same soft blue fill, white - /// text and shape rules as Saves, Costs and Break even. The detail sheet - /// keeps `FuelSavingsCapsule` to itself, unchanged. + /// baseline's "Closest" pill carries the same fill, text and shape rules as + /// Saves, Costs and Break even. The detail sheet keeps `FuelSavingsCapsule` to + /// itself, unchanged. private func savingsCapsule(_ result: FuelSavingsResult) -> some View { ListingFuelSavingsCapsule(result: result) } @@ -2106,6 +2118,7 @@ private extension StationRecommendationBadge { switch self { case .bestValue: return .bestValue case .cheapest: return .cheapest + case .closest: return .closest } } } diff --git a/FuelBoard/FavouritesView.swift b/FuelBoard/FavouritesView.swift index b807650..4c40b4b 100644 --- a/FuelBoard/FavouritesView.swift +++ b/FuelBoard/FavouritesView.swift @@ -77,6 +77,26 @@ struct FavouritesView: View { } private var cheapestPrice: Double? { cheapest?.prices[activeFuel] } + /// Nearest favourited station for the active fuel — this tab's Closest. + /// + /// Measured on the same one-way distance the rows print, so the badge cannot + /// point at a station the list shows as further away than its neighbour. Only + /// favourites with a price for the active fuel are eligible, matching the + /// radius tab, where the closest station is by definition the savings + /// baseline; an unpriced favourite appears last and wears no label. Ties keep + /// the user's own order, which is what `ordered` already encodes. + private var nearest: FuelStation? { + guard let location else { return nil } + var best: (station: FuelStation, km: Double)? + for station in ordered where station.prices[activeFuel] != nil { + let km = FuelStore.stationDistanceDisplay( + station: station, userLat: location.lat, userLng: location.lng + ).km + if best == nil || km < best!.km { best = (station, km) } + } + return best?.station + } + /// The baseline note is useful only when at least one visible favourite has /// an actual savings comparison. An unavailable result (for example, while /// location is missing) must not leave an orphaned explanation below the list. @@ -183,7 +203,8 @@ struct FavouritesView: View { distanceUnit: distanceUnit, priceDisplayStyle: priceDisplayStyle, baselinePrice: cheapestPrice, - isTopResult: station.id == cheapest?.id, + isCheapestResult: station.id == cheapest?.id, + isClosestResult: station.id == nearest?.id, isFavourite: activeFuelFavouriteIDs.contains(station.id), tapAction: stationTapAction, fuelSavings: savingsForStation(station), @@ -261,9 +282,11 @@ struct FavouritesView: View { fuelSavings: savingsForStation(station), fuelSavingsAmount: fuelSavingsAmount, journeyDiagnostics: journeyDiagnosticsForStation(station), - // This tab's cheapest favourite — the station its rows badge, - // and the one the Key calls "Top". - isTopFavourite: station.id == cheapest?.id, + // This tab's cheapest favourite — the same flag its rows get, + // and the same label they wear. + isCheapestInRange: station.id == cheapest?.id, + // This tab's nearest favourite — the same flag its rows get. + isClosestInRange: station.id == nearest?.id, // Every station listed here IS a favourite for the tab's // fuel, so the star starts filled; tapping it unstars. isFavourite: true, diff --git a/FuelBoard/FuelSavingsViews.swift b/FuelBoard/FuelSavingsViews.swift index fe5329e..bed5e6b 100644 --- a/FuelBoard/FuelSavingsViews.swift +++ b/FuelBoard/FuelSavingsViews.swift @@ -140,7 +140,7 @@ struct FuelSavingsCapsule: View { case .worthTheDrive: return .saves case .notWorthTheDrive: return .costs case .breakEven: return .breakEven - case .baseline: return .closestStation + case .baseline: return .closest } } @@ -155,7 +155,11 @@ struct FuelSavingsCapsule: View { case .worthTheDrive: return "checkmark.circle.fill" case .breakEven: return "equal.circle.fill" case .notWorthTheDrive: return "xmark.circle.fill" - case .baseline: return "location.circle.fill" + // The ONE closest glyph, shared with the Key legend and the station + // badges (`StationLabelKind.closest`). The other three verdicts are + // circular because they report an outcome; closest reports a position, + // and a north arrow is what the legend teaches for it. + case .baseline: return StationLabelKind.closest.symbol } } } @@ -242,7 +246,9 @@ struct ListingFuelSavingsCapsule: View { case .worthTheDrive: return "checkmark.circle" case .notWorthTheDrive: return "arrow.up.circle" case .breakEven: return "equal.circle" - case .baseline: return "location.circle" + // Same closest glyph as the detail verdict and the Key — this capsule is + // the listing's copy of the same state, not a third icon for it. + case .baseline: return StationLabelKind.closest.symbol } } } diff --git a/FuelBoard/StationDetailSections.swift b/FuelBoard/StationDetailSections.swift index 5321c57..33e84b7 100644 --- a/FuelBoard/StationDetailSections.swift +++ b/FuelBoard/StationDetailSections.swift @@ -209,7 +209,7 @@ struct BestValueDetailCard: View { /// 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". + /// after the panels — the capsule above already says "Closest". @ViewBuilder private func workings(_ result: FuelSavingsResult) -> some View { if result.status != .baseline { diff --git a/FuelBoard/StationDetailView.swift b/FuelBoard/StationDetailView.swift index 7dd5936..2be8675 100644 --- a/FuelBoard/StationDetailView.swift +++ b/FuelBoard/StationDetailView.swift @@ -26,13 +26,15 @@ struct StationDetailView: View { var fuelSavingsAmount: FuelSavingsFillAmount = FuelSavingsSettings.defaultPreferredAmount var savingsBaselineStationName: String? = nil var journeyDiagnostics: FuelSavingsJourneyDiagnostics? = nil - /// Ranking flags, supplied by the list that opened this sheet — the SAME flags + /// Ranking flags, supplied by the list that opened this sheet — the SAME flag /// it hands its own rows, so this sheet cannot award a capsule the row behind it - /// did not. `isCheapestInRange` is the Stations tab's cheapest price in range; - /// `isTopFavourite` is the cheapest of your Favourites. A host passes one or the - /// other, never both, so one achievement is never labelled twice. + /// did not. Both tabs mean the same thing by "cheapest": the lowest price among + /// the stations they are listing, whether that is the radius or the Favourites. var isCheapestInRange: Bool = false - var isTopFavourite: Bool = false + /// The same "closest" test the host gives its own rows. Its meaning follows + /// the tab, exactly as Cheapest does: the nearest station in the radius, or + /// the nearest favourite. + var isClosestInRange: Bool = false /// The user's Best Value alert threshold — the app's existing gate, defaulted /// exactly as `StationRow` defaults it, so the capsule here and the badge on the /// list agree to the penny. @@ -84,7 +86,7 @@ struct StationDetailView: View { StationRanking.kinds( netSavingGBP: fuelSavings?.result?.netSavingGBP, isCheapestInRange: isCheapestInRange, - isTopFavourite: isTopFavourite, + isClosestInRange: isClosestInRange, bestValueThresholdGBP: bestValueThresholdGBP ) } diff --git a/FuelBoard/StationsView.swift b/FuelBoard/StationsView.swift index 0709e25..a829bbe 100644 --- a/FuelBoard/StationsView.swift +++ b/FuelBoard/StationsView.swift @@ -15,6 +15,16 @@ struct KeySheetView: View { @Binding var showImportantInformation: Bool @Environment(\.dismiss) private var dismiss + /// Where the user last dragged the sheet. Bound rather than fixed at 0.6 so + /// dragging it up to read the Station labels section sticks, instead of + /// snapping back every time the sheet is reopened. + /// + /// The QA launch argument sets the INITIAL detent rather than assigning one + /// after the sheet appears: assigning a new selection while the sheet is still + /// presenting is ignored, which left the legend stuck at 0.6 and the Station + /// labels section off screen. + @State private var detent: PresentationDetent = + ProcessInfo.processInfo.arguments.contains("-qaKeySheetLarge") ? .large : .fraction(0.6) var body: some View { NavigationStack { @@ -46,7 +56,7 @@ struct KeySheetView: View { Section { ForEach(StationLabelKind.allCases, id: \.self) { kind in KeySheetDescriptionRow(item: item(for: kind)) { - KeyStationLabelIndicator(kind: kind) + StationLabelBadge(kind: kind) } } } header: { @@ -81,7 +91,11 @@ struct KeySheetView: View { // Nested sheet: the Key sheet stays behind the disclaimer. .importantInformationSheet(isPresented: $showImportantInformation) } - .presentationDetents([.fraction(0.6), .large]) + // QA hook: `-qaKeySheetLarge` opens the legend at full height — see the + // `detent` state. The 0.6 detent puts the Station labels section below the + // fold on every device, including the tallest iPhone, so the three label + // badges cannot otherwise be seen on screen. + .presentationDetents([.fraction(0.6), .large], selection: $detent) .presentationBackground(Color(UIColor.systemGroupedBackground)) } @@ -107,7 +121,11 @@ struct StationsView: View { let distanceUnit: DistanceUnit let priceDisplayStyle: PriceDisplayStyle let baselinePrice: Double? - let topStationID: String? + let cheapestStationID: String? + /// The station the savings are measured against — the closest station with an + /// eligible price. Passed in from the owner of the scaffold baseline so this + /// list's Closest badge and the reference station cannot disagree. + let closestStationID: String? let location: Coordinate? let favouriteIDs: Set let stationTapAction: StationTapAction @@ -119,7 +137,7 @@ struct StationsView: View { var onRefresh: () async -> Void = {} /// Pagination: one page = 10 rows, reset whenever the underlying list - /// changes (new fetch, fuel/sort/radius switch). RAG/TOP/deltas still come + /// changes (new fetch, fuel/sort/radius switch). RAG/badge/deltas still come /// from the whole radius pool via `cheapestPrice` — paging never changes /// which station is "best". @State private var pageSize = 10 @@ -386,8 +404,10 @@ struct StationsView: View { fuelSavings: savingsForStation(station), fuelSavingsAmount: fuelSavingsAmount, journeyDiagnostics: journeyDiagnosticsForStation(station), - // The same flag the row gets from `topStationID`. - isCheapestInRange: station.id == topStationID, + // The same flag the row gets from `cheapestStationID`. + isCheapestInRange: station.id == cheapestStationID, + // The same flag the row gets from `closestStationID`. + isClosestInRange: station.id == closestStationID, isFavourite: favouriteIDs.contains(station.id), onToggleFavourite: { onToggleFavourite(station, selectedFuel) } ) @@ -411,7 +431,8 @@ struct StationsView: View { distanceUnit: distanceUnit, priceDisplayStyle: priceDisplayStyle, baselinePrice: baselinePrice, - isTopResult: station.id == topStationID, + isCheapestResult: station.id == cheapestStationID, + isClosestResult: station.id == closestStationID, isFavourite: favouriteIDs.contains(station.id), tapAction: stationTapAction, fuelSavings: savingsForStation(station), @@ -557,8 +578,7 @@ struct StationLabelBadge: View { switch kind { case .bestValue: return .bestValue case .cheapest: return .cheapest - case .closestStation: return .closestStation - case .top: return .top + case .closest: return .closest } } @@ -578,34 +598,14 @@ struct StationLabelBadge: View { } } -/// The Key sheet's indicator for a station label. The closest station is the one -/// exception: it renders the listing's real verdict capsule, so the legend -/// demonstrates the same fill, text and shape the rows show. Everything else is -/// the shared badge. -private struct KeyStationLabelIndicator: View { - let kind: StationLabelKind - - var body: some View { - if kind == .closestStation { - ListingFuelSavingsCapsule(result: baselineResult) - } else { - StationLabelBadge(kind: kind) - } - } - - private var baselineResult: FuelSavingsResult { - FuelSavingsResult( - fillLitres: 30, - grossSavingGBP: 0, - extraDrivingCostGBP: 0, - extraDistanceMiles: 0, - netSavingGBP: 0, - status: .baseline, - journeyConfidence: .confirmed - ) - } -} - +/// Every station label in the Key is the SAME `StationLabelBadge` the listing rows +/// and the More info sheet draw. +/// +/// The closest station used to be the exception — the legend rendered the real +/// verdict capsule for it, a wider pill with different fill and shape, so it read as +/// a different class of thing from Best value and Cheapest sitting above it. Now +/// that its label is the single word "Closest", it belongs in the same column, in +/// the same capsule, at the same size. // MARK: - Fuel-type iconography (app target only — FuelStore.swift is Foundation-only) extension FuelType { diff --git a/FuelBoard/en.lproj/Localizable.strings b/FuelBoard/en.lproj/Localizable.strings index 2e72afa..c880330 100644 --- a/FuelBoard/en.lproj/Localizable.strings +++ b/FuelBoard/en.lproj/Localizable.strings @@ -9,10 +9,9 @@ "Alerts" = "Alerts"; "Settings" = "Settings"; "FuelBoard" = "FuelBoard"; -"Top" = "Top"; "Best value" = "Best value"; "Cheapest" = "Cheapest"; -"Closest station" = "Closest station"; +"Closest" = "Closest"; /* Stations tab */ "Closest %@ stations — closest first, lowest price within %lld %@ · %lld stations" = "Closest %@ stations — closest first, lowest price within %lld %@ · %lld stations"; @@ -44,11 +43,9 @@ "(approx.)" = "(approx.)"; "%@ more" = "%@ more"; "Station labels" = "Station labels"; -"Closest station" = "Closest station"; "Meets your minimum saving after travel costs." = "Meets your minimum saving after travel costs."; "The lowest fuel price per litre within your selected range." = "The lowest fuel price per litre within your selected range."; -"The nearest station by journey distance." = "The nearest station by journey distance."; -"The cheapest station in your Favourites for the selected fuel." = "The cheapest station in your Favourites for the selected fuel."; +"The closest station by journey distance." = "The closest station by journey distance."; "Star a station to save it to your Favourites." = "Star a station to save it to your Favourites."; "How does FuelBoard calculate value?" = "How does FuelBoard calculate value?"; /* Title of the explanation sheet the row above opens (a short form of it). */ diff --git a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsCalculationAuditTests.swift b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsCalculationAuditTests.swift index c23babe..6452d26 100644 --- a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsCalculationAuditTests.swift +++ b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsCalculationAuditTests.swift @@ -533,6 +533,6 @@ final class FuelSavingsCalculationAuditTests: XCTestCase { )?.result XCTAssertEqual(result?.status, .baseline) XCTAssertEqual(result?.netSavingGBP, 0) - XCTAssertEqual(result?.headline(), "Closest station") + XCTAssertEqual(result?.headline(), "Closest") } } diff --git a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsEstimateTests.swift b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsEstimateTests.swift index 0fa07be..3cec0a1 100644 --- a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsEstimateTests.swift +++ b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsEstimateTests.swift @@ -47,7 +47,7 @@ final class FuelSavingsEstimateTests: XCTestCase { XCTAssertEqual(result(.breakEven, .confirmed).listingVerdict().lines, ["Break even"]) XCTAssertEqual(result(.breakEven, .estimated).listingVerdict().lines, ["Break even", "(approx.)"]) - XCTAssertEqual(result(.baseline, .confirmed).listingVerdict().lines, ["Closest station"]) + XCTAssertEqual(result(.baseline, .confirmed).listingVerdict().lines, ["Closest"]) // The Key sheet's legend demonstrates the wording, never a figure. XCTAssertEqual( @@ -450,8 +450,8 @@ final class FuelSavingsEstimateTests: XCTestCase { ) XCTAssertEqual(availability?.result?.status, .baseline) - XCTAssertEqual(availability?.result?.headline(compact: true), "Closest station") - XCTAssertEqual(FuelSavingsStatus.baseline.displayName, "Closest station") + XCTAssertEqual(availability?.result?.headline(compact: true), "Closest") + XCTAssertEqual(FuelSavingsStatus.baseline.displayName, "Closest") XCTAssertEqual(availability?.result?.netSavingGBP, 0) XCTAssertEqual(availability?.result?.extraDistanceMiles, 0) } @@ -503,7 +503,7 @@ final class FuelSavingsEstimateTests: XCTestCase { /// The exact strings the brief asks for. func testHeadlineWordingMatchesTheBrief() { - XCTAssertEqual(FuelSavingsResult.baselineHeadline, "Closest station") + XCTAssertEqual(FuelSavingsResult.baselineHeadline, "Closest") XCTAssertEqual(FuelSavingsResult.breakEvenHeadline, "Break even") let saving = FuelSavingsResult(fillLitres: 34.9, @@ -594,7 +594,7 @@ final class FuelSavingsEstimateTests: XCTestCase { } /// The baseline capsule has to say what it IS: it previously repeated the - /// status word above it ("Closest station" twice) and never explained that + /// status word above it ("Closest" twice) and never explained that /// the savings are measured against it. func testCapsuleHeadlineMarksTheBaselineAndKeepsTheEstimateMarker() { let baseline = FuelSavingsResult( @@ -605,7 +605,7 @@ final class FuelSavingsEstimateTests: XCTestCase { netSavingGBP: 0, status: .baseline ) - XCTAssertEqual(baseline.capsuleHeadline(), "Closest station") + XCTAssertEqual(baseline.capsuleHeadline(), "Closest") let saving = FuelSavingsResult( fillLitres: 30, diff --git a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsTests.swift b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsTests.swift index 5c4a683..c45c327 100644 --- a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsTests.swift +++ b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsTests.swift @@ -209,20 +209,82 @@ final class StationRecommendationBadgeTests: XCTestCase { ) } - /// 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() { + /// A station with no saving, not the cheapest and not the closest states + /// nothing a badge could carry. + func testNoSavingNotCheapestAndNotClosestMeansNoBadge() { XCTAssertNil( StationRecommendationBadge.badge(netSavingGBP: 0.0, isCheapest: false, + isClosest: false, bestValueThresholdGBP: threshold) ) XCTAssertNil( StationRecommendationBadge.badge(netSavingGBP: -2.50, isCheapest: false, + isClosest: false, bestValueThresholdGBP: threshold) ) } + func testTheClosestStationWearsClosestWhenNothingElseApplies() { + XCTAssertEqual( + StationRecommendationBadge.badge(netSavingGBP: 0.0, isCheapest: false, + isClosest: true, + bestValueThresholdGBP: threshold), + .closest + ) + // Closest is a position, so it does not need a savings figure to hold. + XCTAssertEqual( + StationRecommendationBadge.badge(netSavingGBP: nil, isCheapest: false, + isClosest: true, + bestValueThresholdGBP: threshold), + .closest + ) + } + + /// The nearest forecourt is usually also the cheapest, so without the + /// ordering the Closest label would win by accident and hide the price news. + func testCheapestOutranksClosestWhenBothApply() { + XCTAssertEqual( + StationRecommendationBadge.badge(netSavingGBP: 0.42, isCheapest: true, + isClosest: true, + bestValueThresholdGBP: threshold), + .cheapest + ) + } + + func testBestValueOutranksBothCheapestAndClosest() { + XCTAssertEqual( + StationRecommendationBadge.badge(netSavingGBP: 3.15, isCheapest: true, + isClosest: true, + bestValueThresholdGBP: threshold), + .bestValue + ) + } + + /// The whole ladder in one place, so a future case cannot slide in between + /// two rungs unnoticed: Best Value, then Cheapest, then Closest, then none. + func testLabelPrecedenceIsTotalAcrossEveryCombination() { + let cases: [(saving: Double, cheapest: Bool, closest: Bool, + badge: StationRecommendationBadge?)] = [ + (3.15, true, true, .bestValue), + (3.15, false, true, .bestValue), + (3.15, false, false, .bestValue), + (0.42, true, true, .cheapest), + (0.42, true, false, .cheapest), + (0.42, false, true, .closest), + (0.42, false, false, nil) + ] + for c in cases { + XCTAssertEqual( + StationRecommendationBadge.badge( + netSavingGBP: c.saving, isCheapest: c.cheapest, isClosest: c.closest, + bestValueThresholdGBP: threshold + ), + c.badge, + "saving \(c.saving), cheapest \(c.cheapest), closest \(c.closest)" + ) + } + } + func testUnavailableSavingsLeaveOnlyTheCheapestBadge() { XCTAssertEqual( StationRecommendationBadge.badge(netSavingGBP: nil, isCheapest: true, @@ -288,7 +350,7 @@ final class KeySheetContentTests: XCTestCase { "\(label) must be sentence case, not all caps") } XCTAssertEqual(KeySheetContent.stationLabels.map(\.label), - ["Best value", "Cheapest", "Closest station", "Top"]) + ["Best value", "Cheapest", "Closest"]) } /// The Key dispatches its indicators on `StationLabelKind`, so every kind must @@ -306,19 +368,48 @@ final class KeySheetContentTests: XCTestCase { func testStationLabelKindsUseTheirRecommendedSymbols() { XCTAssertEqual(StationLabelKind.bestValue.symbol, "medal") XCTAssertEqual(StationLabelKind.cheapest.symbol, "tag") - XCTAssertEqual(StationLabelKind.closestStation.symbol, "location.north.fill") - XCTAssertEqual(StationLabelKind.top.symbol, "trophy") + XCTAssertEqual(StationLabelKind.closest.symbol, "location.north.fill") + // No trophy: "Top" is gone. One state, one label. + XCTAssertFalse(StationLabelKind.allCases.map(\.symbol).contains("trophy")) } func testStationLabelsDescribeTheCurrentQualificationAndRangeRules() { XCTAssertEqual(KeySheetContent.stationLabels, [ .init(label: "Best value", description: "Meets your minimum saving after travel costs."), .init(label: "Cheapest", description: "The lowest fuel price per litre within your selected range."), - .init(label: "Closest station", description: "The nearest station by journey distance."), - .init(label: "Top", description: "The cheapest station in your Favourites for the selected fuel.") + .init(label: "Closest", description: "The closest station by journey distance.") ]) } + /// "Top" is retired. It and "Cheapest" were two words for the same flag, and the + /// app drifted between them by device: the phone's rows said Cheapest while the + /// watch said Top for the identical state. Nothing may reintroduce the synonym. + func testTopLabelIsRetired() { + XCTAssertFalse(StationLabelKind.allCases.contains { $0.label == "Top" }) + XCTAssertFalse(KeySheetContent.stationLabels.contains { $0.label == "Top" }) + // The concept is not lost — it is called Cheapest, which is also the word + // every piece of favourites copy uses. + XCTAssertTrue(StationLabelKind.allCases.contains { $0.label == "Cheapest" }) + } + + /// The closest label is one word everywhere: the badge, the verdict capsule the + /// baseline station wears, and the Key all read it from `baselineHeadline`. + func testClosestLabelIsTheSameWordEverywhere() { + XCTAssertEqual(FuelSavingsResult.baselineHeadline, "Closest") + XCTAssertEqual(StationLabelKind.closest.label, "Closest") + let baseline = FuelSavingsResult( + fillLitres: 30, + grossSavingGBP: 0, + extraDrivingCostGBP: 0, + extraDistanceMiles: 0, + netSavingGBP: 0, + status: .baseline, + journeyConfidence: .confirmed + ) + XCTAssertEqual(baseline.headline(), "Closest") + XCTAssertEqual(baseline.headline(), StationLabelKind.closest.label) + } + func testFavouriteAndHelpCopyRemainPinned() { XCTAssertEqual( KeySheetContent.favourite, @@ -345,7 +436,7 @@ final class StationRankingTests: XCTestCase { func testNoQualificationProducesNoCapsules() { XCTAssertEqual( StationRanking.kinds(netSavingGBP: 0.40, isCheapestInRange: false, - isTopFavourite: false, bestValueThresholdGBP: threshold), + bestValueThresholdGBP: threshold), [] ) } @@ -354,35 +445,75 @@ final class StationRankingTests: XCTestCase { // Exactly at the threshold qualifies, matching StationRecommendationBadge. XCTAssertEqual( StationRanking.kinds(netSavingGBP: threshold, isCheapestInRange: false, - isTopFavourite: false, bestValueThresholdGBP: threshold), + bestValueThresholdGBP: threshold), [.bestValue] ) // A hair under does not. XCTAssertEqual( StationRanking.kinds(netSavingGBP: threshold - 0.01, isCheapestInRange: false, - isTopFavourite: false, bestValueThresholdGBP: threshold), + bestValueThresholdGBP: threshold), [] ) } - func testCheapestInRangeAndTopFavouriteAreDistinctLabels() { + /// Cheapest is the only ranking label besides Best value. Whichever tab opened + /// the sheet — Stations or Favourites — it means the lowest price among the + /// stations on that screen, so there is no second cheapest-flavoured capsule. + func testCheapestIsTheOnlyCheapestFlavouredLabel() { XCTAssertEqual( StationRanking.kinds(netSavingGBP: nil, isCheapestInRange: true, - isTopFavourite: false, bestValueThresholdGBP: threshold), + bestValueThresholdGBP: threshold), [.cheapest] ) XCTAssertEqual( StationRanking.kinds(netSavingGBP: nil, isCheapestInRange: false, - isTopFavourite: true, bestValueThresholdGBP: threshold), - [.top] + bestValueThresholdGBP: threshold), + [] ) } - func testCapsulesAreOrderedBestValueTopCheapest() { + func testCapsulesAreOrderedBestValueThenCheapest() { XCTAssertEqual( StationRanking.kinds(netSavingGBP: 9.99, isCheapestInRange: true, - isTopFavourite: true, bestValueThresholdGBP: threshold), - [.bestValue, .top, .cheapest] + bestValueThresholdGBP: threshold), + [.bestValue, .cheapest] + ) + } + + /// The sheet has room for every qualification, so a station holding all three + /// shows all three — in the Key's order, which is also the row's precedence, + /// so the sheet reads left-to-right the same way a row resolves top-to-bottom. + func testCapsulesAreOrderedBestValueCheapestClosest() { + XCTAssertEqual( + StationRanking.kinds(netSavingGBP: 9.99, isCheapestInRange: true, + isClosestInRange: true, bestValueThresholdGBP: threshold), + [.bestValue, .cheapest, .closest] + ) + } + + /// The closest station with no saving and no price advantage still earns its + /// label on the sheet — the one case a listing row cannot show, because there + /// Best Value and Cheapest are both unavailable and Closest wins the slot. + func testClosestAloneEarnsOneCapsule() { + XCTAssertEqual( + StationRanking.kinds(netSavingGBP: nil, isCheapestInRange: false, + isClosestInRange: true, bestValueThresholdGBP: threshold), + [.closest] + ) + XCTAssertEqual( + StationRanking.kinds(netSavingGBP: 0.0, isCheapestInRange: false, + isClosestInRange: true, bestValueThresholdGBP: threshold), + [.closest] + ) + } + + /// Closest must not sneak in when the host did not flag it — the sheet never + /// invents a rule of its own. + func testNoClosestFlagMeansNoClosestCapsule() { + XCTAssertEqual( + StationRanking.kinds(netSavingGBP: nil, isCheapestInRange: true, + isClosestInRange: false, bestValueThresholdGBP: threshold), + [.cheapest] ) } @@ -396,7 +527,7 @@ final class StationRankingTests: XCTestCase { ) == .bestValue let sheetSaysBestValue = StationRanking.kinds( netSavingGBP: netSaving, isCheapestInRange: false, - isTopFavourite: false, bestValueThresholdGBP: threshold + bestValueThresholdGBP: threshold ).contains(.bestValue) XCTAssertEqual(rowSaysBestValue, sheetSaysBestValue, "disagreed on net saving \(netSaving)") } diff --git a/FuelBoardWatch Watch App/ContentView.swift b/FuelBoardWatch Watch App/ContentView.swift index 952a83b..9406097 100644 --- a/FuelBoardWatch Watch App/ContentView.swift +++ b/FuelBoardWatch Watch App/ContentView.swift @@ -516,9 +516,11 @@ struct ContentView: View { .fontWeight(favourite.isCheapest ? .semibold : .regular) .lineLimit(1) if favourite.isCheapest { - // Sentence case, to match every other surface: the watch - // row is the same label as the phone's, not a different one. - Text("Top") + // The same WORD as every other surface, not just the same + // capitalisation. The phone's rows call this state + // "Cheapest"; a watch that said "Top" made one state look + // like two different achievements. + Text("Cheapest") .font(.system(size: 9, weight: .semibold)) .padding(.horizontal, 4) .padding(.vertical, 2) diff --git a/Shared/FuelSavings.swift b/Shared/FuelSavings.swift index 0e89203..15f4b88 100644 --- a/Shared/FuelSavings.swift +++ b/Shared/FuelSavings.swift @@ -45,7 +45,7 @@ enum FuelSavingsStatus: String, Codable, Equatable { case breakEven case notWorthTheDrive /// The comparison baseline itself — the station every other saving is - /// measured against. Shown as "Closest station", never "Break even". + /// measured against. Shown as "Closest", never "Break even". case baseline var displayName: String { @@ -187,8 +187,13 @@ enum FuelSavingsFormat { } extension FuelSavingsResult { - /// Baseline row label — required wording. - static let baselineHeadline = "Closest station" + /// The baseline's capsule label. + /// + /// "Closest", not "Closest station": it is a station LABEL sitting alongside + /// "Best value" and "Cheapest", and the noun is implied by the row it is on. + /// This is also the ONE source for the word, so the closest station's badge and + /// its verdict capsule cannot word it differently. + static let baselineHeadline = "Closest" /// Neutral result label — required wording. static let breakEvenHeadline = "Break even" @@ -209,7 +214,7 @@ extension FuelSavingsResult { } /// Row headline, e.g. "£1.22 better off after travel", "£0.31 worse off after - /// travel", "Break even", "Closest station". + /// travel", "Break even", "Closest". /// /// Estimated comparisons are prefixed "Approx. " for positive/negative money /// results — the WORD, never a "~": a tilde reads as decoration (and assistive @@ -271,7 +276,7 @@ extension FuelSavingsResult { /// never straddle two lines, and a taller verdict grows the capsule downward /// instead of widening the row's fixed price column. struct ListingVerdict: Equatable { - /// "Saves", "Costs", "Break even" or "Closest station". + /// "Saves", "Costs", "Break even" or "Closest". let headline: String /// "£3.10" / "£1.20 more" — nil when there is no figure to show. let amount: String? @@ -393,11 +398,19 @@ struct FuelSavingsRankedStation: Equatable { Item(label: "Costs", description: "Filling up here costs more than at your closest station, including travel.") ] + /// The station labels, in the order the Key lists them. + /// + /// There is no "Top" entry. Top and Cheapest were two words for one state — + /// both driven by the same "cheapest among the stations you are looking at" + /// flag, differing only in whether the candidate set was the radius or the + /// Favourites — and which word a surface used drifted by device. Cheapest is + /// the survivor: it says what it measures, and it is already the word every + /// piece of favourites copy uses ("your cheapest E10 favourite", "Cheapest + /// favourite" alert). The concept survives; the synonym does not. static let stationLabels = [ Item(label: "Best value", description: "Meets your minimum saving after travel costs."), Item(label: "Cheapest", description: "The lowest fuel price per litre within your selected range."), - Item(label: "Closest station", description: "The nearest station by journey distance."), - Item(label: "Top", description: "The cheapest station in your Favourites for the selected fuel.") + Item(label: "Closest", description: "The closest station by journey distance.") ] static let favourite = Item( @@ -427,16 +440,16 @@ struct FuelSavingsRankedStation: Equatable { enum StationLabelKind: String, CaseIterable { case bestValue case cheapest - case closestStation - case top + case closest /// Matches the corresponding `KeySheetContent.stationLabels` entry. var label: String { switch self { case .bestValue: return String(localized: "Best value") case .cheapest: return String(localized: "Cheapest") - case .closestStation: return FuelSavingsResult.baselineHeadline - case .top: return String(localized: "Top") + // "Closest", from the one shared constant, so the badge, the verdict + // capsule and the Key cannot word the same label three ways. + case .closest: return FuelSavingsResult.baselineHeadline } } @@ -444,8 +457,7 @@ struct FuelSavingsRankedStation: Equatable { switch self { case .bestValue: return "medal" case .cheapest: return "tag" - case .closestStation: return "location.north.fill" - case .top: return "trophy" + case .closest: return "location.north.fill" } } } @@ -456,39 +468,50 @@ struct FuelSavingsRankedStation: Equatable { /// 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. +/// `cheapestStationID`). 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 + case closest + /// One label per row, in this order: Best Value, then Cheapest, then Closest. + /// + /// A station can hold all three at once — the nearest forecourt is often also + /// the cheapest — and a row has room for one, so the order is decided here + /// rather than left to each caller. Being nearby is the weakest claim: a short + /// drive says nothing about the price, and unlike the other two it reports no + /// saving at all. Closest therefore surfaces only when the row earns neither + /// of the others. static func badge(netSavingGBP: Double?, isCheapest: Bool, + isClosest: Bool = false, bestValueThresholdGBP: Double) -> StationRecommendationBadge? { if let netSavingGBP, netSavingGBP.isFinite, bestValueThresholdGBP.isFinite, netSavingGBP >= bestValueThresholdGBP { return .bestValue } - return isCheapest ? .cheapest : nil + if isCheapest { return .cheapest } + return isClosest ? .closest : nil } } /// Every ranking label a station qualifies for, in display order. /// -/// A listing row shows ONE badge (Best Value outranks Cheapest, because a row has -/// room for one). The More info sheet has room for all of them, so it shows each -/// qualification side by side — but it must never invent a rule of its own, so this -/// defers to `StationRecommendationBadge.badge` for the Best Value gate and takes -/// the other two as the SAME flags the host list gives its rows. +/// A listing row shows ONE badge (Best Value outranks Cheapest outranks Closest, +/// because a row has room for one). The More info sheet has room for all of them, +/// so it shows each qualification side by side — but it must never invent a rule +/// of its own, so this defers to `StationRecommendationBadge.badge` for the Best +/// Value gate and takes Cheapest and Closest as the SAME flags the host list +/// gives its rows. /// -/// Top and Cheapest are distinct on purpose: Cheapest means "lowest price in the -/// range the Stations tab is showing", Top means "lowest price among your -/// Favourites". A caller passes whichever context it is listing, never both, so a -/// station is not labelled twice for the same achievement. +/// There is no Top label any more: both tabs mean the same thing by "cheapest" — +/// the lowest price among the stations on screen, whether that set is the radius or +/// the Favourites — so a host passes one flag and the sheet renders one capsule. enum StationRanking { static func kinds(netSavingGBP: Double?, isCheapestInRange: Bool, - isTopFavourite: Bool, + isClosestInRange: Bool = false, bestValueThresholdGBP: Double) -> [StationLabelKind] { var kinds: [StationLabelKind] = [] // The same gate the listing row uses, asked without a cheapest flag so the @@ -496,12 +519,13 @@ enum StationRanking { if StationRecommendationBadge.badge( netSavingGBP: netSavingGBP, isCheapest: false, + isClosest: false, bestValueThresholdGBP: bestValueThresholdGBP ) == .bestValue { kinds.append(.bestValue) } - if isTopFavourite { kinds.append(.top) } if isCheapestInRange { kinds.append(.cheapest) } + if isClosestInRange { kinds.append(.closest) } return kinds } }