Skip to content
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

Added progress bar for electron #1266

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

varun-s22
Copy link
Collaborator

  • Added progress on the icon to show progress of files downloaded.
  • Dialog box for asking file location is switched on by default.
  • File location where the file is downloaded is opened by default.

What's this PR do?
This PR is in reference to #1252 , where the user gets no notification about the file downloaded. This is currently Work in Progress, as it requires a review.

Any background context you want to provide?
I used electron's native progress-bar to show the progress made on downloading. Since I didn't have the size of files downloaded, I used kind of a "hacky" way to deal out of it.
If i could get the progress of how much file is being downloaded (i.e x MB/ y MB) format, I can show a real-time progress bar

Also this is currently below the

Screenshots?
It currently looks like this
Screenshot 2023-01-21 at 02 58 00

I have tested this PR on:

  • Windows
  • Linux/Ubuntu
  • macOS

Future: I'm looking for external packages in TypeScript, which provides a progress-bar in the UI, so i can render a component on the screen itself.

* Added progress on the icon to show progress of files downloaded.
* Dialog box for asking file location is switched on by default.
* File location where the file is downloaded is opened by default.
@varun-s22
Copy link
Collaborator Author

@akashnimare @andersk @vsvipul Please if you could review this PR, it would be great.

Fixed some bugs
Refactored code
Works well on mac-os
@zulipbot zulipbot added size: M and removed size: S labels Jan 23, 2023
@varun-s22 varun-s22 changed the title [WIP] Added progress bar for electron Added progress bar for electron Jan 23, 2023
@varun-s22
Copy link
Collaborator Author

Screen.Recording.2023-01-23.at.21.49.55.mov

This is how it currently looks. Look at the bottom of the dock icon, you can see the progress bar.

@alexmv
Copy link
Collaborator

alexmv commented Jan 23, 2023

Thanks for the contribution! This looks to be just getting asymptotically close to 100%, with no notion of how long the downlaod actually will take. This may actually be more confusing for users, since they may be wondering why a big file is "stuck."

The download provides a Content-Length response header on it, which tells you how big the file is. You should be able to use this to make the progress bar accurate.

Updated progress bar, to show live progress.
Progress is calculated by recivedBytes/totalBytes.
@varun-s22
Copy link
Collaborator Author

varun-s22 commented Jan 24, 2023

Hey @alexmv, thanks for the review. I updated the file now. I didn't use Content-Length, as i couldn't pass the headers into WebContents. I had to change the entire downloading scheme, and use fetch for using the above.
Instead I found a workaround, I explored the downloadItem in electron, and found two member functions which are useful for us, namely getTotalBytes(), and getReceivedBytes(), which solves our problem, and updates the progress bar live with the downloading of the file.

I hope now it works fine, and gets merged

@varun-s22
Copy link
Collaborator Author

This fixes #1252

@varun-s22
Copy link
Collaborator Author

hey @alexmv @andersk @timabbott, will you please review this PR?

@zulipbot
Copy link
Member

Heads up @varun-s22, we just merged some commits that conflict with the changes you made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/main branch and resolve your pull request's merge conflicts accordingly.

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

Successfully merging this pull request may close these issues.

3 participants