From cf3abb3fa71c15310f7005377ce1f1e4e09d24b7 Mon Sep 17 00:00:00 2001 From: FuelBoard Contributor Date: Sun, 11 Oct 2026 17:41:53 +0100 Subject: [PATCH] Fuel Savings: MPG and fill-up are sliders, not typed fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two Fuel Savings figures and the in-list fill-up control were text fields. A typed figure could sit outside the slider's own bounds, so the control and the number it claimed to show could disagree, and the decimal pad needed a Done button to escape. - Settings: "Average fuel economy" and "Typical fill-up" are now sliders with a read-out ("45 MPG", "£40" / "40 L") in place of the field. The warning copy reads "Set a realistic..." rather than "Enter...". - FuelSavingsAdjustmentView (shared by the Stations Worth-the-Drive header and the Favourites tab) drops its text field and gains the same read-out; the typed-value machinery and the -focusFillAmount QA hook go with it. - Slider bounds live in FuelSavingsCalculator (fillAmountSliderRange/Step, mpgSliderRange/Step) so the Settings control and the in-list control can never offer different ranges, and the fill-up slider range stays inside the calculator's validity ceiling (asserted by test). - Two tests pin those invariants: every slider position is a valid amount, and both MPG bounds are valid MPG values. 155 tests, 0 failures. - Comments that described the removed fields were updated; the keyboard Done-bar plumbing is kept (no numeric field remains in the app, so it can no longer fire) rather than deleted, so any field added later still has an escape route. Verified on the simulator: Settings shows "Average fuel economy / 45 MPG" with no field, and the Worth-the-Drive fill-up control shows the unit picker with a "£40" read-out and a slider. The Settings "Typical fill-up" row sits below the fold on every simulator available here (and synthetic input cannot scroll it), so those pixels are unverified. --- FuelBoard/FuelSavingsViews.swift | 69 ++++++------------- FuelBoard/SettingsView.swift | 50 ++++++++++---- FuelBoard/StationsView.swift | 7 +- .../FuelSavingsEstimateTests.swift | 31 +++++++++ Shared/FuelSavings.swift | 24 +++++++ 5 files changed, 116 insertions(+), 65 deletions(-) diff --git a/FuelBoard/FuelSavingsViews.swift b/FuelBoard/FuelSavingsViews.swift index a6713e2..947e10a 100644 --- a/FuelBoard/FuelSavingsViews.swift +++ b/FuelBoard/FuelSavingsViews.swift @@ -4,14 +4,23 @@ struct FuelSavingsAdjustmentView: View { @Binding var amount: FuelSavingsFillAmount let referencePricePencePerLitre: Double? - @State private var textValue: String = "" - /// Focus for the numeric field — also lets the DEBUG QA hook below raise the - /// keyboard on launch so the Done bar can be screenshotted (a script cannot - /// tap the simulator). - @FocusState private var isAmountFieldFocused: Bool - private var range: ClosedRange { - amount.unit == .pounds ? 10...100 : 10...80 + FuelSavingsCalculator.fillAmountSliderRange(for: amount.unit) + } + + private var step: Double { + FuelSavingsCalculator.fillAmountSliderStep(for: amount.unit) + } + + /// Read-out of the figure the slider is on. Deliberately NOT a text field: + /// a typed figure could sit outside the slider's own bounds, leaving the + /// control and the value it claimed to show disagreeing. Slider-only means + /// what you see is the figure the estimate actually uses. + private var valueText: String { + switch amount.unit { + case .pounds: return String(format: "£%.0f", amount.value) + case .litres: return String(format: "%.0f L", amount.value) + } } var body: some View { @@ -25,33 +34,17 @@ struct FuelSavingsAdjustmentView: View { } .pickerStyle(.segmented) HStack { - Slider(value: sliderBinding, in: range, step: amount.unit == .pounds ? 5 : 1) - TextField(amount.unit == .pounds ? "£" : "Litres", text: valueBinding) - .keyboardType(.decimalPad) - .multilineTextAlignment(.trailing) - .frame(width: 72) - .textFieldStyle(.roundedBorder) - .focused($isAmountFieldFocused) - .accessibilityIdentifier("fillAmountField") + Slider(value: sliderBinding, in: range, step: step) + .accessibilityLabel("Fill-up amount") + .accessibilityValue(valueText) + Text(valueText) + .monospacedDigit() + .frame(width: 72, alignment: .trailing) } Text("Changes the estimate for this session only — your Settings default stays unchanged.") .font(.caption2) .foregroundStyle(.secondary) } - .onAppear { - syncText() -#if DEBUG - // QA hook: raise the keyboard without a tap so the Done bar can be - // screenshotted on a simulator (synthetic taps do not reach it). - if ProcessInfo.processInfo.arguments.contains("-focusFillAmount") { - DispatchQueue.main.asyncAfter(deadline: .now() + 1.0) { - isAmountFieldFocused = true - } - } -#endif - } - .onChange(of: amount.value) { _, _ in syncText() } - .onChange(of: amount.unit) { _, _ in syncText() } } private var unitBinding: Binding { @@ -65,7 +58,6 @@ struct FuelSavingsAdjustmentView: View { } else { amount = FuelSavingsFillAmount(value: newUnit == .pounds ? 40 : 30, unit: newUnit) } - syncText() } ) } @@ -76,23 +68,6 @@ struct FuelSavingsAdjustmentView: View { set: { amount.value = min(max($0, range.lowerBound), range.upperBound) } ) } - - private var valueBinding: Binding { - Binding( - get: { textValue }, - set: { raw in - textValue = raw - if let value = Double(raw), value > 0 { - amount.value = min(value, FuelSavingsCalculator.maximumFillAmount(for: amount.unit)) - } - } - ) - } - - private func syncText() { - let value = amount.value - textValue = value.rounded() == value ? String(format: "%.0f", value) : String(format: "%.1f", value) - } } struct FuelSavingsSummaryView: View { diff --git a/FuelBoard/SettingsView.swift b/FuelBoard/SettingsView.swift index e7f010b..54f3661 100644 --- a/FuelBoard/SettingsView.swift +++ b/FuelBoard/SettingsView.swift @@ -71,9 +71,10 @@ struct SettingsView: View { @State private var mediumWidgetDiag: String? @State private var installedWidgets: String = "" - /// Which numeric field in the Fuel Savings section is being edited. Those - /// fields use `.decimalPad`, which has no Return key, so the keyboard's - /// Done button clears this to dismiss AND commit the typed value. + /// Which numeric field on this screen is being edited. `.decimalPad` has no + /// Return key, so the keyboard's Done button clears this to dismiss AND + /// commit the typed value. Both Fuel Savings controls are sliders now, so + /// nothing sets it — it stays for any numeric field this screen takes again. @FocusState private var focusedSavingsField: SavingsField? private enum SavingsField: Hashable { @@ -153,14 +154,15 @@ struct SettingsView: View { HStack { Text("Average fuel economy") Spacer() - TextField("45", value: fuelSavingsMPGBinding, format: .number.precision(.fractionLength(0...1))) - .keyboardType(.decimalPad) - .multilineTextAlignment(.trailing) - .frame(width: 80) - .focused($focusedSavingsField, equals: .averageMPG) - Text("MPG") + Text(mpgText) + .monospacedDigit() .foregroundStyle(.secondary) } + Slider(value: fuelSavingsMPGBinding, + in: FuelSavingsCalculator.mpgSliderRange, + step: FuelSavingsCalculator.mpgSliderStep) + .accessibilityLabel("Average fuel economy") + .accessibilityValue(mpgText) Picker("Fill-up unit", selection: fuelSavingsUnitBinding) { ForEach(FuelSavingsFillUnit.allCases) { unit in Text(unit.displayName).tag(unit) @@ -170,14 +172,17 @@ struct SettingsView: View { HStack { Text("Typical fill-up") Spacer() - TextField("40", value: fuelSavingsAmountBinding, format: .number.precision(.fractionLength(0...1))) - .keyboardType(.decimalPad) - .multilineTextAlignment(.trailing) - .frame(width: 90) - .focused($focusedSavingsField, equals: .fillAmount) + Text(fillUpText) + .monospacedDigit() + .foregroundStyle(.secondary) } + Slider(value: fuelSavingsAmountBinding, + in: FuelSavingsCalculator.fillAmountSliderRange(for: fuelSavingsSettings.preferredAmount.unit), + step: FuelSavingsCalculator.fillAmountSliderStep(for: fuelSavingsSettings.preferredAmount.unit)) + .accessibilityLabel("Typical fill-up") + .accessibilityValue(fillUpText) if !fuelSavingsSettings.isConfigured && fuelSavingsSettings.enabled { - Text("Enter a realistic UK MPG and fill-up amount before Worth the Drive appears in sorting.") + Text("Set a realistic UK MPG and fill-up amount before Worth the Drive appears in sorting.") .font(.caption) .foregroundStyle(.orange) } @@ -655,6 +660,21 @@ struct SettingsView: View { ) } + /// "45 MPG" / "45.5 MPG". The slider is the only control, so this read-out + /// is the one place the figure is stated. + private var mpgText: String { + let mpg = fuelSavingsSettings.mpg + return mpg.rounded() == mpg ? String(format: "%.0f MPG", mpg) : String(format: "%.1f MPG", mpg) + } + + /// "£40" / "40 L" — the fill-up slider's read-out. + private var fillUpText: String { + switch fuelSavingsSettings.preferredAmount.unit { + case .pounds: return String(format: "£%.0f", fuelSavingsSettings.preferredAmount.value) + case .litres: return String(format: "%.0f L", fuelSavingsSettings.preferredAmount.value) + } + } + private func persistFuelSavingsSettings() { FuelStore.saveFuelSavingsSettings(fuelSavingsSettings) } diff --git a/FuelBoard/StationsView.swift b/FuelBoard/StationsView.swift index feb29a0..5e09da3 100644 --- a/FuelBoard/StationsView.swift +++ b/FuelBoard/StationsView.swift @@ -202,9 +202,10 @@ struct StationsView: View { // large title at rest, scrolls it away and fades the compact // inline title in. No custom toolbar needed — the native mechanism // only renders text, which is exactly what we want here. - // The Worth the Drive fill-up box uses `.decimalPad`, which has no - // Return key — give the keyboard a Done button and let a scroll - // dismiss it too. + // No numeric field is left on this screen — the Worth the Drive + // fill-up control is a slider — so no keyboard should appear here. + // The Done bar and interactive dismissal stay as the escape hatch + // for any field that returns. .keyboardDoneButton() .scrollDismissesKeyboard(.interactively) .navigationTitle("FuelBoard") diff --git a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsEstimateTests.swift b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsEstimateTests.swift index 8800c00..07512f1 100644 --- a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsEstimateTests.swift +++ b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsEstimateTests.swift @@ -432,4 +432,35 @@ final class FuelSavingsEstimateTests: XCTestCase { ) XCTAssertEqual(worse.capsuleHeadline(), "£0.31 worse off after travel") } + + // MARK: - Slider bounds (Settings and the in-list control share these) + + /// Both fill-up controls are slider-only now, so every position the slider + /// can reach must be a usable amount — otherwise the control could sit on a + /// figure the calculator rejects. + func testFillAmountSliderBoundsAreAlwaysValidAmounts() { + for unit in FuelSavingsFillUnit.allCases { + let range = FuelSavingsCalculator.fillAmountSliderRange(for: unit) + let step = FuelSavingsCalculator.fillAmountSliderStep(for: unit) + XCTAssertLessThan(range.lowerBound, range.upperBound, "\(unit) range must be usable") + XCTAssertGreaterThan(step, 0, "\(unit) step must advance") + XCTAssertGreaterThan(range.lowerBound, 0, "\(unit) must never offer a zero fill") + XCTAssertLessThanOrEqual(range.upperBound, + FuelSavingsCalculator.maximumFillAmount(for: unit), + "\(unit) slider must stay inside the valid ceiling") + for position in [range.lowerBound, range.upperBound, (range.lowerBound + range.upperBound) / 2] { + XCTAssertTrue(FuelSavingsFillAmount(value: position, unit: unit).isValid, + "\(position) \(unit) must be a valid amount") + } + } + } + + func testMPGSliderBoundsAreInsideTheValidMPGRange() { + let range = FuelSavingsCalculator.mpgSliderRange + XCTAssertLessThan(range.lowerBound, range.upperBound) + XCTAssertGreaterThan(FuelSavingsCalculator.mpgSliderStep, 0) + XCTAssertTrue(FuelSavingsCalculator.isValidMPG(range.lowerBound)) + XCTAssertTrue(FuelSavingsCalculator.isValidMPG(range.upperBound)) + XCTAssertTrue(FuelSavingsCalculator.isValidMPG(45), "a typical UK figure must be reachable") + } } diff --git a/Shared/FuelSavings.swift b/Shared/FuelSavings.swift index 63bff78..d0332ca 100644 --- a/Shared/FuelSavings.swift +++ b/Shared/FuelSavings.swift @@ -224,6 +224,30 @@ enum FuelSavingsCalculator { } } + /// Bounds for every fill-up SLIDER (Settings and the in-list adjustment + /// share them, so the two controls can never offer different ranges). The + /// validity ceiling above is deliberately wider, so a stored outlier is + /// displayed rather than destroyed by the control that shows it. + static func fillAmountSliderRange(for unit: FuelSavingsFillUnit) -> ClosedRange { + switch unit { + case .pounds: return 10...100 + case .litres: return 10...80 + } + } + + static func fillAmountSliderStep(for unit: FuelSavingsFillUnit) -> Double { + switch unit { + case .pounds: return 5 + case .litres: return 1 + } + } + + /// Bounds for the average-MPG slider. The validity bounds (5...200) are + /// wide enough for any stored figure but far too wide to fine-tune by + /// touch; every real-world UK car sits inside this range. + static let mpgSliderRange: ClosedRange = 10...100 + static let mpgSliderStep: Double = 1 + static func isValidMPG(_ mpg: Double) -> Bool { mpg.isFinite && mpg >= minimumMPG && mpg <= maximumMPG }