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

External Command or Message to update the UI? #230

Closed
Folyd opened this issue Mar 21, 2020 · 15 comments
Closed

External Command or Message to update the UI? #230

Folyd opened this issue Mar 21, 2020 · 15 comments
Labels
question Further information is requested
Milestone

Comments

@Folyd
Copy link
Contributor

Folyd commented Mar 21, 2020

I'm a newbie to Iced and don't familiar with The Elm Architecture neither. I found a scenario that is hard even infeasible for the current architecture to achieve. (Maybe I'm wrong or something I missed?)

Here is the scenario:

We have a Download button, the on-press event is to trigger an async download task. In the meanwhile, when the download task started, the Download button should display the percent progress ( 0% -> 1% ->... -> 100%).

Below is the download function, which takes a progress_callback to notify the current progress.

use bytes::BufMut;
use futures_util::StreamExt;
use reqwest;

pub struct Downloader;

impl Downloader {
    pub async fn download<F>(
        url: &str,
        progress_callback: F,
    ) -> std::result::Result<Vec<u8>, Box<dyn std::error::Error>>
    where
        F: Fn(usize),
    {
        let mut bytes: Vec<u8> = vec![];
        let response = reqwest::get(url).await?;
        let total_size = response.content_length().unwrap() as f64;
        let mut stream = response.bytes_stream();
        while let Some(ret) = stream.next().await {
            let b = ret?;
            bytes.put(b);
            progress_callback(((100 * bytes.len()) as f64 / total_size) as usize);
        }
        Ok(bytes)
    }
}

#[cfg(test)]
mod test {
    use std::error::Error;

    use tokio;

    use super::*;

    #[tokio::test]
    async fn test_download() {
        match Downloader::download(
            "https://a-large-file-url",
            |progress| {
                println!("{}", progress);
            },
        )
        .await
        {
            Ok(bytes) => assert!(bytes.len() > 0),
            Err(e) => panic!("Download failed... {}", e.to_string()),
        }
    }
}

Use Subscription or Custom Widget? Well, I really have no idea how to achieve this. Does it mean we need a kind of external Command or Message to support this?

@hecrj
Copy link
Member

hecrj commented Mar 21, 2020

I believe @Songtronix has a similar use case in Airshipper. Maybe they can share some ideas!

In any case, you should be able to model this properly using a Subscription.

You will need to implement the Recipe trait for your Downloader. For this, instead of returning a future, you will need to return a Stream of download events (download started, progressed, finished...).

Then, you can obtain a Subscription from a Recipe by using Subscription::from_recipe.

Finally, you should return this subscription in Application::subscription until the download is finished. The messages produced by the stream will be fed to update, allowing you to change your application state accordingly.

The stopwatch example implements a custom subscription and may serve you as a guide.

@hecrj hecrj added the question Further information is requested label Mar 21, 2020
@hecrj hecrj added this to the 0.1.0 milestone Mar 21, 2020
@Songtronix
Copy link
Contributor

I bet this use case will happen quite often. Anything against adding an example? I would greatly do so 😄

@hecrj
Copy link
Member

hecrj commented Mar 21, 2020

@Songtronix I think it could be a great example.

We'll need to decide what kind of file to download and what do we do with it to prove it has been downloaded correctly.

We could also fake the download in the Subscription and simply showcase how to fire multiple messages asynchronously.

Up to you! Nothing against it for sure. I'd love a PR with a nice example of this.

@Folyd
Copy link
Contributor Author

Folyd commented Mar 21, 2020

Great! Thanks @hecrj @Songtronix .

@Songtronix
Copy link
Contributor

I got everything else so far but damn fiddling with streams is quite a pain if you never have worked with them. Can't wait for generators to land in rust.

@Folyd
Copy link
Contributor Author

Folyd commented Mar 23, 2020

Hey @Songtronix. Which HTTP client are you using in your example? reqwest or isahc? I use reqwest, but it's so hard to get through. I found you used and isahc::Metrics in your Airshipper to get the download progress, which is awesome. 👍

@Songtronix
Copy link
Contributor

Yes. It's really awesome they provide these metrics but it doesn't work that nicely with iced. I currently tick manually to update the screen but also any mouse movement will trigger the update too which looks kinda funny if the download progress smoothes out while moving your mouse ^^ Funny thing is you need to watch out to not call the metrics too often. It will slow down the download in my experience.
Anyway working on a proper example:ok_hand:

@Songtronix
Copy link
Contributor

Initial implementation is here: #232
However I do not feel that satisified with the result right now.

@Folyd
Copy link
Contributor Author

Folyd commented Mar 23, 2020

@Songtronix Great job! Thanks again!

@hecrj
Copy link
Member

hecrj commented Mar 24, 2020

@Folyd
Copy link
Contributor Author

Folyd commented Mar 24, 2020

@hecrj Cool, I already adopt this pattern and it works charmingly in my Iced project. 👍

@hecrj
Copy link
Member

hecrj commented Mar 24, 2020

Awesome! We can close this then :)

@hecrj hecrj closed this as completed Mar 24, 2020
@Folyd
Copy link
Contributor Author

Folyd commented Mar 24, 2020

However, I got two new questions.

  • It's possible to cancel the downloading task?
  • It's possible to support concurrent download tasks for multiple files?

I have tried to support the second one, but it not working. Only the first task updates its progress UI, the rest tasks don't.

Here is part of my code:

#[derive(Debug)]
enum Main {
    Loading,
    Loaded(State),
}

#[derive(Debug, Clone, Default)]
struct State {
    download_urls: Vec<String>,
}

#[derive(Debug, Clone)]
#[non_exhaustive]
enum Message {
    StartDownload(String),
    DownloadProgressed((String, Progress)),
}

impl Application for Main {
    type Executor = iced::executor::Default;
    type Message = Message;

        fn subscription(&self) -> Subscription<Self::Message> {
        match self {
            Main::Loading => Subscription::none(),
            Main::Loaded(State { download_urls, .. }) => {
                Subscription::batch(download_urls.iter().map(|url| {
                    Subscription::from_recipe(Download {
                        url: url.to_string(),
                    })
                    .map(Message::DownloadProgressed)
                }))
            }
        }
    }
}

Any idea on those topics?

@hecrj
Copy link
Member

hecrj commented Mar 24, 2020

It's possible to cancel the downloading task?

Yes, simply stop returning the Subscription of a particular URL in Application::subscription (in your example, removing it from download_urls should do the trick).

It's possible to support concurrent download tasks for multiple files?

Also yes. The current example has a bug in the Recipe::hash implementation. We should be hashing the url too. I just fixed it in master.

@Folyd
Copy link
Contributor Author

Folyd commented Mar 24, 2020

So cool! Thanks, @hecrj .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants