Repository navigation
Conversation
… stays in view UNVALIDATED: not compiled; no package, app or UI tests have run. CI run 37165788075 on 15cf658 (iPhone 17) showed the card covering the circle in the middle of the camera at AX5: on the walk's "Is this the right end of the wall?" (`wallWalk-markEnd-AX5-stacked`) and on a past_end request's offer and refusal (`gapRequest-pastEnd-AX5`, `-refused-AX5`). "Wall ends here" was reachable, but the circle it marks at was hidden. Folding alone didn't fix it: in the folded walk frame a three-line title, Details and the card's reply still ended at 490 pt, past the circle's top at 409 pt (874 pt screen, 56 pt circle at its middle). So, on those two steps at the accessibility sizes, using each screen's existing `aims` and `Instruction.folded`, with no change to CameraChrome or InstructionCard: - The card leads with two lines: "Aim at the right end"; "Keep walking right" on a past_end request; and a refusal's correction: "Aim at the wall", "Turn to the right end", "Wait a moment", "Too short to use". Every word of the unfolded card is under Details (`ScanCopy.endAimFold`). - The reply ("The wall keeps going", "I can't get there") moves from the card to the actions under "Wall ends here". A past_end request's "One more view to finish" leads the words under Details. - Refusals fold too: before, they kept every word and covered the circle. - The default sizes are unchanged. The circle is an accessibility element ("Aiming circle", `aim.circle`), at the call site rather than in `Reticle`. UI tests read its real frame and check that it is the middle of the window, where `circleEnd`'s ray goes; that it is wholly on screen; and that it is clear of the card and every action wherever "Wall ends here" can be pressed. They cover default and AX5 on the walk's end and the past_end offer and refusal, a left past_end request (`-uiDemoGap pastEndLeft`), Details opening every word and closing back to an open circle, the decline's new place, and the end question keeping its three answers in order with no circle. The demo's past_end request now carries its answer and eyebrow, as the real one does. Not tested: Reduce Motion (no hook can force it in a UI test; the moved decline joins the end controls' existing animation, which is off under Reduce Motion), VoiceOver speech, physical aiming, and screens other than the iPhone 17 at AX5. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ly keeps its lock UNVALIDATED: not compiled; no package, app or UI tests have run. Fixes from the prep-for-prod review of 21ac61e: - Coaching riding along kept its note under the folded lead, and the dark note runs several lines at AX5. By the frames' measurements, even one line uses the room left above the circle. Folded around the circle, the coaching now leads in a few words beside its symbol ("It's dark here", `ScanCopy.coachingLead`), and its whole note opens Details (`ScanCopy.foldedAroundCircle`). With a refusal, the correction keeps the lead and the coaching opens Details. - The reply moved to the actions lost the card's 600 ms input lock (#82). `MovedReplyButton` keeps it, keyed to the same task as the card's reply. - Under Reduce Motion the walk's controls eased their swap over 0.15 s, which moved the map as the end step's added reply grew the stack. They now swap at once. - The test helper measured the card's end by its words and Details with 8 pt, but unfolded the card ends at its reply plus 12 pt. It now takes whether the card is folded, and it also checks the circle is the 56 pt reticle. - "Turn to the right end" would likely break to three lines beside the refusal triangle; it is now "Face the right end". Tests added: coached folded cards on both screens (`-uiDemoCoaching tooDark`), and the other-side refusal on both sides (`-uiDemoEndMarkOtherSide`, new in the demo). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
SamGu-NRX
added
the
full-ui
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (9)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughEnd-mark flows now show an accessible aiming circle and fold instructions around it at accessibility text sizes. The changes add action-led copy, move recovery replies into the action row in folded states, and extend demo scenarios and UI tests. ChangesEnd-mark aiming and recovery cards
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
actor User
participant WallWalkScreen
participant ScanCopy
participant EndAimCircle
participant MovedReplyButton
User->>WallWalkScreen: Enters an end-mark step
WallWalkScreen->>ScanCopy: Builds folded instruction from step, coaching, and refusal state
ScanCopy-->>WallWalkScreen: Returns folded instruction
WallWalkScreen->>EndAimCircle: Renders the offered reticle with accessibility identity
WallWalkScreen->>MovedReplyButton: Adds the guidance reply in the action row when folded
Suggested reviewers: Merge Risk: ⚪ Minimal · up to This UI-only change adjusts end-mark guidance layout at accessibility text sizes. No concrete merge-blocking issue was found, though the hosted UI test run should confirm the layout. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Note Review skipped Codemetrics is installed on this repository, but it hasn't been added to your team yet. To enable AI code reviews for this repo, go to your dashboard and add BaseScanning to your team's repositories. |
3 of 4 tasks
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
… step for a too-short wall UNVALIDATED: not compiled; not run. Greptile on #214: folded around the circle, darkness coaching led with "It's dark here" and left the remedy under Details, though darkness can last a whole walk. The coaching leads are now what to do ("Try your flashlight", "Try in daylight"); the moon symbol already says what is wrong, and the whole note still opens Details. The walk can ask for the end again after too-close ends were cleared, with `wallTooShort` still set. Folded, its card kept the full title, "The ends were too close. Walk along the wall first.", several lines at AX5 beside the triangle, which would cover the circle again. It now leads with "Walk the wall first", and the rest goes under Details. Coaching then opens Details too, as with a refusal. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ver can't stop on it under the question CI run 37171342665 on 254a456: the circle's geometry held in every new test. Each one measured it 56x56, at the window's exact centre, and clear of the card and the actions: walk and past-end, default and AX5, the left side, coached and other-side refusals. But testMarkingTheEndAsksWhatIsThere(AndSettlesTheRequest|AtLargestTextSize) failed on "the question shows no circle". The recording shows the circle faded out under the question, yet its `aim.circle` element was still in the tree: `.accessibilityHidden` on the element `.accessibilityElement(children: .ignore)` made didn't remove it. So VoiceOver could stop on an "Aiming circle" that isn't on screen. The circle is now inserted and removed with an opacity transition instead of kept at zero opacity. The fade is unchanged: Motion.settle with "Wall ends here", and a brief ease under Reduce Motion. The test's assertion is unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Owner
Author
|
@codex review |
Greptile on #214: since b1dc252 the folded lead is "Try your flashlight", and the tests asserted both that the card shows it and that the card has no "flashlight". They now check that the note's own words, "Try your phone's flashlight", are under Details, not on the card. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
4 tasks done
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
At accessibility text sizes, wall-end instructions could cover the centred aiming circle while still offering "Wall ends here". This follow-up fixes the ordinary walk and past-end request together.
The card uses a short actionable lead, with its complete instructions under Details. Its decline reply moves below "Wall ends here" and retains the existing accidental-tap lock. Coaching and refusal text use the same limited space without shrinking Dynamic Type. End questions keep their full choices.
This PR targets
fix/past-end-recovery-20261003from #213 at15cf658e. That tested parent remains unchanged. Default-size layout, aiming rays, end semantics and server data are unchanged.Why
The #213 runtime frames showed that button reachability was not enough: the card hid the circle the homeowner needed to aim. Folding the detail alone still left the title, Details and reply covering the centre. The fix moves the reply as well and tests the circle against the actual rendered controls.
UI changes
Only the walk's end-aim step and past-end offer/refusal use the new accessibility-size treatment. The existing Details disclosure retains every instruction, including coaching and follow-up context. Shared camera chrome and the reticle drawing are unchanged.
At AX5 on the iPhone 17 Simulator. Before: #213 at
15cf658e, run 37165788075. After: this PR at6a5b2401, run 37175974280 attempt 2.The run also kept the default-size, Details-open, left-side, coached and other-side refusal frames for both screens.
Verification
6a5b2401(full-ui). In attempt 1, HouseScanKit's 726 tests in 98 suites passed and the app built. One UI failure was the accessibility audit's contrast check on an edge-cut "I can't get there" ingapRequest-followUp-AX5, a state this PR doesn't change. Attempt 2 reran that job: 103 UI tests, 1 skip (testFullFlowFromRealReplay, which needs a real recording), 0 failures.254a4566, showed the circle's accessibility element staying under the end question while faded out.55449e33now inserts the circle only while it's offered.Risk
Native VoiceOver speech, physical aiming and other device sizes remain unverified. Reduced-motion behavior has source review but no forced runtime check. The tests read the reticle through an "Aiming circle" accessibility element, so VoiceOver now stops on it while it's offered.
This does not change the minimum wall-length policy or make an inferred end a homeowner mark. The parent PR's evidence remains separate from this new layout's acceptance.
Checklist
🤖 Generated with Claude Code
Summary by CodeRabbit