-
Notifications
You must be signed in to change notification settings - Fork 11
feat: added transferring progress indicator with error handling. #75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Reviewer's GuideThis PR augments the NFC image transfer workflow by adding fine-grained progress and error callbacks inside the Protocol layer and introducing a modal TransferProgressDialog in the UI to visualize NFC detection, ongoing transfer progress, and failure or success states with animations. Sequence diagram for image transfer with progress and error handlingsequenceDiagram
actor User
participant ImageEditor
participant TransferProgressDialog
participant Protocol
participant Device
User->>ImageEditor: Tap 'Transfer' button
ImageEditor->>TransferProgressDialog: Show dialog
TransferProgressDialog->>Protocol: writeImages(image, onProgress, onTagDetected)
Protocol->>TransferProgressDialog: onProgress(0.0, "Waiting for NFC tag...")
Protocol->>Device: NFC poll
Device-->>Protocol: Tag detected
Protocol->>TransferProgressDialog: onTagDetected()
Protocol->>TransferProgressDialog: onProgress(0.1, "Tag detected! Initializing...")
loop For each frame
Protocol->>TransferProgressDialog: onProgress(progress, status)
Protocol->>Device: Write frame chunk
Device-->>Protocol: Ack
end
Protocol->>TransferProgressDialog: onProgress(0.95, "Refreshing display...")
Protocol->>Device: Refresh display
Protocol->>TransferProgressDialog: onProgress(1.0, "Transfer complete!")
TransferProgressDialog->>User: Show success or error
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Dhruv1797 - I've reviewed your changes - here's some feedback:
- The TransferProgressDialog widget is very large—consider breaking it into smaller sub-widgets or moving parts into their own files to improve readability and maintainability.
- You should extract the hard-coded status messages in Protocol.writeImages into constants or a localization layer to keep them consistent and support future i18n.
- Instead of exposing raw exception strings in the dialog, catch specific errors in writeImages and map them to more user-friendly messages for better UX.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The TransferProgressDialog widget is very large—consider breaking it into smaller sub-widgets or moving parts into their own files to improve readability and maintainability.
- You should extract the hard-coded status messages in Protocol.writeImages into constants or a localization layer to keep them consistent and support future i18n.
- Instead of exposing raw exception strings in the dialog, catch specific errors in writeImages and map them to more user-friendly messages for better UX.
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
@Vishveshwara @kienvo @AsCress @Jhalakupadhyay please have a look and review it once. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Dhruv1797 , please check the comments I have shared
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
It would be nice if we had something like a dialog telling users to hold their phone still after the transfer is done, to wait for the display to refresh.
Currently, there is no way to read the busy state of the display, so deciding to pull away the phone is up to the user's observation.
@Dhruv1797 we just need one progress indicator take the circular one and remove the bottom linear progress indicator. |
update: added the diplay refreshing state for average of 15sec duration, removed linear loader and user friendly error handling Here is the updated demo video: WhatsApp.Video.2025-07-01.at.12.45.57.PM.mp4@kienvo @Jhalakupadhyay @Vishveshwara please review the changes once. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks very good, well done ! can you please share a video of you transferring a small image , like a canvas drawing.
Here is the demo video of transferring small image from canvas drawing: WhatsApp.Video.2025-07-02.at.3.03.37.PM.mp4@Vishveshwara please check it once. |
Looks good , but at 24th second why is it reading the nfc tag ? is it by mistake |
@Vishveshwara actually, it started reading the NFC tag since I used it for NDEF operations earlier. Maybe it still has some data in its memory, which is why it’s being read now. There are no changes from the code side. |
It's Android's system service, probably has higher priority than the app. |
Fixes: #64
added a smooth progress indicator for image transfers with better error handling. Now users can easily track transfer progress and understand any issues if they occur.
Here is the demo video:
VID_20250627_170057529.mp4
Summary by Sourcery
Implement real-time progress reporting and user feedback during E-Paper image transfers by extending the Protocol layer with progress callbacks and introducing a dedicated transfer dialog in the UI that handles animations, status updates, and error/success states.
New Features:
Enhancements: