fix: show updater dialog before updating in windows

tribhuwan-kumar commented 3 weeks ago

name: pull request about: title: "[pr] " fix: #574

description

it'll ask user whether they want to update or not by showing a dialog ps: need to test in linux

type of change

[x] bug fix
[ ] new feature
[ ] breaking change
[ ] documentation update

how to test

add a few steps to test the pr in the most time efficient way.

if relevant add screenshots or screen captures to prove that this PR works to save us time.

checklist

[ ] MOST IMPORTANT: this PR will require less than 30 min to review, merge, and release to production and not crash in the hand of thousands of users
[ ] i have read the CONTRIBUTING.md file
[ ] i have updated the documentation if necessary
[ ] my changes generate no new warnings
[ ] i have added tests that prove my fix is effective or that my feature works

additional notes

any other relevant information about the pr.

vercel[bot] commented 3 weeks ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
screenpipe	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 4, 2024 4:05am

louis030195 commented 3 weeks ago

@tribhuwan-kumar is that a change only for windows?

tribhuwan-kumar commented 3 weeks ago

@tribhuwan-kumar is that a change only for windows?

no, it applies on all os, currently it prompts a dialog to let user decides to update or not, if user selects update it'll start the download and install after killing the processes, but i'm thinking to add an another option where user can update even from system tray right now system tray is only able to restart screenpipe https://github.com/mediar-ai/screenpipe/blob/2031e53b5f353c48284e89d58a79e005a1724624/screenpipe-app-tauri/src-tauri/src/main.rs#L231 https://github.com/mediar-ai/screenpipe/blob/2031e53b5f353c48284e89d58a79e005a1724624/screenpipe-app-tauri/src-tauri/src/updates.rs#L109

louis030195 commented 3 weeks ago

@tribhuwan-kumar yeah agree could have a menu item "check for updates" since many people ask me how to update

the experience on mac is good now but it's different / weird on windows so i'd prefer it does not change on macos (except for the new menu item if you add it)

tribhuwan-kumar commented 3 weeks ago

@tribhuwan-kumar yeah agree could have a menu item "check for updates" since many people ask me how to update

the experience on mac is good now but it's different / weird on windows so i'd prefer it does not change on macos (except for the new menu item if you add it)

there is already a periodic update event so i guess "check for updates" item is not necessary

tribhuwan-kumar commented 3 weeks ago

yeah agree could have a menu item "check for updates" since many people ask me how to update

the experience on mac is good now but it's different / weird on windows so i'd prefer it does not change on macos (except for the new menu item if you add it)

now it'll keep the mac and linux experience same as prior, only changed it for windows to take user consent before downloading update!

tribhuwan-kumar commented 3 weeks ago

(except for the new menu item if you add it)

there is no need of new menu item for updating bcz check_for_updates function is automatically downloading and installing update without user's intervention (this pr will change this only for windows).

https://github.com/mediar-ai/screenpipe/blob/main/screenpipe-app-tauri/src-tauri/src/updates.rs

   pub async fn check_for_updates(
        &self,
        show_dialog: bool,
    ) -> Result<bool, Box<dyn std::error::Error>> {
        if let Some(update) = self.app.updater()?.check().await? {
            *self.update_available.lock().await = true;

            self.update_menu_item
                .set_text("downloading latest version of screenpipe")?;

            if let Some(tray) = self.app.tray_by_id("screenpipe_main") {
                let path = self.app.path().resolve(
                    "assets/update-logo-black.png",
                    tauri::path::BaseDirectory::Resource,
                )?;

                if let Ok(image) = tauri::image::Image::from_path(path) {
                    tray.set_icon(Some(image))?;
                    tray.set_icon_as_template(true)?;
                }
            }

            update.download_and_install(|_, _| {}, || {}).await?;

            *self.update_installed.lock().await = true;
            self.update_menu_item.set_enabled(true)?;
            self.update_menu_item.set_text("update now")?;

            if show_dialog {
                let (tx, rx) = oneshot::channel();
                let update_dialog = self
                    .app
                    .dialog()
                    .message("update available")
                    .title("screenpipe update")
                    .buttons(MessageDialogButtons::OkCancelCustom(
                        "update now".to_string(),
                        "later".to_string(),
                    ))
                    .parent(&self.app.get_webview_window("main").unwrap());

                update_dialog.show(move |answer| {
                    let _ = tx.send(answer);
                });

                if rx.await? {
                    // Proceed with the update

                    // i think it shouldn't kill if we're in dev mode (on macos, windows need to kill)
                    // bad UX: i use CLI and it kills my CLI because i updated app

                    if let Err(err) =
                        kill_all_sreenpipes(self.app.state::<SidecarState>(), self.app.clone())
                            .await
                    {
                        error!("Failed to kill sidecar: {}", err);
                    }
                    self.update_screenpipe();
                }
            }

            return Result::Ok(true);
        }

        Result::Ok(false)
    }

tribhuwan-kumar commented 3 weeks ago

@louis030195 tested with a build you can merge it now :)

louis030195 commented 3 weeks ago

/tip $50 @tribhuwan-kumar thx!

algora-pbc[bot] commented 3 weeks ago

🎉🎈 @tribhuwan-kumar has been awarded $50! 🎈🎊

louis030195 commented 3 weeks ago

@tribhuwan-kumar im still wondering if it can be too intrusive on windows or macos

it shouldnt distract the user from its task or whatever (something outside of screenpipe)

tribhuwan-kumar commented 3 weeks ago

@tribhuwan-kumar im still wondering if it can be too intrusive on windows or macos

it shouldnt distract the user from its task or whatever (something outside of screenpipe)

macos and linux experience is same as prior, only changed it for windows to show dialog, it can't be too intrusive for windows users, they can simply pass the update if they don't want to update, next updater dialog will show on periodic event or when app will restart

louis030195 commented 3 weeks ago

@tribhuwan-kumar someone on windows complains that "it checks update every time computer is booting" or something like that

do you know how it looks like?

tribhuwan-kumar commented 2 weeks ago

@tribhuwan-kumar someone on windows complains that "it checks update every time computer is booting" or something like that

do you know how it looks like?

since the screenpipe auto restarts on booting therefore updater dialog is also appears when there is an update available

mediar-ai / screenpipe