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 } }