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 }