diff --git a/ios/HouseScan/UI/Components/InstructionCard.swift b/ios/HouseScan/UI/Components/InstructionCard.swift index ab5bf95de..2ed64a4a3 100644 --- a/ios/HouseScan/UI/Components/InstructionCard.swift +++ b/ios/HouseScan/UI/Components/InstructionCard.swift @@ -2,12 +2,12 @@ import SwiftUI /// The single instruction at the top of a camera screen. /// -/// When the text changes, the words swap at once and the card eases to its new height. A -/// blur or fade between them left both lines half-transparent for a moment, which the +/// When the text changes, the words and the reply land at once and the scrim eases to its new +/// height. A blur or fade between them left both lines half-transparent for a moment, which the /// accessibility audit reported as low contrast and, for the outgoing line, as clipped text -/// (the marking prompt, the end question). Coaching -/// takes the same slot with an amber icon so a problem replaces the instruction instead of -/// stacking on top of it. +/// (the marking prompt, the end question). When the task changes, the reply under the words +/// swaps with them. Coaching shows in the same slot, marked with an amber icon; the screen +/// decides whether its words stand in for the task's or go under them. struct InstructionCard: View { enum Tone: Equatable { case normal @@ -22,6 +22,10 @@ struct InstructionCard: View { var identifier: String var hint: String var perform: () -> Void + /// The task the reply answers, when that isn't the card's words: the task under a + /// coaching line, or the walk without its distance to go. The reply swaps with the words + /// and takes no taps for `replyLock` when this changes. Nil: the card's instruction. + var task: Instruction? = nil } var instruction: Instruction @@ -31,7 +35,18 @@ struct InstructionCard: View { /// finish"). Read with the instruction as one VoiceOver element. var eyebrow: String? + /// How long a new card's reply ignores taps, so a tap meant for the card that just left + /// can't answer the next one: in the 4.1 field test, quick taps on "Can't get there" each + /// answered a different card (#82). A guess, not measured: the card's own change + /// (`Motion.text`, 0.2 s) plus a tap's reaction time. + static let replyLock: Duration = .milliseconds(600) + @Environment(\.accessibilityReduceMotion) private var reduceMotion + /// The task whose reply has been on screen for `replyLock`. Compared with the current task + /// in the same update that changes it, so a new reply is locked from its first frame. + @State private var unlockedTask: Instruction? + + private var replyTask: Instruction { reply?.task ?? instruction } var body: some View { VStack(alignment: .trailing, spacing: 0) { @@ -47,6 +62,9 @@ struct InstructionCard: View { .background(Palette.replyFill, in: .capsule) .contentShape(.capsule) .buttonStyle(PressableStyle()) + // The colours are set above and PressableStyle doesn't read isEnabled, so the + // pill looks the same while locked; VoiceOver and UI tests read it as dimmed. + .disabled(unlockedTask != replyTask) .accessibilityHint(reply.hint) .accessibilityIdentifier(reply.identifier) .padding([.horizontal, .bottom], 12) @@ -62,10 +80,29 @@ struct InstructionCard: View { .transition(.identity) } } + // New words land at once with the reply at its final place; only the scrim, outside + // this stack, eases to the new height. When the stack's layout eased too, the reply slid + // from its old place over the new words for `Motion.text` (#64): on a coaching change, + // which swaps the words and keeps the reply, and comes and goes often while walking. + .transaction(value: instruction) { $0.animation = nil } + // A new task swaps the words and the reply together, as one new view. With the reply + // outside the swap it faded out over the new words (#64). The words alone still swap on + // their own (a coaching line, the distance to go), so the reply keeps a press that is + // under way. + .id(replyTask) + .transition(.identity) .frame(maxWidth: .infinity) .background(ScrimShape.rounded()) .animation(reduceMotion ? .easeOut(duration: 0.15) : Motion.text, value: instruction) .animation(Motion.text, value: reply == nil) + .task(id: replyTask) { + // Cleared first, so a task that comes back within the lock (A, B, A) is locked again. + let shown = replyTask + unlockedTask = nil + // A new id cancels this wait and starts its own. + do { try await Task.sleep(for: Self.replyLock) } catch { return } + unlockedTask = shown + } } private var message: some View { diff --git a/ios/HouseScan/UI/Copy/ScanCopy.swift b/ios/HouseScan/UI/Copy/ScanCopy.swift index c0684ca70..39b71fe9a 100644 --- a/ios/HouseScan/UI/Copy/ScanCopy.swift +++ b/ios/HouseScan/UI/Copy/ScanCopy.swift @@ -284,6 +284,30 @@ enum ScanCopy { /// The reply on the see-behind step: the homeowner can't get a view past the obstruction. static let cannotSeeBehind = "Can't see past it" + // MARK: Card replies + + /// The walk card's reply (`ScanActions.cannotAccessArea`) and its VoiceOver hint, worded for + /// what it does on `step`; nil on a step that offers none. On an aim or tilt step the + /// homeowner is already at the spot and it's the view that can't be had, so "Can't get + /// there" read as the wrong answer and testers kept tilting (#63). It stays on the steps + /// that ask to go somewhere. The wall's end (`markEnd`) has "Wall ends here" instead. + static func reply(for step: GuidanceStep) -> (title: String, hint: String)? { + switch step { + case .aimAtGround, .aimAtWall: + (title: "Skip this spot", hint: "An installer will look at it instead.") + case .tiltUp: + (title: "Skip this", hint: "Skips the view above this part of the wall. An installer will look at it instead.") + case .walk: + (title: "Can't get there", hint: "Ends the wall at the dashed line on the map. An installer will look at what's past it.") + case .markNextWall: + (title: "Can't get there", hint: "Skips this part of the wall. An installer will look at it instead.") + case .seeBehind: + (title: cannotSeeBehind, hint: "Skips the part behind it. An installer will look at it instead.") + case .findMeter, .aimAtWallForMeter, .holdOnMeter, .markEnd, .stepBack, .walkComplete, .gap: + nil + } + } + // MARK: Follow-up view /// The check came back asking for views the camera can take now; the scan goes straight diff --git a/ios/HouseScan/UI/Preview/DemoEngine.swift b/ios/HouseScan/UI/Preview/DemoEngine.swift index 80fd8bfd5..f16f4f97a 100644 --- a/ios/HouseScan/UI/Preview/DemoEngine.swift +++ b/ios/HouseScan/UI/Preview/DemoEngine.swift @@ -29,7 +29,10 @@ final class DemoEngine: ScanActions { /// How far the walk has seen to each side of the meter, meters. private var reachedLeft: Float = 0.3 private var reachedRight: Float = 0.3 - private var skippedSpan: ClosedRange? + /// Stretches the homeowner skipped, and in which bands, kept apart so a later skip doesn't + /// undo an earlier one. An aim step's skip covers only the band it asked for, as in the real + /// engine; skipping the rest of a walk covers both. + private var skippedSpans: [(span: ClosedRange, bands: Set)] = [] /// `-uiDemoEndPreview`: the homeowner walked back 1.5 m, so ending the wall now leaves part /// of the walk out. private var walkedBack: Float? @@ -146,6 +149,13 @@ final class DemoEngine: ScanActions { state.path = DemoScene.path(toward: 2.4, out: 1.8) } } + if arguments.contains("-uiDemoAim"), state.phase == .wallWalk { + // The walk asks for the ground about 2 ft right of the meter, which it hasn't seen + // from two places; the card's reply says "Skip this spot" there. + state.guidance = .aimAtGround(s: 0.6) + state.target = DemoScene.wall.world(s: 0.6, height: 0, out: 0.5) + state.path = [] + } if arguments.contains("-uiDemoCorner") { // The walk followed an outside corner right of the meter, between the battery spot and // the window, and went on 1.6 m along the next wall: that wall runs away from the @@ -573,11 +583,12 @@ final class DemoEngine: ScanActions { wallCells.append(Self.state(at: center, left: reachedLeft, right: reachedRight, lag: 0)) groundCells.append(Self.state(at: center, left: reachedLeft, right: reachedRight, lag: 0.35)) } - if let skippedSpan { + for skipped in skippedSpans { for index in 0..`: the gap screen shows that server request. /// - `-uiDemoSample`: no server is configured, so the upload screen says the result is a sample. diff --git a/ios/HouseScan/UI/Screens/GapRequestScreen.swift b/ios/HouseScan/UI/Screens/GapRequestScreen.swift index eaf00f2e4..e9edb32c6 100644 --- a/ios/HouseScan/UI/Screens/GapRequestScreen.swift +++ b/ios/HouseScan/UI/Screens/GapRequestScreen.swift @@ -33,7 +33,11 @@ struct GapRequestScreen: View { hint: followUps > 0 ? "Skips this view and shows your result. An installer will look at this part instead." : "Skips this view. An installer will look at this part instead.", - perform: { actions.skipGap() } + perform: { actions.skipGap() }, + // The request, not the coaching that can show over it: coaching coming and going + // doesn't lock the reply again (InstructionCard.replyLock). Before the request + // is known, the step's own words, for the same reason. + task: state.gap.map { ScanCopy.gap($0) } ?? ScanCopy.guidance(.gap) ), eyebrow: followUps > 0 && state.gap?.isSatisfied != true ? ScanCopy.followUp(remaining: followUps) : nil, photoCount: state.captureCount, diff --git a/ios/HouseScan/UI/Screens/WallWalkScreen.swift b/ios/HouseScan/UI/Screens/WallWalkScreen.swift index aec821cb0..ae4e9692b 100644 --- a/ios/HouseScan/UI/Screens/WallWalkScreen.swift +++ b/ios/HouseScan/UI/Screens/WallWalkScreen.swift @@ -121,14 +121,6 @@ struct WallWalkScreen: View { return .walking } - /// True while the guidance points at a particular stretch the homeowner might not reach. - private var asksForArea: Bool { - switch state.guidance { - case .walk, .aimAtGround, .aimAtWall, .tiltUp, .markNextWall, .seeBehind: true - default: false - } - } - /// The end preview shows only with the buttons that set it: not over a question, a mark or /// the feature tray. private var showsEndPreview: Bool { @@ -170,7 +162,7 @@ struct WallWalkScreen: View { } .transition(.opacity) case .endQuestion: - // One question, two equal full-width answers that say what they mean (checklist I4). + // One question, three equal full-width answers that say what they mean (checklist I4). VStack(spacing: 8) { Button { actions.answerWallEnd(turnsCorner: true) @@ -189,6 +181,18 @@ struct WallWalkScreen: View { .buttonStyle(.secondaryProminent) .accessibilityHint("A fence, gate, or your neighbor's yard") .accessibilityIdentifier("action.endBlocked") + // A garden wall or a fence can just stop, with no corner and nothing in the way, + // and neither answer above fits it (#70). Ends the wall as "Something blocks it" + // does: the usable wall stops here. + Button { + actions.answerWallEnd(turnsCorner: false) + } label: { + Label("The wall just ends", systemImage: "arrow.right.to.line") + .frame(maxWidth: .infinity) + } + .buttonStyle(.secondaryProminent) + .accessibilityHint("No corner and nothing in the way. The wall stops here.") + .accessibilityIdentifier("action.endEnds") } .transition(.opacity) case .overheadQuestion: @@ -262,11 +266,12 @@ struct WallWalkScreen: View { } /// The capture gate's coaching (slow down, texture, light, hold steady) comes and goes within - /// a second while the homeowner walks, and the task under it stays the same, so "Can't get - /// there" stays put through it: hiding it each time made the button fade in and out under the + /// a second while the homeowner walks, and the task under it stays the same, so the card's + /// reply stays put through it: hiding it each time made the button fade in and out under the /// homeowner's thumb, and the accessibility audit caught it half faded (CI run 36295565916). /// Coaching about tracking itself hides it, since where an end would land needs the phone's - /// place, as does being past an end. + /// place, as does being past an end, where the way on is to walk back (#80). No `default`: + /// a new coaching case has to choose. private var coachingHidesReply: Bool { switch state.coaching { case nil, .slowDown?, .needsTexture?, .tooDark?, .holdSteady?: false @@ -274,33 +279,29 @@ struct WallWalkScreen: View { } } + /// The card's reply, worded for the step by `ScanCopy.reply(for:)` ("Skip this spot" on an + /// aim step, "Can't get there" on the walk). Its identifier is `action.cannotAccess` on + /// every step. private var reply: InstructionCard.Reply? { - guard asksForArea, state.marking == nil, state.endQuestion == nil, !state.overheadQuestion, - !coachingHidesReply, !trayOpen else { return nil } - if case .seeBehind = state.guidance { - return InstructionCard.Reply( - title: ScanCopy.cannotSeeBehind, - identifier: "action.cannotAccess", - hint: "Skips the part behind it. An installer will look at it instead.", - perform: { actions.cannotAccessArea() } - ) - } - if case .walk = state.guidance { - return InstructionCard.Reply( - title: "Can't get there", - identifier: "action.cannotAccess", - hint: "Ends the wall at the dashed line on the map. An installer will look at what's past it.", - perform: { actions.cannotAccessArea() } - ) - } + guard state.marking == nil, state.endQuestion == nil, !state.overheadQuestion, !coachingHidesReply, !trayOpen, + let copy = ScanCopy.reply(for: state.guidance) else { return nil } return InstructionCard.Reply( - title: "Can't get there", + title: copy.title, identifier: "action.cannotAccess", - hint: "Skips this part of the wall. An installer will look at it instead.", - perform: { actions.cannotAccessArea() } + hint: copy.hint, + perform: { actions.cannotAccessArea() }, + task: replyTask ) } + /// The step the reply answers, in words, for the card's input lock: the step's own words + /// while coaching shows over it, and the walk without its distance to go, which changes as + /// the homeowner walks and would lock "Can't get there" again each time. + private var replyTask: Instruction { + if case .walk(let side, _) = state.guidance { return ScanCopy.guidance(.walk(side: side, remaining: nil)) } + return ScanCopy.guidance(state.guidance) + } + private var nextWallSymbol: String { if case .markNextWall(.left, _) = state.guidance { return "arrow.turn.up.left" } return "arrow.turn.up.right" diff --git a/ios/HouseScanUITests/ScreenStatesUITests.swift b/ios/HouseScanUITests/ScreenStatesUITests.swift index 65a6b8f40..8a5c1b5c6 100644 --- a/ios/HouseScanUITests/ScreenStatesUITests.swift +++ b/ios/HouseScanUITests/ScreenStatesUITests.swift @@ -27,6 +27,7 @@ final class ScreenStatesUITests: XCTestCase { ("wallWalk-nextWall", ["-uiDemoPhase", "wallWalk", "-uiDemoNextWall"], "wallWalk"), ("wallWalk-nextWallRefused", ["-uiDemoPhase", "wallWalk", "-uiDemoNextWall", "-uiDemoRefusal"], "wallWalk"), ("wallWalk-tiltUp", ["-uiDemoPhase", "wallWalk", "-uiDemoTiltUp"], "wallWalk"), + ("wallWalk-aim", ["-uiDemoPhase", "wallWalk", "-uiDemoAim"], "wallWalk"), ("wallWalk-overheadQuestion", ["-uiDemoPhase", "wallWalk", "-uiDemoOverheadQuestion"], "wallWalk"), ("wallWalk-hidden", ["-uiDemoPhase", "wallWalk", "-uiDemoHidden"], "wallWalk"), ("wallWalk-seeBehind", ["-uiDemoPhase", "wallWalk", "-uiDemoSeeBehind"], "wallWalk"), @@ -77,6 +78,8 @@ final class ScreenStatesUITests: XCTestCase { "wallWalk-hidden", "wallWalk-seeBehind", "wallWalk-fullLegend", "gapRequest-followUp", "uploading-followUp", "markFeatures-lostPlace", "spotConfirm", "spotConfirm-which", "spotConfirm-ground", "spotConfirm-answered", "spotConfirm-cantMark", "spotConfirm-unconfirmable", + // The card's reply under the aim step's words and under coaching. + "wallWalk-aim", "wallWalk-slowDown", ] /// Words a state must show: in the named element's label or value, or with no identifier, @@ -216,6 +219,75 @@ final class ScreenStatesUITests: XCTestCase { XCTAssertTrue(element(app, "screen.onboarding").waitForExistence(timeout: 10)) } + /// The card's reply says what it does on each step (#63): "Skip this spot" where the phone is + /// already at the spot, "Can't get there" where the walk asks to go somewhere. It stays under + /// the capture gate's coaching (#80) and goes while the phone has lost its place or is past + /// the end of the wall. + @MainActor + func testCardReplyFollowsTheStep() throws { + let steps: [(name: String, arguments: [String], reply: String?)] = [ + ("walk", [], "Can't get there"), + ("aim", ["-uiDemoAim"], "Skip this spot"), + ("tiltUp", ["-uiDemoTiltUp"], "Skip this"), + ("seeBehind", ["-uiDemoSeeBehind"], "Can't see past it"), + ("slowDown", ["-uiDemoCoaching", "slowDown"], "Can't get there"), + ("tooDark", ["-uiDemoCoaching", "tooDark"], "Can't get there"), + ("relocalizing", ["-uiDemoCoaching", "relocalizing"], nil), + ("pastWallEnd", ["-uiDemoCoaching", "pastWallEnd"], nil), + ] + for step in steps { + let app = XCUIApplication() + app.launchArguments = ["-uiDemo", "-uiDemoFreeze", "-uiDemoPhase", "wallWalk"] + step.arguments + app.launch() + defer { app.terminate() } + guard element(app, "screen.wallWalk").waitForExistence(timeout: 15) else { + XCTFail("\(step.name): screen.wallWalk never appeared") + continue + } + let shown = element(app, "action.cannotAccess") + if let expected = step.reply { + XCTAssertTrue(shown.waitForExistence(timeout: 5), "\(step.name): the card has no reply") + // One read (`ElementRead`), as elsewhere in this file. + XCTAssertEqual(ElementRead.snapshot(shown)?.label, expected, "\(step.name): wrong reply") + } else { + XCTAssertFalse(shown.waitForExistence(timeout: 2), "\(step.name): the reply must not show") + } + } + } + + /// Each reply answers its own card: "Skip this spot" on the aim card leads to the walk, whose + /// "Can't get there" ends the wall there. A new card's reply takes taps only after a moment + /// (#82), so each tap waits for it. The end question's third answer ends the wall (#70), and + /// the walk goes on to the other side. + @MainActor + func testRepliesAnswerTheirOwnCardAndTheWallCanJustEnd() throws { + continueAfterFailure = false + let app = XCUIApplication() + app.launchArguments = ["-uiDemo", "-uiDemoFreeze", "-uiDemoPhase", "wallWalk", "-uiDemoAim"] + app.launch() + XCTAssertTrue(element(app, "screen.wallWalk").waitForExistence(timeout: 15)) + let tape = element(app, "wallTape") + XCTAssertTrue(tape.waitForExistence(timeout: 5)) + let wallBefore = Self.rightOfMeterSeen(ElementRead.snapshot(tape)?.value as? String) + XCTAssertNotNil(wallBefore, "the map must say how much of the wall right of the meter is seen") + tapReply(app, "Skip this spot") + // Skipping the ground spot sends that ground to review and leaves the wall above it as it + // was, as in the real engine: the map shows a skipped section, and the wall seen is unchanged. + let afterSkip = ElementRead.waitForValue(of: tape, timeout: 5) { $0.contains("skipped") } + XCTAssertNotNil(afterSkip, "the skipped ground must show on the map") + XCTAssertEqual(Self.rightOfMeterSeen(afterSkip), wallBefore, "skipping the ground must leave the wall band alone, got \(afterSkip ?? "nil")") + tapReply(app, "Can't get there") + tap(app, "action.markEnd", timeout: 10) + XCTAssertTrue(element(app, "action.endCorner").waitForExistence(timeout: 5), "the end question must ask") + XCTAssertTrue(element(app, "action.endBlocked").exists) + tap(app, "action.endEnds", timeout: 5) + XCTAssertTrue(element(app, "action.endEnds").waitForNonExistence(timeout: 5), "the answer must close the question") + let walkLeft = app.descendants(matching: .any) + .matching(NSPredicate(format: "identifier == 'instruction' AND label CONTAINS 'to your left'")).firstMatch + XCTAssertTrue(walkLeft.waitForExistence(timeout: 5), "the walk must go on to the left once the right end is answered") + XCTAssertTrue(reply(app, "Can't get there").waitForExistence(timeout: 5), "the walk's reply must come back") + } + /// B-09: "Add something" on the review opens the camera with the marking prompt, and the /// review comes back with the new item once it is marked, or unchanged after Cancel. @MainActor @@ -410,4 +482,27 @@ final class ScreenStatesUITests: XCTestCase { _ = XCTWaiter().wait(for: [hittable], timeout: 3) target.tap() } + + /// "Right of your meter: N percent seen" from the wall map's value: the wall band only. + private static func rightOfMeterSeen(_ value: String?) -> String? { + guard let value, let range = value.range(of: "Right of your meter: [0-9]+ percent seen", options: .regularExpression) else { return nil } + return String(value[range]) + } + + /// The card's reply with these words. + @MainActor + private func reply(_ app: XCUIApplication, _ title: String) -> XCUIElement { + app.buttons.matching(NSPredicate(format: "identifier == 'action.cannotAccess' AND label == %@", title)).firstMatch + } + + /// Taps the card's reply once it takes taps: a new card's reply ignores them for a moment + /// (`InstructionCard.replyLock`), and a tap then does nothing. + @MainActor + private func tapReply(_ app: XCUIApplication, _ title: String) { + let target = reply(app, title) + XCTAssertTrue(target.waitForExistence(timeout: 10), "missing the reply \"\(title)\"") + let ready = XCTNSPredicateExpectation(predicate: NSPredicate(format: "isEnabled == true AND isHittable == true"), object: target) + XCTAssertEqual(XCTWaiter().wait(for: [ready], timeout: 5), .completed, "the reply \"\(title)\" never took taps") + target.tap() + } }