diff --git a/ios/HouseScan/UI/Camera/CameraOverlays.swift b/ios/HouseScan/UI/Camera/CameraOverlays.swift index 8fb4f7615..ad1840899 100644 --- a/ios/HouseScan/UI/Camera/CameraOverlays.swift +++ b/ios/HouseScan/UI/Camera/CameraOverlays.swift @@ -4,17 +4,17 @@ import SwiftUI struct CameraOverlays: View { let state: ScanViewState var highlight: GapRequest? - /// The camera area the chrome leaves open (`CameraChrome.openArea`); the ring's legend - /// keeps inside it. - var openArea: OpenCameraArea? = nil - /// Set to the ring's legend while it doesn't fit beside the ring, for the screen to draw - /// under its card (`CameraChrome.legend`). Without it, the legend shows only beside the ring. + /// Set to the aim ring's legend while it shows, for the screen to draw under its card + /// (`CameraChrome.legend`). Nil on screens with no legend. var cardLegend: Binding? = nil /// The step the aim ring's legend first showed with. The legend explains the first ring that /// fills and retires once that step ends (#81); it isn't needed on every ring after. @State private var legendStep: GuidanceStep? = nil @State private var legendRetired = false + /// Whether a filling ring is on screen (`WayfindingOverlay.onFillingRingShown`): the legend + /// explains the ring, so it shows only with it. + @State private var fillingRingShown = false /// The aim step on screen, with its target, so the update that ends it can still score it. @State private var lastAim: AimSnapshot? = nil /// The target of an aim step that just completed, held on screen as a full green ring with a @@ -49,6 +49,11 @@ struct CameraOverlays: View { do { try await Task.sleep(for: Self.completedHold) } catch { return } completedTarget = nil } + .onChange(of: shownLegend, initial: true) { _, line in + if let cardLegend, cardLegend.wrappedValue != line { cardLegend.wrappedValue = line } + // Counted as shown only once drawn, so it retires with the step it showed on. + if line != nil, cardLegend != nil, legendStep == nil { legendStep = state.guidance } + } } @ViewBuilder @@ -67,20 +72,14 @@ struct CameraOverlays: View { ) if state.marking == nil { // While marking, the reticle is the only aim; the path and ring would compete. - let progress = state.aimProgress - let held = heldTarget - let line = held == nil ? legend(progress: progress) : nil WayfindingOverlay( projection: projection, wall: wall, path: state.path, target: state.target, - progress: progress, - completed: held, - legend: line, - legendShort: ScanCopy.aimRingLegendShort, - openArea: openArea, - onLegendPlaced: { placement in legendPlaced(placement, line) } + progress: state.aimProgress, + completed: heldTarget, + onFillingRingShown: { shown in fillingRingShown = shown } ) .transition(.opacity) } @@ -106,20 +105,16 @@ struct CameraOverlays: View { return target } - /// Where the legend went. Beside the ring it is drawn there; where no wording fits there, - /// it goes under the card. It counts as shown, and so retires with its step, only where it - /// drew: a legend with no room on a short screen or at a large text size once used up the - /// one explanation without being seen. - private func legendPlaced(_ placement: WayfindingOverlay.LegendPlacement?, _ legend: String?) { - let underCard = placement == .underCard ? legend : nil - if let cardLegend, cardLegend.wrappedValue != underCard { cardLegend.wrappedValue = underCard } - let drawn = placement == .besideRing || (placement == .underCard && cardLegend != nil) - if drawn, legendStep == nil { legendStep = state.guidance } - } - - /// The legend, while the ring fills for the first time. - private func legend(progress: Double?) -> String? { - guard !legendRetired, let progress, progress < 1 else { return nil } + /// The legend, while the first filling ring is on screen and short of full (#81). + /// + /// It goes under the instruction card, in the chrome's own stack, rather than beside the + /// ring: beside it, it went behind the card at accessibility text sizes (the card grows + /// and is drawn over the camera layers), and any layout that dropped it where there was no + /// room left the people with the largest text without it. In the stack it grows with the + /// card and scrolls with it at any size. + private var shownLegend: String? { + guard fillingRingShown, !legendRetired, state.marking == nil, heldTarget == nil, + let progress = state.aimProgress, progress < 1 else { return nil } return ScanCopy.aimRingLegend } } diff --git a/ios/HouseScan/UI/Camera/WayfindingOverlay.swift b/ios/HouseScan/UI/Camera/WayfindingOverlay.swift index 1a57ee642..129093187 100644 --- a/ios/HouseScan/UI/Camera/WayfindingOverlay.swift +++ b/ios/HouseScan/UI/Camera/WayfindingOverlay.swift @@ -5,7 +5,8 @@ import SwiftUI /// stand, and a ring on the point to aim at (or an edge arrow when it is off screen). /// /// Each part sets its own accessibility: the path and arrow are hidden, while the filling ring -/// and the legend are read (#81). A new part is read by VoiceOver unless it hides itself. +/// is read (#81). A new part is read by VoiceOver unless it hides itself. The ring's legend is +/// drawn under the instruction card (`CameraChrome.legend`), not here. struct WayfindingOverlay: View { var projection: CameraProjection var wall: WallGeometry? @@ -17,33 +18,13 @@ struct WayfindingOverlay: View { /// An aim step's target that was just completed, drawn as a full ring in place of `target` /// while it is on screen (`CameraOverlays` holds it for a moment). var completed: SIMD3? = nil - /// One line saying what the ring is for, drawn beside it while it is on screen. - var legend: String? = nil - /// A shorter legend, drawn where `legend` doesn't fit between the ring and the card or the - /// controls. - var legendShort: String? = nil - /// The camera area the chrome leaves open, in global coordinates (`CameraChrome.openArea`). - /// The legend keeps inside it. Nil: the fixed lane the arrows use. - var openArea: OpenCameraArea? = nil - /// Called with where the legend went each time that changes: beside the ring, or nowhere - /// here because no wording fits (the caller draws it under the card instead). Nil once it - /// is gone. - var onLegendPlaced: ((LegendPlacement?) -> Void)? = nil - - enum LegendPlacement: Equatable { - case besideRing - /// No wording fits beside the ring: at the largest text sizes, where the card and the - /// controls fill most of the screen, and on short screens. - case underCard - } - - /// The side of the ring the legend keeps to once shown: true for below. - @State private var legendBelow: Bool? = nil + /// Called with whether a filling ring is on screen, each time that changes (`CameraOverlays` + /// shows the ring's legend under the card only while it is). + var onFillingRingShown: ((Bool) -> Void)? = nil var body: some View { GeometryReader { proxy in let size = proxy.size - let top = proxy.frame(in: .global).minY ZStack { Canvas { context, size in drawPath(in: &context, size: size) @@ -53,8 +34,11 @@ struct WayfindingOverlay: View { // from its last fill to green. if let shown = marker(in: size) { TargetMarker(placement: shown.placement, progress: shown.progress) - if let legend, case .onScreen(let point, let radius) = shown.placement { - legendView(legend, beside: point, radius: radius, in: size, top: top) + if progress != nil, completed == nil, case .onScreen = shown.placement { + Color.clear + .frame(width: 0, height: 0) + .onAppear { onFillingRingShown?(true) } + .onDisappear { onFillingRingShown?(false) } } } } @@ -67,92 +51,10 @@ struct WayfindingOverlay: View { // MARK: Lane /// The band clear of the instruction card above and the buttons and map below, which are - /// drawn over this layer, at the default text size. The off-screen arrows keep to it, and - /// the legend does where the chrome's open area isn't measured. + /// drawn over this layer. The off-screen arrows keep to it. private static let laneTop: CGFloat = 260 private static let laneBottomInset: CGFloat = 300 - // MARK: Legend - - /// How much more room the other side of the ring needs before the legend moves there. The - /// two sides are equal near the middle of the screen, where the ring is held, and pose - /// jitter there would otherwise flip the legend across the ring every few frames. - private static let legendSwitchMargin: CGFloat = 40 - - /// The gap between the ring and the legend, and between the legend and the card or the - /// controls. - private static let legendGap: CGFloat = 12 - - /// The legend goes above or below the ring, whichever has more room between the card and - /// the controls, and moves with the ring without covering it. Once shown, it changes sides - /// only when the other side has `legendSwitchMargin` more room. - /// - /// The room is measured from the card's real bottom and the controls' real top - /// (`openArea`), which move with Dynamic Type. Both are drawn over this layer, and against - /// a fixed lane the legend went under the enlarged card. The longest wording that fits on - /// its side is drawn. Where neither fits, nothing is drawn here and `onLegendPlaced` says - /// so, and the caller puts the legend under the card, which scrolls at any text size. - private func legendView( - _ text: String, beside point: CGPoint, radius: CGFloat, in size: CGSize, top: CGFloat - ) -> some View { - let width = max(120, min(300, size.width - 48)) - let x = min(max(point.x, 24 + width / 2), size.width - 24 - width / 2) - // The open area in this layer's coordinates. - let laneTop = openArea.map { $0.top - top + Self.legendGap } ?? Self.laneTop - let laneBottom = openArea.map { $0.bottom - top - Self.legendGap } ?? size.height - Self.laneBottomInset - let roomAbove = point.y - radius - laneTop - let roomBelow = laneBottom - (point.y + radius) - let below: Bool - if let kept = legendBelow { - below = kept - ? roomAbove - roomBelow < Self.legendSwitchMargin - : roomBelow - roomAbove >= Self.legendSwitchMargin - } else { - below = roomBelow >= roomAbove - } - // The room on the legend's side, pinned at the edge nearer the ring, so the text's height - // (it wraps, and grows with Dynamic Type) never moves its near edge onto the ring. - let reach = max(0, (below ? roomBelow : roomAbove) - Self.legendGap) - // Each choice reports itself when it is the one drawn, so the legend counts as shown - // only where it drew. - return ViewThatFits(in: .vertical) { - legendText(text) - .onAppear { onLegendPlaced?(.besideRing) } - if let legendShort { - legendText(legendShort) - .onAppear { onLegendPlaced?(.besideRing) } - } - Color.clear.frame(width: 0, height: 0) - .onAppear { onLegendPlaced?(.underCard) } - } - .onAppear { legendBelow = below } - .onChange(of: below) { _, side in legendBelow = side } - .onDisappear { - legendBelow = nil - onLegendPlaced?(nil) - } - .frame(width: width, height: reach, alignment: below ? .top : .bottom) - .position( - x: x, - y: below ? point.y + radius + Self.legendGap + reach / 2 : point.y - radius - Self.legendGap - reach / 2 - ) - .transition(.opacity) - } - - private func legendText(_ text: String) -> some View { - Text(text) - .font(.subheadline.weight(.semibold)) - .foregroundStyle(Palette.chalk) - .multilineTextAlignment(.center) - .padding(.horizontal, 14) - .padding(.vertical, 8) - // The instruction card's solid scrim. A see-through black let the camera image show - // through and failed the accessibility audit's contrast check over a bright wall. - .background(ScrimShape.rounded(14)) - .fixedSize(horizontal: false, vertical: true) - .accessibilityIdentifier("aim.legend") - } - // MARK: Path /// Dot spacing on the ground. A display choice: dense enough to read as a line at 3 m, diff --git a/ios/HouseScan/UI/Components/CameraChrome.swift b/ios/HouseScan/UI/Components/CameraChrome.swift index 181cd5858..003cbb99a 100644 --- a/ios/HouseScan/UI/Components/CameraChrome.swift +++ b/ios/HouseScan/UI/Components/CameraChrome.swift @@ -15,14 +15,10 @@ struct CameraChrome: View { /// Taps on the open camera area, in the camera view's coordinates (full screen, which is /// the window's global space). Nil when the screen has nothing to tap. var onCameraTap: ((CGPoint) -> Void)? - /// A line under the card about something drawn on the camera: the aim ring's legend where - /// it doesn't fit beside the ring (`WayfindingOverlay.LegendPlacement.underCard`). It is - /// part of the chrome, so it scrolls with the card at the largest text sizes instead of - /// being covered or dropped. + /// A line under the card about something drawn on the camera: the aim ring's legend + /// (`CameraOverlays`). It is part of the chrome's stack, so it grows with the card and + /// scrolls with it at the largest text sizes, where the card covers most of the camera. var legend: String? = nil - /// Set to the camera area left open between the card (with `legend`) and `bottom`, for - /// overlays that draw beside a point on the camera and must keep clear of both. - var openArea: Binding? = nil @ViewBuilder var bottom: Bottom var body: some View { @@ -59,24 +55,23 @@ struct CameraChrome: View { } } .frame(minHeight: 36) - VStack(spacing: 8) { - InstructionCard(instruction: instruction, tone: tone, reply: reply, eyebrow: eyebrow) - if let legend { - legendLine(legend) - } + InstructionCard(instruction: instruction, tone: tone, reply: reply, eyebrow: eyebrow) + if let legend { + legendLine(legend) } - .background { edgeReader { area, frame in area.top = frame.maxY } } Spacer(minLength: 0) bottom - .background { edgeReader { area, frame in area.bottom = frame.minY } } } + .animation(.easeOut(duration: 0.2), value: legend) .padding(.horizontal, Metrics.edge) .padding(.top, 4) .padding(.bottom, 8) } + /// One Text with the symbol inline, as the result's sample badge is: the audit reported a + /// Label with fixedSize as partially supporting Dynamic Type. private func legendLine(_ text: String) -> some View { - Label(text, systemImage: "scope") + Text("\(Image(systemName: "scope")) \(text)") .font(Typeface.hint) .foregroundStyle(Palette.chalk) .fixedSize(horizontal: false, vertical: true) @@ -84,39 +79,10 @@ struct CameraChrome: View { .padding(.vertical, 10) .frame(maxWidth: .infinity, alignment: .leading) .background(ScrimShape.rounded(14)) - .accessibilityElement(children: .combine) + .accessibilityLabel(text) .accessibilityIdentifier("aim.legend") .transition(.opacity) } - - /// Writes one edge of the open area from the frame of the view it is the background of, in - /// global coordinates (the camera layers fill the screen, as `CameraSizeReader` notes). - @ViewBuilder - private func edgeReader(_ write: @escaping (inout OpenCameraArea, CGRect) -> Void) -> some View { - if let openArea { - GeometryReader { proxy in - Color.clear - .onAppear { update(openArea, write, proxy.frame(in: .global)) } - .onChange(of: proxy.frame(in: .global)) { _, frame in update(openArea, write, frame) } - } - .accessibilityHidden(true) - } - } - - private func update( - _ binding: Binding, _ write: (inout OpenCameraArea, CGRect) -> Void, _ frame: CGRect - ) { - var area = binding.wrappedValue ?? OpenCameraArea(top: 0, bottom: .greatestFiniteMagnitude) - write(&area, frame) - if area != binding.wrappedValue { binding.wrappedValue = area } - } -} - -/// The camera area between the chrome's card and its controls, in global coordinates. The -/// top is below the bottom when the chrome fills the screen (the largest text sizes). -struct OpenCameraArea: Equatable { - var top: CGFloat - var bottom: CGFloat } /// Reads the full-screen camera view size so buttons can send "the reticle" (nil point) diff --git a/ios/HouseScan/UI/Copy/ScanCopy.swift b/ios/HouseScan/UI/Copy/ScanCopy.swift index de5312482..e4608211d 100644 --- a/ios/HouseScan/UI/Copy/ScanCopy.swift +++ b/ios/HouseScan/UI/Copy/ScanCopy.swift @@ -179,16 +179,14 @@ enum ScanCopy { // MARK: Aim ring - /// The line beside the walk's first aim ring, which fills as its stretch is captured (#81). + /// The line under the card while the walk's first aim ring is on screen; the ring fills as + /// its stretch is captured (#81). /// A stretch counts only once it is seen from two places a step apart /// (`CoverageConfig.coveringBaseline`), so holding still never fills the ring: the line asks /// for a step, in the tilt-down card's own words ("Take a small step sideways as you look."), /// so the two lines on screen together ask for the same thing. If the card's wording /// changes, change this with it. static let aimRingLegend = "Keep the ring in view and take a small step sideways. It fills as your phone captures this spot." - /// The legend where the whole line doesn't fit beside the ring (large text sizes, short - /// screens). Under the card, the whole line is drawn instead. - static let aimRingLegendShort = "It fills as your phone captures this spot." // MARK: Wall ends diff --git a/ios/HouseScan/UI/Screens/WallWalkScreen.swift b/ios/HouseScan/UI/Screens/WallWalkScreen.swift index 48e5f2752..75bd950d4 100644 --- a/ios/HouseScan/UI/Screens/WallWalkScreen.swift +++ b/ios/HouseScan/UI/Screens/WallWalkScreen.swift @@ -14,9 +14,7 @@ struct WallWalkScreen: View { @State private var cameraSize: CGSize = .zero @State private var trayOpen = false @State private var taps: [TapRipple.Ripple] = [] - /// The camera area between the card and the controls, for the aim ring's legend. - @State private var openArea: OpenCameraArea? = nil - /// The aim ring's legend while it doesn't fit beside the ring: drawn under the card (#81). + /// The aim ring's legend while it shows, drawn under the card (#81). @State private var cardLegend: String? = nil @Environment(\.accessibilityReduceMotion) private var reduceMotion @Environment(\.dynamicTypeSize) private var typeSize @@ -24,7 +22,7 @@ struct WallWalkScreen: View { var body: some View { ZStack { CameraSizeReader(size: $cameraSize) - CameraOverlays(state: state, highlight: nil, openArea: openArea, cardLegend: $cardLegend) + CameraOverlays(state: state, highlight: nil, cardLegend: $cardLegend) if state.coaching == .relocalizing, let meterPhoto { // "Point at the meter like this.": the saved close-up shows what to aim at. SavedMeterPhoto(image: meterPhoto) @@ -50,8 +48,7 @@ struct WallWalkScreen: View { taps.append(.init(point: point)) actions.markFeaturePoint(at: point, viewSize: cameraSize) }, - legend: cardLegend, - openArea: $openArea + legend: cardLegend ) { VStack(spacing: 10) { controls diff --git a/ios/HouseScanUITests/ScreenStatesUITests.swift b/ios/HouseScanUITests/ScreenStatesUITests.swift index c464055cb..da59032e2 100644 --- a/ios/HouseScanUITests/ScreenStatesUITests.swift +++ b/ios/HouseScanUITests/ScreenStatesUITests.swift @@ -456,8 +456,9 @@ final class ScreenStatesUITests: XCTestCase { XCTAssertTrue(element(app, "screen.gapRequest").waitForExistence(timeout: 10), "the second tap must send anyway") } - /// #81: the first aim ring comes with a line saying what it is for, and reads its progress to - /// VoiceOver. Off screen, the edge arrow stands in for it, and neither shows. + /// #81: the first aim ring comes with a line under the card saying what it is for, clear of + /// the card at every text size, and reads its progress to VoiceOver. Off screen, the edge + /// arrow stands in for the ring, and neither shows. @MainActor func testAimRingShowsProgressAndItsLegend() throws { continueAfterFailure = false @@ -475,8 +476,8 @@ final class ScreenStatesUITests: XCTestCase { XCTAssertFalse(legend.frame.intersects(card.frame), "the legend must keep clear of the card: \(legend.frame) vs \(card.frame)") app.terminate() - // At the largest text size the card and the controls fill the screen, so there is no - // room beside the ring: the legend moves under the card instead of going behind it or + // At the largest text size the card fills most of the screen. The legend sits under it in + // the same stack, so it grows and scrolls with the card instead of going behind it or // disappearing. app.launchArguments = ["-uiDemo", "-uiDemoFreeze", "-uiDemoPhase", "wallWalk", "-uiDemoAim"] + Self.largestText app.launch() @@ -613,14 +614,30 @@ final class ScreenStatesUITests: XCTestCase { /// appeared can still be moving into place (the walk's controls settle after the close-up), /// and a tap there misses without an error (the button flow at 577acc4 never opened the mark /// tray). + /// + /// A control below the bottom edge of a scrolling screen (the review's "Add something" chips, + /// the result's Details) is never hittable where it is. `tap()` scrolls to it and the wait + /// doesn't, so once it has had a moment to settle it is tapped where it is. @MainActor private func tap(_ app: XCUIApplication, _ identifier: String, timeout: TimeInterval = 20) { let target = element(app, identifier) - let hittable = XCTNSPredicateExpectation(predicate: NSPredicate(format: "isHittable == true"), object: target) - XCTAssertEqual(XCTWaiter().wait(for: [hittable], timeout: timeout), .completed, "missing or not tappable: \(identifier)") + let deadline = Date().addingTimeInterval(timeout) + XCTAssertTrue(target.waitForExistence(timeout: timeout), "missing \(identifier)") + let settled = waitUntilHittable(target, timeout: min(3, max(0, deadline.timeIntervalSinceNow))) + if settled || target.frame.maxY > app.windows.firstMatch.frame.maxY { + target.tap() + return + } + XCTAssertTrue(waitUntilHittable(target, timeout: max(1, deadline.timeIntervalSinceNow)), "missing or not tappable: \(identifier)") target.tap() } + @MainActor + private func waitUntilHittable(_ target: XCUIElement, timeout: TimeInterval) -> Bool { + let hittable = XCTNSPredicateExpectation(predicate: NSPredicate(format: "isHittable == true"), object: target) + return XCTWaiter().wait(for: [hittable], timeout: timeout) == .completed + } + /// The card's reply with these words. @MainActor private func reply(_ app: XCUIApplication, _ title: String) -> XCUIElement {