feat: Improve VIN photo capture camera crop experience #123

New Issue

Closed

opened 2026-02-08 01:46:48 +00:00 by egullickson · 4 comments

egullickson commented 2026-02-08 01:46:48 +00:00

Owner

Copy Link

Copy Source

Summary

Improve the camera crop feature of the VIN photo capture flow. Currently there are two usability issues with the crop step after taking a VIN photo.

Current Behavior

1. Overlay/crop mismatch: The dashed rectangle guide overlay shown during camera capture ("Position VIN plate within the guide") does not match what the resulting cropped image looks like. The captured photo appears different from what was framed in the overlay.

2. Crop box is not adjustable: After capturing, the user can "Tap and drag to select crop area", but once the crop box is drawn it cannot be resized or repositioned. The user must start over if the crop selection is incorrect.

Expected Behavior

1. Consistent overlay-to-crop: The overlay guide shown during capture should accurately represent the area that will be cropped. What the user sees in the viewfinder should match the resulting image.

2. Adjustable crop box: After drawing an initial crop selection, the user should be able to:

   • Drag corner/edge handles to resize the crop box
   • Drag the crop box to reposition it
   • Confirm the crop when satisfied

Acceptance Criteria

• Camera overlay guide accurately represents the crop area
• Crop box supports resize via corner/edge handles after initial draw
• Crop box supports repositioning via drag after initial draw
• Works on both mobile and desktop viewports
• OCR accuracy is not degraded by crop changes

Screenshots

See attached screenshots showing:

• The crop screen after capture with "Tap and drag to select crop area" (non-adjustable)
• The camera overlay with dashed guide rectangle during capture

## Summary Improve the camera crop feature of the VIN photo capture flow. Currently there are two usability issues with the crop step after taking a VIN photo. ## Current Behavior 1. **Overlay/crop mismatch**: The dashed rectangle guide overlay shown during camera capture ("Position VIN plate within the guide") does not match what the resulting cropped image looks like. The captured photo appears different from what was framed in the overlay. 2. **Crop box is not adjustable**: After capturing, the user can "Tap and drag to select crop area", but once the crop box is drawn it cannot be resized or repositioned. The user must start over if the crop selection is incorrect. ## Expected Behavior 1. **Consistent overlay-to-crop**: The overlay guide shown during capture should accurately represent the area that will be cropped. What the user sees in the viewfinder should match the resulting image. 2. **Adjustable crop box**: After drawing an initial crop selection, the user should be able to: - Drag corner/edge handles to resize the crop box - Drag the crop box to reposition it - Confirm the crop when satisfied ## Acceptance Criteria - [ ] Camera overlay guide accurately represents the crop area - [ ] Crop box supports resize via corner/edge handles after initial draw - [ ] Crop box supports repositioning via drag after initial draw - [ ] Works on both mobile and desktop viewports - [ ] OCR accuracy is not degraded by crop changes ## Screenshots See attached screenshots showing: - The crop screen after capture with "Tap and drag to select crop area" (non-adjustable) - The camera overlay with dashed guide rectangle during capture

egullickson added the

status

backlog

type

feature

labels 2026-02-08 01:46:53 +00:00

egullickson added this to the Sprint 2026-02-02 milestone 2026-02-08 01:46:57 +00:00

egullickson added

status

in-progress

and removed

status

backlog

labels 2026-02-08 01:47:32 +00:00

egullickson commented 2026-02-08 01:55:31 +00:00

Author

Owner

Copy Link

Copy Source

Plan: Improve VIN Photo Capture Camera Crop

Phase: Planning | Agent: Planner | Status: AWAITING_REVIEW

Overview

The VIN camera crop has two root cause issues: (1) the GuidanceOverlay positions a centered 85%-width guide via CSS flexbox, but the CropTool defaults to {x:10, y:10, width:80, height:80} -- these systems share no coordinate data, so the crop area appears at the wrong position; (2) with VIN aspect ratio 6:1, the crop becomes a thin 13.33%-height strip at the top of the image with 24px touch targets, making handles nearly invisible on mobile.

The fix uses Approach A: calculate guidance-aware initial crop coordinates from the existing GUIDANCE_CONFIGS and pass them through to useImageCrop via the existing initialCrop prop. Touch targets are increased to 44px per Apple HIG and project conventions.

Planning Context

Decision Log

Decision	Reasoning Chain
Calculated initialCrop (Approach A)	Overlay uses CSS centering with known percentage values (85% width for VIN) -> same percentages can derive centered crop coordinates mathematically -> useImageCrop already accepts initialCrop prop -> minimal code changes with high confidence and no new abstractions
44px touch targets	Apple HIG minimum is 44px, project default-conventions.md requires >= 44px -> current 24px is approximately half the minimum -> mobile users cannot reliably grab handles -> increase to 44px touch area with 16px visual indicator
16px visual handle size (from 12px)	12px circle is difficult to see on high-DPI mobile screens -> 16px provides better visibility while 44px touch area covers interaction -> proportional increase without visual clutter
getInitialCropForGuidance() in types.ts	Function is pure (GuidanceType -> CropArea) with no dependencies -> co-locating with GUIDANCE_CONFIGS keeps configuration centralized -> avoids creating new utility file for a single function
No backend/OCR changes	executeCrop() in useImageCrop.ts produces the same JPEG 0.92 output regardless of initial crop position -> OCR preprocessing pipeline receives identical image format -> crop position change is frontend-only

Rejected Alternatives

Alternative	Why Rejected
DOM coordinate measurement (Approach B)	Requires measuring overlay DOM position relative to video element during capture transition -> race conditions between DOM measurement and stream stop -> fragile across screen sizes and orientations -> complexity not justified when mathematical derivation achieves the same result
Shared coordinate system (Approach C)	Would require refactoring GuidanceOverlay from CSS layout to percentage-based positioning -> creates coupling between viewfinder overlay (visual hint) and crop tool (interactive tool) -> over-engineers a solution that only needs a one-time coordinate bridge

Constraints & Assumptions

• React 18 + MUI 6 + TypeScript strict mode
• Touch targets >= 44px (default-conventions.md)
• Mobile + desktop required (320px, 768px, 1920px viewports)
• useImageCrop already accepts optional initialCrop parameter (useImageCrop.ts:11)
• GUIDANCE_CONFIGS defines aspect ratios and overlay width percentages (types.ts:104-120)

Known Risks

Risk	Mitigation	Anchor
Overlay CSS centering may not produce exact same visual position as percentage-based crop	Accepted: CSS flexbox centering on a percentage-width box produces mathematically equivalent positioning to the percentage calculation. Both center a box of known proportions.	GuidanceOverlay.tsx:L50-L57 (flexbox center)
Larger touch targets may overlap on very thin VIN crop strip	Centered initial crop at ~14% height provides sufficient space for 44px handles. On 640px screen, 14% = ~90px, handles at corners with 44px do not overlap.	N/A - mathematical
executeCrop image quality unchanged	executeCrop uses cropArea percentages to calculate pixel coords on the source image. Changing initial position does not affect the drawImage or toBlob logic.	useImageCrop.ts:L262-L319

Invisible Knowledge

Architecture

types.ts
  GUIDANCE_CONFIGS (aspectRatio, width%) ──> getInitialCropForGuidance()
                                                     |
                                                     v
CameraCapture.tsx                              CropArea (centered)
  guidanceType ──> getInitialCropForGuidance() ──> initialCrop prop
                                                     |
                                                     v
CropTool.tsx                                   useImageCrop(initialCrop)
  Renders crop box at centered position
  44px touch targets on all handles

Data Flow

Camera Viewfinder                    Crop Tool
  GuidanceOverlay ─── (visual only,  getInitialCropForGuidance()
  CSS centered at     no data flows)   calculates matching
  85% width, 6:1 AR  ─────────────>   centered CropArea
                                       {x:7.5, y:42.92, w:85, h:14.17}
                                            |
                                            v
                                       useImageCrop(initialCrop)
                                            |
                                            v
                                       User adjusts with handles
                                            |
                                            v
                                       executeCrop() -> Blob -> OCR

Invariants

• getInitialCropForGuidance() output must produce a CropArea that, when rendered on the captured image, visually matches the GuidanceOverlay position from the viewfinder
• For width-constrained guides (VIN): crop width matches overlay width (85%), centered horizontally
• For height-constrained guides (receipt, document): crop height matches overlay height (70%), centered vertically
• All touch targets >= 44px on interactive elements

Tradeoffs

• Approximate vs pixel-exact positioning: Mathematical derivation from configuration values is simpler and more maintainable than DOM measurement, at the cost of not accounting for browser rendering quirks. Acceptable because the overlay is a guide hint, not a pixel-precise tool.
• Larger handles increase visual noise but significantly improve mobile usability.

Milestones

Milestone 1: Bridge overlay position to crop initial coordinates

Files: frontend/src/shared/components/CameraCapture/types.ts, frontend/src/shared/components/CameraCapture/CameraCapture.tsx, frontend/src/shared/components/CameraCapture/CropTool.tsx

Requirements:

• Add getInitialCropForGuidance(type: GuidanceType): CropArea | undefined to types.ts
• For VIN (AR=6, 85% width): return {x: 7.5, y: 42.92, width: 85, height: 14.17}
• For receipt (AR=2/3, 70% height): return {x: 26.67, y: 15, width: 46.67, height: 70}
• For document (AR=8.5/11, 70% height): return {x: 22.95, y: 15, width: 54.09, height: 70}
• For 'none': return undefined (use default)
• Add initialCrop optional prop to CropToolProps
• CameraCapture.tsx: compute initialCrop and pass to CropTool
• CropTool.tsx: forward initialCrop to useImageCrop

Acceptance Criteria:

• VIN crop box appears centered on the captured image, matching overlay guide position
• Receipt crop box appears centered, matching overlay guide position
• Document crop box appears centered, matching overlay guide position
• 'none' guidance type uses existing default crop

Tests:

• Test file: frontend/src/shared/components/CameraCapture/__tests__/getInitialCropForGuidance.test.ts
• Test type: unit
• Backing: default-derived (pure function with clear input/output contract)
• Scenarios:
  • Normal: VIN returns centered 85%-width crop with 6:1 AR
  • Normal: Receipt returns centered 70%-height crop with 2:3 AR
  • Normal: Document returns centered 70%-height crop with 8.5:11 AR
  • Edge: 'none' type returns undefined
  • Edge: All returned CropAreas have x+width <= 100 and y+height <= 100

Code Changes:

--- a/frontend/src/shared/components/CameraCapture/types.ts
+++ b/frontend/src/shared/components/CameraCapture/types.ts
@@ -58,6 +58,8 @@ export interface CropToolProps {
   imageSrc: string;
   /** Whether to lock aspect ratio */
   lockAspectRatio?: boolean;
+  /** Initial crop area matching guidance overlay position */
+  initialCrop?: CropArea;
   /** Aspect ratio to lock to (width/height) */
   aspectRatio?: number;
   /** Callback when crop is confirmed */
@@ -119,3 +121,30 @@ export const GUIDANCE_CONFIGS: Record<Exclude<GuidanceType, 'none'>, GuidanceCon

 /** Default max file size (10MB) */
 export const DEFAULT_MAX_FILE_SIZE = 10 * 1024 * 1024;
+
+/** Derives centered CropArea matching GuidanceOverlay position for each type */
+export function getInitialCropForGuidance(type: GuidanceType): CropArea | undefined {
+  if (type === 'none') return undefined;
+
+  const config = GUIDANCE_CONFIGS[type];
+
+  if (config.aspectRatio > 1) {
+    // Width-constrained (VIN): 85% width, height from AR, centered
+    const width = 85;
+    const height = width / config.aspectRatio;
+    const x = (100 - width) / 2;
+    const y = (100 - height) / 2;
+    return { x, y, width, height };
+  }
+
+  // Height-constrained (receipt, document): 70% height, width from AR, centered
+  const height = 70;
+  const width = height * config.aspectRatio;
+  const x = (100 - width) / 2;
+  const y = (100 - height) / 2;
+  return { x, y, width, height };
+}

--- a/frontend/src/shared/components/CameraCapture/CameraCapture.tsx
+++ b/frontend/src/shared/components/CameraCapture/CameraCapture.tsx
@@ -11,6 +11,7 @@ import {
   DEFAULT_ACCEPTED_FORMATS,
   DEFAULT_MAX_FILE_SIZE,
   GUIDANCE_CONFIGS,
+  getInitialCropForGuidance,
 } from './types';
 import { useCameraPermission } from './useCameraPermission';
@@ -191,6 +192,9 @@ export const CameraCapture: React.FC<CameraCaptureProps> = ({
   const cropAspectRatio =
     guidanceType !== 'none' ? GUIDANCE_CONFIGS[guidanceType].aspectRatio : undefined;

+  // Crop initial position matches guidance overlay center
+  const cropInitialArea = getInitialCropForGuidance(guidanceType);
+
   // Render permission error state
   if (permissionState === 'denied' && !useFallback) {
@@ -244,6 +248,7 @@ export const CameraCapture: React.FC<CameraCaptureProps> = ({
     return (
       <CropTool
         imageSrc={capturedImageSrc}
+        initialCrop={cropInitialArea}
         lockAspectRatio={guidanceType !== 'none'}
         aspectRatio={cropAspectRatio}
         onConfirm={handleCropConfirm}

--- a/frontend/src/shared/components/CameraCapture/CropTool.tsx
+++ b/frontend/src/shared/components/CameraCapture/CropTool.tsx
@@ -15,6 +15,7 @@ import type { CropToolProps } from './types';
 export const CropTool: React.FC<CropToolProps> = ({
   imageSrc,
   lockAspectRatio = false,
+  initialCrop,
   aspectRatio,
   onConfirm,
   onReset,
@@ -26,6 +27,7 @@ export const CropTool: React.FC<CropToolProps> = ({
   const { cropArea, isDragging, resetCrop, executeCrop, handleDragStart } =
     useImageCrop({
       aspectRatio: lockAspectRatio ? aspectRatio : undefined,
+      initialCrop,
     });

Milestone 2: Improve crop handle touch targets for mobile

Files: frontend/src/shared/components/CameraCapture/CropTool.tsx

Flags: needs conformance check (44px touch target requirement)

Requirements:

• Increase handle touch target from 24px to 44px
• Increase handle visual indicator from 12px to 16px
• Increase edge handle bar dimensions proportionally (width 8->12 for thin, 24->32 for long)
• Adjust CropHandleArea move inset from 8px to 16px to accommodate larger corner handles

Acceptance Criteria:

• All crop handles have >= 44px touch target area
• Corner handles show 16px circle visual
• Edge handles show proportionally larger bar visuals
• Handles do not overflow crop box boundary on VIN-sized crop (~14% height)
• Move drag still works within the crop area

Tests:

• Skip: visual layout testing requires browser environment. Verified via mobile viewport manual testing during QR post-implementation review.

Code Changes:

--- a/frontend/src/shared/components/CameraCapture/CropTool.tsx
+++ b/frontend/src/shared/components/CameraCapture/CropTool.tsx
@@ -162,7 +162,7 @@ export const CropTool: React.FC<CropToolProps> = ({
             {/* Move handle (center area) */}
             <CropHandleArea
               handle="move"
               onDragStart={handleDragStart}
               sx={{
                 position: 'absolute',
-                inset: 8,
+                inset: 16,
                 cursor: 'move',
               }}
@@ -297,8 +297,8 @@ interface CropHandleProps {
 }

 const CropHandle: React.FC<CropHandleProps> = ({ handle, onDragStart, position }) => {
-  const handleSize = 24;
-  const handleVisualSize = 12;
+  const handleSize = 44;
+  const handleVisualSize = 16;

   const positionStyles: Record<string, React.CSSProperties> = {
@@ -366,8 +366,8 @@ const CropHandle: React.FC<CropHandleProps> = ({ handle, onDragStart, position }
       <Box
         sx={{
-          width: isCorner ? handleVisualSize : position === 'top' || position === 'bottom' ? 24 : 8,
-          height: isCorner ? handleVisualSize : position === 'left' || position === 'right' ? 24 : 8,
+          width: isCorner ? handleVisualSize : position === 'top' || position === 'bottom' ? 32 : 12,
+          height: isCorner ? handleVisualSize : position === 'left' || position === 'right' ? 32 : 12,
           backgroundColor: 'white',
           borderRadius: isCorner ? '50%' : 1,
           boxShadow: '0 2px 4px rgba(0,0,0,0.3)',

Milestone 3: Documentation

Files: frontend/src/shared/components/CameraCapture/README.md (if needed)

Requirements:

• No new files created; existing CLAUDE.md indexes already cover CameraCapture/
• If Invisible Knowledge warrants it, add architecture note to CameraCapture README

Acceptance Criteria:

• Developers understand the overlay-to-crop coordinate bridge from reading source

Source Material: Invisible Knowledge section of this plan

Milestone Dependencies

M1 (coordinate bridge) ---> M2 (touch targets) ---> M3 (documentation)

M2 depends on M1 because correct crop positioning must exist before handle improvements matter. M3 depends on both being complete.

Verdict: AWAITING_REVIEW | Next: Plan review cycle (QR plan-completeness -> TW plan-scrub -> QR plan-code -> QR plan-docs)

## Plan: Improve VIN Photo Capture Camera Crop **Phase**: Planning | **Agent**: Planner | **Status**: AWAITING_REVIEW ## Overview The VIN camera crop has two root cause issues: (1) the GuidanceOverlay positions a centered 85%-width guide via CSS flexbox, but the CropTool defaults to `{x:10, y:10, width:80, height:80}` -- these systems share no coordinate data, so the crop area appears at the wrong position; (2) with VIN aspect ratio 6:1, the crop becomes a thin 13.33%-height strip at the top of the image with 24px touch targets, making handles nearly invisible on mobile. The fix uses Approach A: calculate guidance-aware initial crop coordinates from the existing `GUIDANCE_CONFIGS` and pass them through to `useImageCrop` via the existing `initialCrop` prop. Touch targets are increased to 44px per Apple HIG and project conventions. ## Planning Context ### Decision Log | Decision | Reasoning Chain | | --- | --- | | Calculated initialCrop (Approach A) | Overlay uses CSS centering with known percentage values (85% width for VIN) -> same percentages can derive centered crop coordinates mathematically -> `useImageCrop` already accepts `initialCrop` prop -> minimal code changes with high confidence and no new abstractions | | 44px touch targets | Apple HIG minimum is 44px, project default-conventions.md requires >= 44px -> current 24px is approximately half the minimum -> mobile users cannot reliably grab handles -> increase to 44px touch area with 16px visual indicator | | 16px visual handle size (from 12px) | 12px circle is difficult to see on high-DPI mobile screens -> 16px provides better visibility while 44px touch area covers interaction -> proportional increase without visual clutter | | `getInitialCropForGuidance()` in types.ts | Function is pure (GuidanceType -> CropArea) with no dependencies -> co-locating with GUIDANCE_CONFIGS keeps configuration centralized -> avoids creating new utility file for a single function | | No backend/OCR changes | `executeCrop()` in `useImageCrop.ts` produces the same JPEG 0.92 output regardless of initial crop position -> OCR preprocessing pipeline receives identical image format -> crop position change is frontend-only | ### Rejected Alternatives | Alternative | Why Rejected | | --- | --- | | DOM coordinate measurement (Approach B) | Requires measuring overlay DOM position relative to video element during capture transition -> race conditions between DOM measurement and stream stop -> fragile across screen sizes and orientations -> complexity not justified when mathematical derivation achieves the same result | | Shared coordinate system (Approach C) | Would require refactoring GuidanceOverlay from CSS layout to percentage-based positioning -> creates coupling between viewfinder overlay (visual hint) and crop tool (interactive tool) -> over-engineers a solution that only needs a one-time coordinate bridge | ### Constraints & Assumptions - React 18 + MUI 6 + TypeScript strict mode - Touch targets >= 44px (default-conventions.md) - Mobile + desktop required (320px, 768px, 1920px viewports) - `useImageCrop` already accepts optional `initialCrop` parameter (useImageCrop.ts:11) - `GUIDANCE_CONFIGS` defines aspect ratios and overlay width percentages (types.ts:104-120) ### Known Risks | Risk | Mitigation | Anchor | | --- | --- | --- | | Overlay CSS centering may not produce exact same visual position as percentage-based crop | Accepted: CSS flexbox centering on a percentage-width box produces mathematically equivalent positioning to the percentage calculation. Both center a box of known proportions. | GuidanceOverlay.tsx:L50-L57 (flexbox center) | | Larger touch targets may overlap on very thin VIN crop strip | Centered initial crop at ~14% height provides sufficient space for 44px handles. On 640px screen, 14% = ~90px, handles at corners with 44px do not overlap. | N/A - mathematical | | `executeCrop` image quality unchanged | `executeCrop` uses `cropArea` percentages to calculate pixel coords on the source image. Changing initial position does not affect the drawImage or toBlob logic. | useImageCrop.ts:L262-L319 | ## Invisible Knowledge ### Architecture ``` types.ts GUIDANCE_CONFIGS (aspectRatio, width%) ──> getInitialCropForGuidance() | v CameraCapture.tsx CropArea (centered) guidanceType ──> getInitialCropForGuidance() ──> initialCrop prop | v CropTool.tsx useImageCrop(initialCrop) Renders crop box at centered position 44px touch targets on all handles ``` ### Data Flow ``` Camera Viewfinder Crop Tool GuidanceOverlay ─── (visual only, getInitialCropForGuidance() CSS centered at no data flows) calculates matching 85% width, 6:1 AR ─────────────> centered CropArea {x:7.5, y:42.92, w:85, h:14.17} | v useImageCrop(initialCrop) | v User adjusts with handles | v executeCrop() -> Blob -> OCR ``` ### Invariants - `getInitialCropForGuidance()` output must produce a CropArea that, when rendered on the captured image, visually matches the GuidanceOverlay position from the viewfinder - For width-constrained guides (VIN): crop width matches overlay width (85%), centered horizontally - For height-constrained guides (receipt, document): crop height matches overlay height (70%), centered vertically - All touch targets >= 44px on interactive elements ### Tradeoffs - Approximate vs pixel-exact positioning: Mathematical derivation from configuration values is simpler and more maintainable than DOM measurement, at the cost of not accounting for browser rendering quirks. Acceptable because the overlay is a guide hint, not a pixel-precise tool. - Larger handles increase visual noise but significantly improve mobile usability. ## Milestones ### Milestone 1: Bridge overlay position to crop initial coordinates **Files**: `frontend/src/shared/components/CameraCapture/types.ts`, `frontend/src/shared/components/CameraCapture/CameraCapture.tsx`, `frontend/src/shared/components/CameraCapture/CropTool.tsx` **Requirements**: - Add `getInitialCropForGuidance(type: GuidanceType): CropArea | undefined` to types.ts - For VIN (AR=6, 85% width): return `{x: 7.5, y: 42.92, width: 85, height: 14.17}` - For receipt (AR=2/3, 70% height): return `{x: 26.67, y: 15, width: 46.67, height: 70}` - For document (AR=8.5/11, 70% height): return `{x: 22.95, y: 15, width: 54.09, height: 70}` - For 'none': return undefined (use default) - Add `initialCrop` optional prop to CropToolProps - CameraCapture.tsx: compute initialCrop and pass to CropTool - CropTool.tsx: forward initialCrop to useImageCrop **Acceptance Criteria**: - VIN crop box appears centered on the captured image, matching overlay guide position - Receipt crop box appears centered, matching overlay guide position - Document crop box appears centered, matching overlay guide position - 'none' guidance type uses existing default crop **Tests**: - **Test file**: `frontend/src/shared/components/CameraCapture/__tests__/getInitialCropForGuidance.test.ts` - **Test type**: unit - **Backing**: default-derived (pure function with clear input/output contract) - **Scenarios**: - Normal: VIN returns centered 85%-width crop with 6:1 AR - Normal: Receipt returns centered 70%-height crop with 2:3 AR - Normal: Document returns centered 70%-height crop with 8.5:11 AR - Edge: 'none' type returns undefined - Edge: All returned CropAreas have x+width <= 100 and y+height <= 100 **Code Changes**: ```diff --- a/frontend/src/shared/components/CameraCapture/types.ts +++ b/frontend/src/shared/components/CameraCapture/types.ts @@ -58,6 +58,8 @@ export interface CropToolProps { imageSrc: string; /** Whether to lock aspect ratio */ lockAspectRatio?: boolean; + /** Initial crop area matching guidance overlay position */ + initialCrop?: CropArea; /** Aspect ratio to lock to (width/height) */ aspectRatio?: number; /** Callback when crop is confirmed */ @@ -119,3 +121,30 @@ export const GUIDANCE_CONFIGS: Record<Exclude<GuidanceType, 'none'>, GuidanceCon /** Default max file size (10MB) */ export const DEFAULT_MAX_FILE_SIZE = 10 * 1024 * 1024; + +/** Derives centered CropArea matching GuidanceOverlay position for each type */ +export function getInitialCropForGuidance(type: GuidanceType): CropArea | undefined { + if (type === 'none') return undefined; + + const config = GUIDANCE_CONFIGS[type]; + + if (config.aspectRatio > 1) { + // Width-constrained (VIN): 85% width, height from AR, centered + const width = 85; + const height = width / config.aspectRatio; + const x = (100 - width) / 2; + const y = (100 - height) / 2; + return { x, y, width, height }; + } + + // Height-constrained (receipt, document): 70% height, width from AR, centered + const height = 70; + const width = height * config.aspectRatio; + const x = (100 - width) / 2; + const y = (100 - height) / 2; + return { x, y, width, height }; +} ``` ```diff --- a/frontend/src/shared/components/CameraCapture/CameraCapture.tsx +++ b/frontend/src/shared/components/CameraCapture/CameraCapture.tsx @@ -11,6 +11,7 @@ import { DEFAULT_ACCEPTED_FORMATS, DEFAULT_MAX_FILE_SIZE, GUIDANCE_CONFIGS, + getInitialCropForGuidance, } from './types'; import { useCameraPermission } from './useCameraPermission'; @@ -191,6 +192,9 @@ export const CameraCapture: React.FC<CameraCaptureProps> = ({ const cropAspectRatio = guidanceType !== 'none' ? GUIDANCE_CONFIGS[guidanceType].aspectRatio : undefined; + // Crop initial position matches guidance overlay center + const cropInitialArea = getInitialCropForGuidance(guidanceType); + // Render permission error state if (permissionState === 'denied' && !useFallback) { @@ -244,6 +248,7 @@ export const CameraCapture: React.FC<CameraCaptureProps> = ({ return ( <CropTool imageSrc={capturedImageSrc} + initialCrop={cropInitialArea} lockAspectRatio={guidanceType !== 'none'} aspectRatio={cropAspectRatio} onConfirm={handleCropConfirm} ``` ```diff --- a/frontend/src/shared/components/CameraCapture/CropTool.tsx +++ b/frontend/src/shared/components/CameraCapture/CropTool.tsx @@ -15,6 +15,7 @@ import type { CropToolProps } from './types'; export const CropTool: React.FC<CropToolProps> = ({ imageSrc, lockAspectRatio = false, + initialCrop, aspectRatio, onConfirm, onReset, @@ -26,6 +27,7 @@ export const CropTool: React.FC<CropToolProps> = ({ const { cropArea, isDragging, resetCrop, executeCrop, handleDragStart } = useImageCrop({ aspectRatio: lockAspectRatio ? aspectRatio : undefined, + initialCrop, }); ``` ### Milestone 2: Improve crop handle touch targets for mobile **Files**: `frontend/src/shared/components/CameraCapture/CropTool.tsx` **Flags**: needs conformance check (44px touch target requirement) **Requirements**: - Increase handle touch target from 24px to 44px - Increase handle visual indicator from 12px to 16px - Increase edge handle bar dimensions proportionally (width 8->12 for thin, 24->32 for long) - Adjust CropHandleArea move inset from 8px to 16px to accommodate larger corner handles **Acceptance Criteria**: - All crop handles have >= 44px touch target area - Corner handles show 16px circle visual - Edge handles show proportionally larger bar visuals - Handles do not overflow crop box boundary on VIN-sized crop (~14% height) - Move drag still works within the crop area **Tests**: - Skip: visual layout testing requires browser environment. Verified via mobile viewport manual testing during QR post-implementation review. **Code Changes**: ```diff --- a/frontend/src/shared/components/CameraCapture/CropTool.tsx +++ b/frontend/src/shared/components/CameraCapture/CropTool.tsx @@ -162,7 +162,7 @@ export const CropTool: React.FC<CropToolProps> = ({ {/* Move handle (center area) */} <CropHandleArea handle="move" onDragStart={handleDragStart} sx={{ position: 'absolute', - inset: 8, + inset: 16, cursor: 'move', }} @@ -297,8 +297,8 @@ interface CropHandleProps { } const CropHandle: React.FC<CropHandleProps> = ({ handle, onDragStart, position }) => { - const handleSize = 24; - const handleVisualSize = 12; + const handleSize = 44; + const handleVisualSize = 16; const positionStyles: Record<string, React.CSSProperties> = { @@ -366,8 +366,8 @@ const CropHandle: React.FC<CropHandleProps> = ({ handle, onDragStart, position } <Box sx={{ - width: isCorner ? handleVisualSize : position === 'top' || position === 'bottom' ? 24 : 8, - height: isCorner ? handleVisualSize : position === 'left' || position === 'right' ? 24 : 8, + width: isCorner ? handleVisualSize : position === 'top' || position === 'bottom' ? 32 : 12, + height: isCorner ? handleVisualSize : position === 'left' || position === 'right' ? 32 : 12, backgroundColor: 'white', borderRadius: isCorner ? '50%' : 1, boxShadow: '0 2px 4px rgba(0,0,0,0.3)', ``` ### Milestone 3: Documentation **Files**: `frontend/src/shared/components/CameraCapture/README.md` (if needed) **Requirements**: - No new files created; existing CLAUDE.md indexes already cover CameraCapture/ - If Invisible Knowledge warrants it, add architecture note to CameraCapture README **Acceptance Criteria**: - Developers understand the overlay-to-crop coordinate bridge from reading source **Source Material**: Invisible Knowledge section of this plan ## Milestone Dependencies ``` M1 (coordinate bridge) ---> M2 (touch targets) ---> M3 (documentation) ``` M2 depends on M1 because correct crop positioning must exist before handle improvements matter. M3 depends on both being complete. *Verdict*: AWAITING_REVIEW | *Next*: Plan review cycle (QR plan-completeness -> TW plan-scrub -> QR plan-code -> QR plan-docs)

egullickson commented 2026-02-08 01:57:21 +00:00

Author

Owner

Copy Link

Copy Source

QR Review: Plan Completeness + Plan Code

Phase: Plan-Review | Agent: Quality Reviewer | Status: NEEDS_CHANGES

Findings

RULE 0 (CRITICAL):

1. getInitialCropForGuidance lacks bounds validation -- if GUIDANCE_CONFIGS change, coordinates could exceed 0-100 range
2. aspectRatio === 1 falls through to height-constrained path -- should use >= 1

RULE 1 (HIGH):
3. At 320px viewport height, VIN crop strip is ~45px tall. With 44px corner handles, they nearly fill the entire strip height. Handles need to be clamped or use a responsive approach.
4. Diff line numbers are approximate (acceptable per diff-format spec -- context lines are the authoritative anchors)

RULE 2 (SHOULD_FIX):
5. Centering calculation (100 - dimension) / 2 duplicated in both paths -- minor, acceptable for a 2-path function

Plan Revisions Required

1. Add Math.min/Math.max bounds clamping to returned CropArea
2. Change config.aspectRatio > 1 to config.aspectRatio >= 1
3. Address 320px viewport: use responsive handle sizes (44px on screens > 400px, 32px on smaller) or accept that VIN crop on very small screens has tighter constraints

Verdict: NEEDS_CHANGES | Next: Revise plan, re-review

## QR Review: Plan Completeness + Plan Code **Phase**: Plan-Review | **Agent**: Quality Reviewer | **Status**: NEEDS_CHANGES ### Findings **RULE 0 (CRITICAL):** 1. `getInitialCropForGuidance` lacks bounds validation -- if GUIDANCE_CONFIGS change, coordinates could exceed 0-100 range 2. `aspectRatio === 1` falls through to height-constrained path -- should use `>= 1` **RULE 1 (HIGH):** 3. At 320px viewport height, VIN crop strip is ~45px tall. With 44px corner handles, they nearly fill the entire strip height. Handles need to be clamped or use a responsive approach. 4. Diff line numbers are approximate (acceptable per diff-format spec -- context lines are the authoritative anchors) **RULE 2 (SHOULD_FIX):** 5. Centering calculation `(100 - dimension) / 2` duplicated in both paths -- minor, acceptable for a 2-path function ### Plan Revisions Required 1. Add `Math.min`/`Math.max` bounds clamping to returned CropArea 2. Change `config.aspectRatio > 1` to `config.aspectRatio >= 1` 3. Address 320px viewport: use responsive handle sizes (44px on screens > 400px, 32px on smaller) or accept that VIN crop on very small screens has tighter constraints *Verdict*: NEEDS_CHANGES | *Next*: Revise plan, re-review

egullickson commented 2026-02-08 01:58:08 +00:00

Author

Owner

Copy Link

Copy Source

Plan Revision: Addressing QR Findings

Phase: Planning | Agent: Planner | Status: AWAITING_REVIEW

Changes from QR Review

RULE 0 Fix 1 -- Bounds validation: getInitialCropForGuidance now clamps all values to 0-100 range using Math.max(0, Math.min(100, value)). This guards against future GUIDANCE_CONFIGS changes producing out-of-bounds coordinates.

RULE 0 Fix 2 -- Aspect ratio boundary: Changed condition from config.aspectRatio > 1 to config.aspectRatio >= 1 so square (1:1) aspect ratios use the width-constrained path.

RULE 1 Fix 3 -- 320px viewport handle sizes: Use useTheme + useMediaQuery to detect small screens (< 400px height) and reduce handle size to 32px. This keeps handles within the VIN crop strip on very small viewports while maintaining 44px on normal mobile and desktop.

Revised Code: getInitialCropForGuidance (types.ts)

--- a/frontend/src/shared/components/CameraCapture/types.ts
+++ b/frontend/src/shared/components/CameraCapture/types.ts
@@ -58,6 +58,8 @@ export interface CropToolProps {
   imageSrc: string;
   /** Whether to lock aspect ratio */
   lockAspectRatio?: boolean;
+  /** Initial crop area matching guidance overlay position */
+  initialCrop?: CropArea;
   /** Aspect ratio to lock to (width/height) */
   aspectRatio?: number;
   /** Callback when crop is confirmed */
@@ -129,3 +131,35 @@ export const DEFAULT_ACCEPTED_FORMATS = [

 /** Default max file size (10MB) */
 export const DEFAULT_MAX_FILE_SIZE = 10 * 1024 * 1024;
+
+/** Clamps a value to the 0-100 percentage range */
+function clampPct(value: number): number {
+  return Math.max(0, Math.min(100, value));
+}
+
+/** Derives centered CropArea matching GuidanceOverlay position for each type */
+export function getInitialCropForGuidance(type: GuidanceType): CropArea | undefined {
+  if (type === 'none') return undefined;
+
+  const config = GUIDANCE_CONFIGS[type];
+  let width: number;
+  let height: number;
+
+  if (config.aspectRatio >= 1) {
+    // Width-constrained (VIN, square): 85% width, height from AR, centered
+    width = 85;
+    height = width / config.aspectRatio;
+  } else {
+    // Height-constrained (receipt, document): 70% height, width from AR, centered
+    height = 70;
+    width = height * config.aspectRatio;
+  }
+
+  return {
+    x: clampPct((100 - width) / 2),
+    y: clampPct((100 - height) / 2),
+    width: clampPct(width),
+    height: clampPct(height),
+  };
+}

Revised Code: Responsive Handle Sizes (CropTool.tsx)

--- a/frontend/src/shared/components/CameraCapture/CropTool.tsx
+++ b/frontend/src/shared/components/CameraCapture/CropTool.tsx
@@ -5,7 +5,7 @@

 import React, { useCallback, useState } from 'react';
-import { Box, IconButton, Button, Typography, CircularProgress } from '@mui/material';
+import { Box, IconButton, Button, Typography, CircularProgress, useTheme, useMediaQuery } from '@mui/material';
 import CheckIcon from '@mui/icons-material/Check';
 import RefreshIcon from '@mui/icons-material/Refresh';
@@ -15,6 +15,7 @@ import type { CropToolProps } from './types';
 export const CropTool: React.FC<CropToolProps> = ({
   imageSrc,
   lockAspectRatio = false,
+  initialCrop,
   aspectRatio,
   onConfirm,
   onReset,
@@ -23,9 +24,14 @@ export const CropTool: React.FC<CropToolProps> = ({
 }) => {
   const [isProcessing, setIsProcessing] = useState(false);

+  // Responsive handle sizing: 44px standard, 32px on small viewports
+  const theme = useTheme();
+  const isCompactViewport = useMediaQuery('(max-height: 400px)');
+  const handleConfig = { size: isCompactViewport ? 32 : 44, visual: isCompactViewport ? 12 : 16 };
+
   const { cropArea, isDragging, resetCrop, executeCrop, handleDragStart } =
     useImageCrop({
       aspectRatio: lockAspectRatio ? aspectRatio : undefined,
+      initialCrop,
     });

Handle component updated to use passed config instead of hardcoded values:

--- a/frontend/src/shared/components/CameraCapture/CropTool.tsx
+++ b/frontend/src/shared/components/CameraCapture/CropTool.tsx
@@ -162,7 +167,7 @@ export const CropTool: React.FC<CropToolProps> = ({
             {/* Move handle (center area) */}
             <CropHandleArea
               handle="move"
               onDragStart={handleDragStart}
               sx={{
                 position: 'absolute',
-                inset: 8,
+                inset: handleConfig.size / 2,
                 cursor: 'move',
               }}
@@ -173,14 +178,14 @@ export const CropTool: React.FC<CropToolProps> = ({

             {/* Corner handles */}
-            <CropHandle handle="nw" onDragStart={handleDragStart} position="top-left" />
-            <CropHandle handle="ne" onDragStart={handleDragStart} position="top-right" />
-            <CropHandle handle="sw" onDragStart={handleDragStart} position="bottom-left" />
-            <CropHandle handle="se" onDragStart={handleDragStart} position="bottom-right" />
+            <CropHandle handle="nw" onDragStart={handleDragStart} position="top-left" config={handleConfig} />
+            <CropHandle handle="ne" onDragStart={handleDragStart} position="top-right" config={handleConfig} />
+            <CropHandle handle="sw" onDragStart={handleDragStart} position="bottom-left" config={handleConfig} />
+            <CropHandle handle="se" onDragStart={handleDragStart} position="bottom-right" config={handleConfig} />

             {/* Edge handles */}
-            <CropHandle handle="n" onDragStart={handleDragStart} position="top" />
-            <CropHandle handle="s" onDragStart={handleDragStart} position="bottom" />
-            <CropHandle handle="w" onDragStart={handleDragStart} position="left" />
-            <CropHandle handle="e" onDragStart={handleDragStart} position="right" />
+            <CropHandle handle="n" onDragStart={handleDragStart} position="top" config={handleConfig} />
+            <CropHandle handle="s" onDragStart={handleDragStart} position="bottom" config={handleConfig} />
+            <CropHandle handle="w" onDragStart={handleDragStart} position="left" config={handleConfig} />
+            <CropHandle handle="e" onDragStart={handleDragStart} position="right" config={handleConfig} />

CropHandle component accepts config prop:

--- a/frontend/src/shared/components/CameraCapture/CropTool.tsx
+++ b/frontend/src/shared/components/CameraCapture/CropTool.tsx
@@ -284,6 +289,7 @@ interface CropHandleProps {
   handle: CropHandle;
   onDragStart: (handle: CropHandle, event: React.MouseEvent | React.TouchEvent) => void;
+  config: { size: number; visual: number };
   position:
     | 'top-left'
     | 'top-right'
@@ -297,8 +303,8 @@ interface CropHandleProps {
 }

-const CropHandle: React.FC<CropHandleProps> = ({ handle, onDragStart, position }) => {
-  const handleSize = 24;
-  const handleVisualSize = 12;
+const CropHandle: React.FC<CropHandleProps> = ({ handle, onDragStart, position, config }) => {
+  const handleSize = config.size;
+  const handleVisualSize = config.visual;

   const positionStyles: Record<string, React.CSSProperties> = {
@@ -366,8 +372,8 @@ const CropHandle: React.FC<CropHandleProps> = ({ handle, onDragStart, position }
       <Box
         sx={{
-          width: isCorner ? handleVisualSize : position === 'top' || position === 'bottom' ? 24 : 8,
-          height: isCorner ? handleVisualSize : position === 'left' || position === 'right' ? 24 : 8,
+          width: isCorner ? handleVisualSize : position === 'top' || position === 'bottom' ? handleSize * 0.7 : handleVisualSize * 0.75,
+          height: isCorner ? handleVisualSize : position === 'left' || position === 'right' ? handleSize * 0.7 : handleVisualSize * 0.75,
           backgroundColor: 'white',
           borderRadius: isCorner ? '50%' : 1,
           boxShadow: '0 2px 4px rgba(0,0,0,0.3)',

Updated Decision Log Entry

Decision	Reasoning Chain
Responsive handle sizes (44px/32px)	Apple HIG requires 44px minimum touch targets -> but VIN crop strip at 320px viewport height is only ~45px tall -> 44px handles would nearly fill entire strip -> use useMediaQuery('(max-height: 400px)') to detect compact viewports -> 32px handles on compact, 44px on standard -> maintains usability without overflowing
Bounds clamping via clampPct	GUIDANCE_CONFIGS values could change in future -> unclamped arithmetic could produce negative or >100 percentages -> explicit clamping guards against invalid crop coordinates at trivial cost
aspectRatio >= 1 (from > 1)	Edge case: 1:1 square guidance type -> > 1 sends to height-constrained path which uses 70% height -> >= 1 sends to width-constrained path using 85% width -> width-constrained is more appropriate for square guides on wide screens

Updated Test Scenarios (M1)

Add to existing scenarios:

• Edge: All CropArea values are within 0-100 range (validated by clampPct)
• Edge: Hypothetical square guidance (AR=1) returns 85% width, 85% height centered

Updated Known Risks

Risk	Mitigation	Anchor
320px viewport VIN handles	32px responsive handles on compact viewports via useMediaQuery	CropTool.tsx (planned)

Verdict: AWAITING_REVIEW | Next: Re-review QR plan-completeness

## Plan Revision: Addressing QR Findings **Phase**: Planning | **Agent**: Planner | **Status**: AWAITING_REVIEW ### Changes from QR Review **RULE 0 Fix 1 -- Bounds validation**: `getInitialCropForGuidance` now clamps all values to 0-100 range using `Math.max(0, Math.min(100, value))`. This guards against future GUIDANCE_CONFIGS changes producing out-of-bounds coordinates. **RULE 0 Fix 2 -- Aspect ratio boundary**: Changed condition from `config.aspectRatio > 1` to `config.aspectRatio >= 1` so square (1:1) aspect ratios use the width-constrained path. **RULE 1 Fix 3 -- 320px viewport handle sizes**: Use `useTheme` + `useMediaQuery` to detect small screens (< 400px height) and reduce handle size to 32px. This keeps handles within the VIN crop strip on very small viewports while maintaining 44px on normal mobile and desktop. ### Revised Code: getInitialCropForGuidance (types.ts) ```diff --- a/frontend/src/shared/components/CameraCapture/types.ts +++ b/frontend/src/shared/components/CameraCapture/types.ts @@ -58,6 +58,8 @@ export interface CropToolProps { imageSrc: string; /** Whether to lock aspect ratio */ lockAspectRatio?: boolean; + /** Initial crop area matching guidance overlay position */ + initialCrop?: CropArea; /** Aspect ratio to lock to (width/height) */ aspectRatio?: number; /** Callback when crop is confirmed */ @@ -129,3 +131,35 @@ export const DEFAULT_ACCEPTED_FORMATS = [ /** Default max file size (10MB) */ export const DEFAULT_MAX_FILE_SIZE = 10 * 1024 * 1024; + +/** Clamps a value to the 0-100 percentage range */ +function clampPct(value: number): number { + return Math.max(0, Math.min(100, value)); +} + +/** Derives centered CropArea matching GuidanceOverlay position for each type */ +export function getInitialCropForGuidance(type: GuidanceType): CropArea | undefined { + if (type === 'none') return undefined; + + const config = GUIDANCE_CONFIGS[type]; + let width: number; + let height: number; + + if (config.aspectRatio >= 1) { + // Width-constrained (VIN, square): 85% width, height from AR, centered + width = 85; + height = width / config.aspectRatio; + } else { + // Height-constrained (receipt, document): 70% height, width from AR, centered + height = 70; + width = height * config.aspectRatio; + } + + return { + x: clampPct((100 - width) / 2), + y: clampPct((100 - height) / 2), + width: clampPct(width), + height: clampPct(height), + }; +} ``` ### Revised Code: Responsive Handle Sizes (CropTool.tsx) ```diff --- a/frontend/src/shared/components/CameraCapture/CropTool.tsx +++ b/frontend/src/shared/components/CameraCapture/CropTool.tsx @@ -5,7 +5,7 @@ import React, { useCallback, useState } from 'react'; -import { Box, IconButton, Button, Typography, CircularProgress } from '@mui/material'; +import { Box, IconButton, Button, Typography, CircularProgress, useTheme, useMediaQuery } from '@mui/material'; import CheckIcon from '@mui/icons-material/Check'; import RefreshIcon from '@mui/icons-material/Refresh'; @@ -15,6 +15,7 @@ import type { CropToolProps } from './types'; export const CropTool: React.FC<CropToolProps> = ({ imageSrc, lockAspectRatio = false, + initialCrop, aspectRatio, onConfirm, onReset, @@ -23,9 +24,14 @@ export const CropTool: React.FC<CropToolProps> = ({ }) => { const [isProcessing, setIsProcessing] = useState(false); + // Responsive handle sizing: 44px standard, 32px on small viewports + const theme = useTheme(); + const isCompactViewport = useMediaQuery('(max-height: 400px)'); + const handleConfig = { size: isCompactViewport ? 32 : 44, visual: isCompactViewport ? 12 : 16 }; + const { cropArea, isDragging, resetCrop, executeCrop, handleDragStart } = useImageCrop({ aspectRatio: lockAspectRatio ? aspectRatio : undefined, + initialCrop, }); ``` Handle component updated to use passed config instead of hardcoded values: ```diff --- a/frontend/src/shared/components/CameraCapture/CropTool.tsx +++ b/frontend/src/shared/components/CameraCapture/CropTool.tsx @@ -162,7 +167,7 @@ export const CropTool: React.FC<CropToolProps> = ({ {/* Move handle (center area) */} <CropHandleArea handle="move" onDragStart={handleDragStart} sx={{ position: 'absolute', - inset: 8, + inset: handleConfig.size / 2, cursor: 'move', }} @@ -173,14 +178,14 @@ export const CropTool: React.FC<CropToolProps> = ({ {/* Corner handles */} - <CropHandle handle="nw" onDragStart={handleDragStart} position="top-left" /> - <CropHandle handle="ne" onDragStart={handleDragStart} position="top-right" /> - <CropHandle handle="sw" onDragStart={handleDragStart} position="bottom-left" /> - <CropHandle handle="se" onDragStart={handleDragStart} position="bottom-right" /> + <CropHandle handle="nw" onDragStart={handleDragStart} position="top-left" config={handleConfig} /> + <CropHandle handle="ne" onDragStart={handleDragStart} position="top-right" config={handleConfig} /> + <CropHandle handle="sw" onDragStart={handleDragStart} position="bottom-left" config={handleConfig} /> + <CropHandle handle="se" onDragStart={handleDragStart} position="bottom-right" config={handleConfig} /> {/* Edge handles */} - <CropHandle handle="n" onDragStart={handleDragStart} position="top" /> - <CropHandle handle="s" onDragStart={handleDragStart} position="bottom" /> - <CropHandle handle="w" onDragStart={handleDragStart} position="left" /> - <CropHandle handle="e" onDragStart={handleDragStart} position="right" /> + <CropHandle handle="n" onDragStart={handleDragStart} position="top" config={handleConfig} /> + <CropHandle handle="s" onDragStart={handleDragStart} position="bottom" config={handleConfig} /> + <CropHandle handle="w" onDragStart={handleDragStart} position="left" config={handleConfig} /> + <CropHandle handle="e" onDragStart={handleDragStart} position="right" config={handleConfig} /> ``` CropHandle component accepts config prop: ```diff --- a/frontend/src/shared/components/CameraCapture/CropTool.tsx +++ b/frontend/src/shared/components/CameraCapture/CropTool.tsx @@ -284,6 +289,7 @@ interface CropHandleProps { handle: CropHandle; onDragStart: (handle: CropHandle, event: React.MouseEvent | React.TouchEvent) => void; + config: { size: number; visual: number }; position: | 'top-left' | 'top-right' @@ -297,8 +303,8 @@ interface CropHandleProps { } -const CropHandle: React.FC<CropHandleProps> = ({ handle, onDragStart, position }) => { - const handleSize = 24; - const handleVisualSize = 12; +const CropHandle: React.FC<CropHandleProps> = ({ handle, onDragStart, position, config }) => { + const handleSize = config.size; + const handleVisualSize = config.visual; const positionStyles: Record<string, React.CSSProperties> = { @@ -366,8 +372,8 @@ const CropHandle: React.FC<CropHandleProps> = ({ handle, onDragStart, position } <Box sx={{ - width: isCorner ? handleVisualSize : position === 'top' || position === 'bottom' ? 24 : 8, - height: isCorner ? handleVisualSize : position === 'left' || position === 'right' ? 24 : 8, + width: isCorner ? handleVisualSize : position === 'top' || position === 'bottom' ? handleSize * 0.7 : handleVisualSize * 0.75, + height: isCorner ? handleVisualSize : position === 'left' || position === 'right' ? handleSize * 0.7 : handleVisualSize * 0.75, backgroundColor: 'white', borderRadius: isCorner ? '50%' : 1, boxShadow: '0 2px 4px rgba(0,0,0,0.3)', ``` ### Updated Decision Log Entry | Decision | Reasoning Chain | | --- | --- | | Responsive handle sizes (44px/32px) | Apple HIG requires 44px minimum touch targets -> but VIN crop strip at 320px viewport height is only ~45px tall -> 44px handles would nearly fill entire strip -> use `useMediaQuery('(max-height: 400px)')` to detect compact viewports -> 32px handles on compact, 44px on standard -> maintains usability without overflowing | | Bounds clamping via clampPct | GUIDANCE_CONFIGS values could change in future -> unclamped arithmetic could produce negative or >100 percentages -> explicit clamping guards against invalid crop coordinates at trivial cost | | aspectRatio >= 1 (from > 1) | Edge case: 1:1 square guidance type -> `> 1` sends to height-constrained path which uses 70% height -> `>= 1` sends to width-constrained path using 85% width -> width-constrained is more appropriate for square guides on wide screens | ### Updated Test Scenarios (M1) Add to existing scenarios: - Edge: All CropArea values are within 0-100 range (validated by clampPct) - Edge: Hypothetical square guidance (AR=1) returns 85% width, 85% height centered ### Updated Known Risks | Risk | Mitigation | Anchor | | --- | --- | --- | | 320px viewport VIN handles | 32px responsive handles on compact viewports via useMediaQuery | CropTool.tsx (planned) | *Verdict*: AWAITING_REVIEW | *Next*: Re-review QR plan-completeness

egullickson commented 2026-02-08 01:59:21 +00:00

Author

Owner

Copy Link

Copy Source

Plan Review Results

Phase: Plan-Review | Agent: Multiple | Status: PASS

QR Plan-Completeness Re-Review: PASS

All three RULE 0/1 findings addressed. Plan structure, decision log, tests, and risks verified complete.

TW Plan-Scrub: PASS (with 2 minor corrections)

1. CameraCapture.tsx comment (intent leakage): "Crop initial position matches guidance overlay center" -> "Calculate crop coordinates centered on the guidance overlay area"
2. CropTool.tsx (missing WHY): Add comment for edge handle proportions: "Edge handles: 70% of touch target for horizontal bars, 75% for vertical bars"

Both corrections will be applied during execution.

QR Plan-Code: Addressed in revision

All RULE 0/1/2 findings resolved. No remaining blockers.

Verdict: PLAN APPROVED | Next: Create branch, begin execution

## Plan Review Results **Phase**: Plan-Review | **Agent**: Multiple | **Status**: PASS ### QR Plan-Completeness Re-Review: PASS All three RULE 0/1 findings addressed. Plan structure, decision log, tests, and risks verified complete. ### TW Plan-Scrub: PASS (with 2 minor corrections) 1. **CameraCapture.tsx comment** (intent leakage): "Crop initial position matches guidance overlay center" -> "Calculate crop coordinates centered on the guidance overlay area" 2. **CropTool.tsx** (missing WHY): Add comment for edge handle proportions: "Edge handles: 70% of touch target for horizontal bars, 75% for vertical bars" Both corrections will be applied during execution. ### QR Plan-Code: Addressed in revision All RULE 0/1/2 findings resolved. No remaining blockers. *Verdict*: PLAN APPROVED | *Next*: Create branch, begin execution

egullickson referenced this issue from a commit 2026-02-08 02:05:48 +00:00

feat: improve VIN camera crop overlay-to-crop alignment and touch targets (refs #123)

egullickson referenced a pull request that will close this issue 2026-02-08 02:06:08 +00:00

feat: Improve VIN photo capture camera crop (#123) #124

egullickson added

status

review

and removed

status

in-progress

labels 2026-02-08 02:06:15 +00:00

egullickson referenced this issue from a commit 2026-02-08 02:26:45 +00:00

fix: re-request camera stream on retake when tracks are inactive (refs #123)

egullickson closed this issue 2026-02-09 00:36:44 +00:00

egullickson referenced this issue from a commit 2026-02-09 00:36:45 +00:00

Merge pull request 'feat: Improve VIN photo capture camera crop (#123)' (#124) from issue-123-improve-vin-camera-crop into main

egullickson referenced this issue 2026-02-11 03:04:24 +00:00

feat: Expand OCR with fuel receipt scanning and owners manual maintenance extraction #129

egullickson referenced this issue 2026-02-11 03:04:45 +00:00

feat: Station matching from receipt (#129) #132

egullickson referenced this issue 2026-02-11 03:49:27 +00:00

feat: Station matching from receipt (#129) #141

egullickson referenced this issue 2026-02-11 15:45:36 +00:00

feat: Station matching from receipt (#129) #132

egullickson referenced this issue 2026-02-11 19:03:51 +00:00

feat: Station matching from receipt (#129) #141

egullickson referenced this issue from a commit 2026-02-11 21:27:47 +00:00

feat: add station matching from receipt merchant name (refs #132)

Sign in to join this conversation.

No Branch/Tag Specified

Branches Tags

main

No results found.

Labels

Clear labels

status

backlog

status/blocked

status

done

status

in-progress

status

ready

status

review

type

bug

type

chore

type

docs

type

feature

No labels

status

review

type

feature

Milestone

No items

No Milestone Sprint 2026-02-02

Projects

Clear projects

No projects

Assignees

Clear assignees

egullickson

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: egullickson/motovaultpro#123