marimo-team / marimo

A reactive notebook for Python — run reproducible experiments, execute as a script, deploy as an app, and version with git.
https://marimo.io
Apache License 2.0
5.35k stars 156 forks source link

feat: Feedback on notebook run completion #1634

Closed wasimsandhu closed 2 weeks ago

wasimsandhu commented 2 weeks ago

📝 Summary

Gives user feedback on notebook run completion via dynamic favicon and optional system notifications.

Addresses feature request in #1039.

🔍 Description of Changes

Dynamic favicon

Favicon changes to indicate whether a notebook is running, and whether the run completed successfully or encountered errors.

This favicon persists until the user navigates back to their notebook, or resets back to the marimo favicon after 3 seconds if the notebook is in focus.

Favicon when notebook is running:

blue play

Favicon when a run completed successfully:

green check

Favicon when a run produced errors:

red x

Notifications

Implemented by @mscolnick, more info in this comment.

Screenshot 2024-06-18 at 7 08 46 PM Screenshot 2024-06-18 at 7 09 16 PM

📋 Checklist

📜 Reviewers

@mscolnick

vercel[bot] commented 2 weeks ago

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

Name Status Preview Comments Updated (UTC)
marimo-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 19, 2024 2:44am
marimo-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 19, 2024 2:44am
wasimsandhu commented 2 weeks ago

@mscolnick My changes work in development mode, but I can't get the favicons to appear when running in prod. Tried clearing my cache and testing in incognito, but no luck. Any ideas?

akshayka commented 2 weeks ago

Thanks for this, @wasimsandhu!

Add optional app (user?) settings for audio and system notifications on run completion.

I'd say to do just the favicon for now. I'd prefer not to add another app setting (unless people really want it).

mscolnick commented 2 weeks ago

if this feels right, you could also use the native browser notification. this way you dont need to add anything to user settings, and they can configure it on the first try (and if they deny access, it wont happen again)

function maybeSendNotification() {
  // If in focus, skip
  if (document.visibilityState === "visible") {
    return;
  }

  const sendNotification = () => {
    if (succeeded) {
      const notification = new Notification("Execution completed", {icon: icon});
    } else {
      const notification = new Notification("Execution failed", {icon: icon}););
    }
  }

  if (!("Notification" in window)) {
    // Skip no support
  } else if (Notification.permission === "granted") {
    // Permission already granted
    sendNotification();
  } else if (Notification.permission !== "denied") {
    // We need to ask the user for permission
    Notification.requestPermission().then((permission) => {
      // If the user accepts, let's create a notification
      if (permission === "granted") {
        sendNotification();
      }
    });
  }
}
mscolnick commented 2 weeks ago

@wasimsandhu for it working in production - you might need to add those assets to here: https://github.com/marimo-team/marimo/blob/main/marimo/_server/api/endpoints/assets.py#L93-L98

mscolnick commented 2 weeks ago

nice! looks great!

github-actions[bot] commented 2 weeks ago

🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.6.20-dev18