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

Download percentage added #356

Merged
merged 2 commits into from
Aug 17, 2020
Merged

Conversation

tusharjindal353
Copy link
Contributor

@tusharjindal353 tusharjindal353 commented Aug 17, 2020

Added the percentage text in sizeDateLabel in Download list collection cell
Fixes #347
@cpg Please review this

let size = offlineFile.getFileSize()
let date = offlineFile.downloadDate?.asString ?? ""
let progress = offlineFile.progress * 100
let dateSizeText = "\(Int(progress))% | \(size), \(date)"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution. I would suggest a change to this line for the following reasons:

  • The date does not matter much while downloading
  • The line keeps on moving because the % is variable font and has variable digits
  • The | keeps it a little busy

I would suggest

let dateSizeText = "\(size) · \(Int(progress))%"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cpg Thanks for your valuable feedback

1 I will not append the date while downloading.
2 I will add the percentage in last
3 i will add "." instead of "|"

Is this right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a precise suggestion of change for one line. If you like it, do it, if not, explain why and suggest otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cpg I liked your suggestion.
I am doing that way.

@cpg cpg merged commit fe301d2 into amahi:beta Aug 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show size and percentage downloaded when downloading a file
2 participants