fix(desktop): restore last confirmed capture area when reopening selector#1872
Open
ManthanNimodiya wants to merge 3 commits into
Open
fix(desktop): restore last confirmed capture area when reopening selector#1872ManthanNimodiya wants to merge 3 commits into
ManthanNimodiya wants to merge 3 commits into
Conversation
…nitialCrop to captureTarget bounds
Contributor
Author
|
@richiemcilroy, have a look when you get chance and lmk for any changes |
Contributor
Author
|
@greptileai please re-review |
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
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.
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 toundefined, even when an existing area target was already set.Solution
Added an
effectiveInitialBoundsmemo that falls back to the currentcaptureTargetarea bounds when no explicitinitialAreaBoundsis set.Both
initialCropandshouldShowSelectionHintnow use this memo, so:Also fixed
capture-area.tsx(the alternate area selection flow) with the same pattern,activeScreenIdnow handles both"display"and"area"capture target variants.Test Plan
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 neweffectiveInitialBoundsmemo falls back tooptions.captureTarget.bounds(area variant) when no explicitinitialAreaBoundsis set, and is wired into bothinitialCropandshouldShowSelectionHint.capture-area.tsx: A newactiveScreenIdmemo handles both\"display\"and\"area\"variants, a frozenscreenIdsignal guards against stale async updates, andinitialCropnow falls back torawOptions.captureTarget.boundsfor 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.
computeInitialBoundsin the Cropper is called imperatively (not inside a reactive computation), so the newly reactiveeffectiveInitialBoundsmemo cannot trigger unintended crop resets. The frozenscreenIdsignal correctly guards against late-arriving async updates.No files require special attention.
Important Files Changed
effectiveInitialBoundsmemo that falls back tooptions.captureTarget.boundsfor the area variant, wiring it intoshouldShowSelectionHintandinitialCrop. Cropper readsinitialCropimperatively (not reactively), so the new reactive dependency onoptions.captureTargetin the memo is safe.activeScreenIdmemo (handles both "display" and "area" variants), a frozenscreenIdsignal, and a fallback ininitialCroptorawOptions.captureTarget.boundsfor the area variant.hasStoredSelectionstill only checksstate.lastSelectedBoundsand not therawOptions.captureTargetarea fallback, which can briefly show the draw-hint before the Cropper fires its firstonCropChange.Prompt To Fix All With AI
Reviews (2): Last reviewed commit: "fix(desktop): address review — untrack s..." | Re-trigger Greptile