fix: switch to active action data tab on capture #861
Conversation
WalkthroughUnified per-item editing state and handlers in the InterpretationLog component; consolidated list/text/screenshot edit flows and tab-switching logic using a single getLatestCaptureType() decision and generalized startEdit/saveEdit/cancelEdit flows. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as InterpretationLog UI
participant EditState as Unified Editing State
participant Store as Local State (steps/tabs)
participant Backend as Backend / Emitter
rect rgb(235,245,255)
UI->>EditState: startEdit(stepId, type, currentValue)
EditState-->>UI: isEditing=true (controls input render)
end
rect rgb(240,255,240)
UI->>EditState: saveEdit() (onBlur / Enter)
EditState->>Store: updateStepName(type, stepId, value)
Store-->>UI: updated step data
EditState->>Backend: emitForStepId(stepId, action)
EditState-->>UI: isEditing=null
end
rect rgb(255,245,235)
UI->>EditState: cancelEdit() (onEscape / blur)
EditState-->>UI: revert value, isEditing=null
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/run/InterpretationLog.tsx(5 hunks)
🔇 Additional comments (2)
src/components/run/InterpretationLog.tsx (2)
391-399: LGTM!The
getLatestCaptureTypehelper function correctly identifies the most recent capture type by iterating backward throughbrowserSteps. The implementation is clean and straightforward.
403-434: Nice refactoring of tab-switching logic!The unified approach using
getLatestCaptureType()is cleaner than the previous separate conditionals. The implementation correctly:
- Determines the most recent capture type
- Switches to the appropriate tab based on that type
- Preserves the existing auto-focus behavior for screenshots with proper timing delays
| const screenshotSteps = browserSteps.filter(step => step.type === "screenshot"); | ||
| const latestScreenshotStep = screenshotSteps[latestIndex]; | ||
| if (latestScreenshotStep) { | ||
| const screenshotName = latestScreenshotStep.name || `Screenshot ${latestIndex + 1}`; | ||
| handleStartEditScreenshotName(latestScreenshotStep.id, screenshotName); | ||
| startEdit(latestScreenshotStep.id, 'screenshot', screenshotName); | ||
| } |
There was a problem hiding this comment.
Fix screenshot auto-edit targeting.
latestIndex is derived from screenshotData, which only includes steps where step.screenshotData exists. However, the follow-up lookup builds screenshotSteps from all screenshot steps (including ones without data), so the indexes can drift and we end up renaming the wrong step—or failing to rename at all—whenever a screenshot action is recorded before its payload arrives. Filter the steps the same way you populated screenshotData before indexing.
Apply this diff to keep the arrays aligned:
- const screenshotSteps = browserSteps.filter(step => step.type === "screenshot");
- const latestScreenshotStep = screenshotSteps[latestIndex];
+ const screenshotStepsWithData = browserSteps.filter(
+ (step): step is { id: number; name?: string; type: 'screenshot'; screenshotData?: string } =>
+ step.type === "screenshot" && step.screenshotData
+ );
+ const latestScreenshotStep = screenshotStepsWithData[latestIndex];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const screenshotSteps = browserSteps.filter(step => step.type === "screenshot"); | |
| const latestScreenshotStep = screenshotSteps[latestIndex]; | |
| if (latestScreenshotStep) { | |
| const screenshotName = latestScreenshotStep.name || `Screenshot ${latestIndex + 1}`; | |
| handleStartEditScreenshotName(latestScreenshotStep.id, screenshotName); | |
| startEdit(latestScreenshotStep.id, 'screenshot', screenshotName); | |
| } | |
| const screenshotStepsWithData = browserSteps.filter( | |
| (step): step is { id: number; name?: string; type: 'screenshot'; screenshotData?: string } => | |
| step.type === "screenshot" && step.screenshotData | |
| ); | |
| const latestScreenshotStep = screenshotStepsWithData[latestIndex]; | |
| if (latestScreenshotStep) { | |
| const screenshotName = latestScreenshotStep.name || `Screenshot ${latestIndex + 1}`; | |
| startEdit(latestScreenshotStep.id, 'screenshot', screenshotName); | |
| } |
🤖 Prompt for AI Agents
In src/components/run/InterpretationLog.tsx around lines 362 to 367, the code
builds screenshotSteps from all steps but latestIndex was computed from
screenshotData (only steps with step.screenshotData), causing index drift; fix
by constructing screenshotSteps using the same filter used for screenshotData
(i.e., only include steps with step.screenshotData) so latestIndex maps to the
correct element, then use that filtered array to pick latestScreenshotStep and
call startEdit as before.
Summary by CodeRabbit