Skip to content

fix(desktop): restore last confirmed capture area when reopening selector#1872

Open
ManthanNimodiya wants to merge 3 commits into
CapSoftware:mainfrom
ManthanNimodiya:fix/capture-area-restore-last-selection
Open

fix(desktop): restore last confirmed capture area when reopening selector#1872
ManthanNimodiya wants to merge 3 commits into
CapSoftware:mainfrom
ManthanNimodiya:fix/capture-area-restore-last-selection

Conversation

@ManthanNimodiya
Copy link
Copy Markdown
Contributor

@ManthanNimodiya ManthanNimodiya commented May 26, 2026

Problem

When a user had previously selected a capture area and reopened the area selector, it always started blank, forcing them to redraw their selection every time.
This was reported by multiple users in the community.

The root cause was in target-select-overlay.tsx: initialAreaBounds (which seeds the Cropper's starting position) was always initialized to undefined, even when an existing area target was already set.

Solution

Added an effectiveInitialBounds memo that falls back to the current captureTarget area bounds when no explicit initialAreaBounds is set.
Both initialCrop and shouldShowSelectionHint now use this memo, so:

  • Opening the selector with a previously confirmed area pre-loads those bounds
  • The "draw a selection" hint is suppressed when bounds are available
  • Re-entering area mode within the same session also restores correctly

Also fixed capture-area.tsx (the alternate area selection flow) with the same pattern, activeScreenId now handles both "display" and "area" capture target variants.

Test Plan

  • Confirm a capture area → reopen selector → previous selection is pre-loaded ✓
  • Adjust the pre-loaded area → close → reopen → shows updated bounds ✓
  • Switch to display mode and back to area mode within same session → bounds restored ✓
  • First-time use (no previous area) → blank selector with draw hint ✓

Greptile Summary

This PR fixes the blank-selector regression by pre-loading a previously confirmed capture area when the selector is reopened. Two complementary changes implement the fix.

  • target-select-overlay.tsx: A new effectiveInitialBounds memo falls back to options.captureTarget.bounds (area variant) when no explicit initialAreaBounds is set, and is wired into both initialCrop and shouldShowSelectionHint.
  • capture-area.tsx: A new activeScreenId memo handles both \"display\" and \"area\" variants, a frozen screenId signal guards against stale async updates, and initialCrop now falls back to rawOptions.captureTarget.bounds for the area variant — closing the inconsistency flagged in the previous review round.

Confidence Score: 5/5

Safe to merge — changes are narrowly scoped to restoring crop state and do not touch recording, persistence, or IPC logic.

Both files implement the fallback pattern correctly. computeInitialBounds in the Cropper is called imperatively (not inside a reactive computation), so the newly reactive effectiveInitialBounds memo cannot trigger unintended crop resets. The frozen screenId signal correctly guards against late-arriving async updates.

No files require special attention.

Important Files Changed

Filename Overview
apps/desktop/src/routes/target-select-overlay.tsx Adds effectiveInitialBounds memo that falls back to options.captureTarget.bounds for the area variant, wiring it into shouldShowSelectionHint and initialCrop. Cropper reads initialCrop imperatively (not reactively), so the new reactive dependency on options.captureTarget in the memo is safe.
apps/desktop/src/routes/capture-area.tsx Introduces activeScreenId memo (handles both "display" and "area" variants), a frozen screenId signal, and a fallback in initialCrop to rawOptions.captureTarget.bounds for the area variant. hasStoredSelection still only checks state.lastSelectedBounds and not the rawOptions.captureTarget area fallback, which can briefly show the draw-hint before the Cropper fires its first onCropChange.
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
apps/desktop/src/routes/capture-area.tsx:100-106
`hasStoredSelection` only checks `state.lastSelectedBounds`, but that store is written exclusively by `capture-area.tsx`'s own `handleConfirm`. If the user previously confirmed an area through `target-select-overlay.tsx`, `lastSelectedBounds` has no entry and `hasStoredSelection()` stays `false`, so `showSelectionHint` can briefly flash "draw a selection" during the Cropper's initialization window (before `onCropChange` fires and `crop()` reflects the restored bounds). Adding the same `rawOptions.captureTarget` area fallback already used in `initialCrop` prevents the flash.

```suggestion
	const hasStoredSelection = createMemo(() => {
		const id = screenId();
		if (!id) return false;
		if (state.lastSelectedBounds?.some((entry) => entry.screenId === id))
			return true;
		const target = rawOptions.captureTarget;
		if (target.variant === "area" && target.screen === id) return true;
		return false;
	});
```

Reviews (2): Last reviewed commit: "fix(desktop): address review — untrack s..." | Re-trigger Greptile

@superagent-security superagent-security Bot added contributor:verified Contributor passed trust analysis. pr:verified PR passed security analysis. labels May 26, 2026
Comment thread apps/desktop/src/routes/capture-area.tsx
Comment thread apps/desktop/src/routes/capture-area.tsx
@ManthanNimodiya
Copy link
Copy Markdown
Contributor Author

@richiemcilroy, have a look when you get chance and lmk for any changes

@ManthanNimodiya
Copy link
Copy Markdown
Contributor Author

@greptileai please re-review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor:verified Contributor passed trust analysis. pr:verified PR passed security analysis.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant