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

feat: Show download progress for installing Northstar #200

Merged
merged 39 commits into from
Apr 10, 2023

Conversation

GeckoEidechse
Copy link
Member

@GeckoEidechse GeckoEidechse commented Mar 3, 2023

Still WIP. The end goal would be to have a way to show download and install progress of Northstar.

Right now we do not show any progress which is frustrating to anyone where the install takes longer than a few seconds.

TODO:

  • i18n

This adds a change needed for making progressbar work on dowload
Simply prints it to JavaScript console for now
@GeckoEidechse
Copy link
Member Author

Currently this is done using events which is fine but I need to pass the window handle all the way down to the install function which is annoying. If I could somehow instead grab the default window or something similar that doesn't require me to pass the window object all the way down, that would be great ^^

@GeckoEidechse GeckoEidechse added the enhancement New feature or request label Mar 14, 2023
@GeckoEidechse
Copy link
Member Author

Okay, I think I got the basics figured out. From here I "only" need to refactor and settle on a format for transmitting data...

@Alystrasz I assume I just transfer the raw info, i.e. downloaded bytes and total size and then do the calculation on the frontend?

@GeckoEidechse GeckoEidechse mentioned this pull request Mar 20, 2023
100 tasks
@Alystrasz
Copy link
Contributor

@Alystrasz I assume I just transfer the raw info, i.e. downloaded bytes and total size and then do the calculation on the frontend?

Yep you only need those two values to display download progress in UI :)

Do you plan to distinguish between archive downloading and extraction?
(I had the issue with mods autodownload where extraction would take more time than downloading)

@GeckoEidechse
Copy link
Member Author

Do you plan to distinguish between archive downloading and extraction? (I had the issue with mods autodownload where extraction would take more time than downloading)

Yes. I guess the way to go would be to also supply an enum in the message that indicates the state i.e. DOWNLOADING vs EXTRACTING? ^^

@Alystrasz
Copy link
Contributor

Do you plan to distinguish between archive downloading and extraction? (I had the issue with mods autodownload where extraction would take more time than downloading)

Yes. I guess the way to go would be to also supply an enum in the message that indicates the state i.e. DOWNLOADING vs EXTRACTING? ^^

That's what I ended up doing, yep

@GeckoEidechse
Copy link
Member Author

Yes. I guess the way to go would be to also supply an enum in the message that indicates the state i.e. DOWNLOADING vs EXTRACTING? ^^

That's what I ended up doing, yep

Alright, should be implemented now. For now it simply does a console.log. I suppose you'd be willing to take it from here and look at the front-end part of it? ^^"

image

@Alystrasz
Copy link
Contributor

Why do you only have 3 download updates?
No extraction updates at all? :(

@GeckoEidechse
Copy link
Member Author

GeckoEidechse commented Mar 20, 2023

Why do you only have 3 download updates?

It sends an update every 250ms. However if you have a 1gbps downlink, that means that those 70MB of Northstar zip get downloaded in less than a second, i.e. only 3 updates sent out before download is done xD

No extraction updates at all? :(

Don't know yet how to do progress tracking of the extraction unfortunately so I can only emit starting (and done if wanted). Need to learn more Rust before I know how to do callbacks from extract but that's gonna take a few weeks based on how much time I can invest into this atm :/

@GeckoEidechse
Copy link
Member Author

Tried a bit with implementing a progress bar myself but I'm failing on actually listening to the event in the right Vue component... ^^"

@GeckoEidechse
Copy link
Member Author

Aight, I got some rudimentary progress bar done now. Only issue is that CSS seems to be squishing it to the point that it becomes invisible but I have no idea how to fix that...

@GeckoEidechse GeckoEidechse requested a review from Alystrasz April 2, 2023 16:40
@GeckoEidechse
Copy link
Member Author

Manually specified progress bar size via CSS now in 4449a4d

Feels a bit like a hack, probably also is one, but it works and tbh we really need to get a version with progress bar out cause it's the only feature that Viper has that we still don't have ^^

Copy link
Contributor

@Alystrasz Alystrasz left a comment

Choose a reason for hiding this comment

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

image

Progress bar is too close to play button.

src-tauri/src/lib.rs Outdated Show resolved Hide resolved
src-tauri/src/lib.rs Show resolved Hide resolved
src-vue/src/components/PlayButton.vue Outdated Show resolved Hide resolved
src-vue/src/components/PlayButton.vue Outdated Show resolved Hide resolved
src-vue/src/components/PlayButton.vue Outdated Show resolved Hide resolved
src-vue/src/components/PlayButton.vue Outdated Show resolved Hide resolved
src-vue/src/views/RepairView.vue Outdated Show resolved Hide resolved
src-vue/src/components/PlayButton.vue Outdated Show resolved Hide resolved
src-vue/src/components/PlayButton.vue Outdated Show resolved Hide resolved
instead of 250ms

Gives impression of faster / more fluent download.
@Alystrasz Alystrasz self-assigned this Apr 10, 2023
@Alystrasz
Copy link
Contributor

@GeckoEidechse I reworked your PR a bit, feel free to review again (I can't request your review through GitHub since you're the PR author).

@GeckoEidechse
Copy link
Member Author

@GeckoEidechse I reworked your PR a bit, feel free to review again (I can't request your review through GitHub since you're the PR author).

Looks good so far, many thanks <3

I'll try to address the remaining issues now. Maybe I'll come across anything in the process :P

@GeckoEidechse
Copy link
Member Author

Alright, done again from my side now. Back to you for review :P

@GeckoEidechse GeckoEidechse requested a review from Alystrasz April 10, 2023 12:19
Copy link
Contributor

@Alystrasz Alystrasz left a comment

Choose a reason for hiding this comment

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

Comments have been addressed, feature works as expected on Ubuntu.

@GeckoEidechse GeckoEidechse merged commit 2be2ef6 into main Apr 10, 2023
@Alystrasz Alystrasz deleted the feat/northstar-install-progressbar branch April 10, 2023 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants