Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
| const handleDecode = async (input: string): Promise<boolean> => { | ||
| const selectDestinationResult = await selectDestination(input); | ||
| if (!selectDestinationResult.success) { | ||
| // TODO: implement this https://github.com/MakePrisms/agicash/pull/331#discussion_r2024690976 |
There was a problem hiding this comment.
I took a different approach to this. Now the scanner keeps scanning if there's an error and I show the toast. We needed to add the cooldown to the qr-scanner otherwise if there's an error it will continue decoding and erroring and spamming the toasts
| setCurrentFragment(''); | ||
| decodeRef.current(decoded); | ||
| scanner.current?.destroy(); | ||
| await handleDecodeRef.current(decoded); |
There was a problem hiding this comment.
I wonder if this approach is wrong. Correct me if I am wrong, but every time after successful decode we are navigating to another route? If so, maybe we should destroy the scanner on component unmount instead of after decode. In that case the scanner would just be concerned with scanning and if you want to stop scanning you just need to unmount the component
There was a problem hiding this comment.
yea good point. I changed things a bit so that now it only destroy on unmount which is nice because we can remove the scannerRef
There was a problem hiding this comment.
I feel like this can be even simpler. How does useAnimatedQRDecoder work? Lets forget other things for a bit now and say that camera is on and you just have conosle.log here in onDecode. Does it keep firing as long as you keep scanning the qr code?
There was a problem hiding this comment.
yea. It fires everytime it decodes and is constantly trying to decode. So the logs will be spaced apart by however long it takes to decode
gudnuf
force-pushed
the
qr-scanner-error-handling
branch
from
99ff4c5 to
78cbaa0
Compare
gudnuf
force-pushed
the
qr-scanner-error-handling
branch
from
78cbaa0 to
b9314d0
Compare
There was a problem hiding this comment.
I've been doing this a lot, so I created this skill. I iterated on it a couple times, but the fisrt version of it gave me this output below when I started addressing your feedback. This was a super simple PR, so I'll keep iterating on this skill as I use it. I also started a pr-feedback-final-check skill that starts a fresh context and makes sure all comments were properly addressed and that no new issues were introduced, but I need to work on that one some more.
There was a problem hiding this comment.
I tried to make it reflect how our review process works, so lmk if there's anything you think that should be different. We may also need to change this or create a new one so that we can give it to claude in github to address our feedback, but I've been finding the most success with just doing it locally so its optimized for that
There was a problem hiding this comment.
one thing we should also instruct claude to do (I am not sure if skill is the way to do it or something else), is to make a temp test and verify some behavior when not sure. I caught him few times doing something that it wasn't sure about and then I would tell it fi you can't find make a small test and lets see how it works and that worked pretty well. would be great if it could do that on its own
| variant: 'destructive', | ||
| }); | ||
| return; | ||
| throw new Error('Invalid cashu token'); |
There was a problem hiding this comment.
not sure about this usage. I wanted the qr-scanner to be able to handle the error cooldown, but I didn't want it do toasts because that seems like something the caller should do. So the way this is now, the caller can toast if they want, then they throw an error to signal to the scanner that it should pause calling onDecode until the cooldown.
Another idea I had was make these components handle the cooldown and not toast until after the cooldown expires.
wdyt? previously, I had this returning false to tell the scanner an error occurred but that seemed a bit wrong
- Keep scanner running after validation errors instead of showing blank screen - Deduplicate rapid scan events to prevent toast spam while QR is held up - Show specific error messages for DomainErrors in send scanner - Rename receive scan component to receive-scanner for consistency
jbojcic1
force-pushed
the
qr-scanner-error-handling
branch
from
b9314d0 to
62ed12f
Compare
2 participants
Summary
🤖 Assisted by Claude Code