feat: show progress and outcome for OCR capture - #505
Conversation
8c8c77b to
73da124
Compare
|
Hi @vxirau, love to see your PR again! and thanks for the PR and for taking the time on this! 🙏 One thing though — OCR progress and outcome feedback is already implemented on the app side: while the user is running OCR, the state is surfaced through Snapzy's menubar icon. So this change would end up duplicating that behavior with a second progress indicator. Could we drop the progress/outcome UI from this PR and rely on the existing menubar feedback instead? If you feel the menubar indication isn't clear enough in some cases, happy to discuss improving that path directly rather than adding a parallel one. |
An OCR capture reported nothing between mouse-up and the result landing on the clipboard except the menu-bar spinner, so a slow recognition read as a freeze. A progress toast now appears synchronously on mouse-up, before the first async hop. It is shown only after the mouse-up snapshots are secured, so the panel can never end up in the captured pixels. One label covers the whole operation. Relabelling once the image was in hand sounds better but is not: the normal path only crops the snapshot already taken at mouse-up, which finishes well inside the toast's own fade -in, so the first label never became visible. The toast also resolves into a terminal state rather than being dismissed. `OCRResultNotifier` prefers a native notification and deliberately shows no toast when one is delivered, and it reports from a detached task that resolves after the capture scope exits — so a plain dismiss always won the race and the spinner vanished into nothing. Success, no-text, unsupported- QR and failure now each land on a visible outcome. The notification behaviour is unchanged.
9f4cb16 to
6452ebe
Compare
|
Thanks for the review! Honestly, I had no idea the menubar icon showed progress during OCR. I have "Show menu bar icon" turned off in Preferences > General, so I've never actually seen that feedback, which is why I assumed there was nothing there at all. My bad for not checking that first. But digging into it, I think that's the real gap here. With So instead of a second indicator, what about only showing the toast when the menubar icon is hidden? Something like: let progressToast = AppStatusBarController.shared.isMenuBarIconVisible
? nil
: AppToastManager.shared.show(...)If the icon is visible, nothing changes at all and the spinner stays the only surface. If it's hidden, the toast covers it instead. They'd never both show up, so no duplication. Happy to adjust if you'd rather solve it another way, or to scope this down to just the progress part and drop the outcome states. Let me know what you think! |
duongductrong
left a comment
There was a problem hiding this comment.
Thanks @vxirau for adjusting the implementation! Checking AppStatusBarController.shared.isMenuBarIconVisible makes complete sense and nicely solves the hidden menu bar icon case without duplicating feedback indicators. The code and localization changes look very clean.
Approving this PR now — I won't merge it immediately just yet as we are organizing the upcoming release cycle, but will merge it during the next release batch. Thanks again for your great contribution! 🙏
|
No problem @duongductrong ! ;) |
An OCR capture reports nothing between mouse-up and the text landing on the clipboard except the menu-bar spinner, so a slow recognition reads as a freeze.
Progress
A toast now appears synchronously on mouse-up, before the first async hop. It is shown only after the mouse-up snapshots are secured, so the panel can never end up in the captured pixels.
One label covers the whole operation. Relabelling once the image is in hand sounds better but is not: the normal path only crops the snapshot already taken at mouse-up, which finishes well inside the toast's own fade-in, so the first label never became visible.
Outcome
The toast resolves into a terminal state rather than being dismissed.
OCRResultNotifierprefers a native notification and deliberately shows no toast when one is delivered, and it reports from a detached task that resolves after the capture scope exits - so an unconditional dismiss always won the race and the spinner vanished into nothing.Each exit now lands on a visible outcome:
Notification behaviour is unchanged - this only adds the in-app confirmation for users who miss it, or who have Focus enabled and never receive it.
Notes
Reuses the existing
AppToastManagerspinner presentation already used by the Annotate editor's OCR, so no new UI. Adds one string,ocr.capturing-content, localized across all ten supported languages.