eframe_template icon indicating copy to clipboard operation
eframe_template copied to clipboard

Reintroduce the loading spinner

Open sedsedus opened this issue 2 years ago • 2 comments

Context / before

  • The loading spinner (and loading text) were removed in d12c19da37de943d06703bec09613cf7121a70f9
  • If WebRunner::start fails nothing is shown in HTML, the user has to open the console to see if something happened (a.k.a. halting problem)

What changed / now

  • before the canvas is loaded a CSS loading spinner with "Loading..." text is displayed
  • if the loading fails, "the app has crashed" text is shown instead
    • still panic! in this case (as it did)
  • the corresponding div is removed after loading successfully completes

How does it look

image image

What exactly changed

  • introduce dependency to web-sys
  • add the loading_text div that shows the spinner text during loading, or error if loading fails
  • access the DOM (in main.rs) after the app has been loaded, and remove the spinner, or show an error
    • if we fail to locate loading_text (like when someone removes the div from index.html) - nothing happens

Discussion

  • I wanted to avoid dependency on web-sys, but, I don't see any other simple way to access the spinner
  • Another idea I had was to do it straight from the js side (like it was before), however, I don't see a way to receive events from WebRunner
  • If this solution is undesired, I think the CSS remnants of the spinner should be removed (like lds-dual-ring)

Alternative solution

A more thorough solution (in main.rs) is https://github.com/emilk/eframe_template/compare/master...sedsedus:eframe_template:fix-css-spinner-install-hook, where all panics are caught to show HTML error. I encountered this myself, where I would mistype the canvas id and wait a few seconds wondering why the app doesn't load, only to discover in the console that something failed. This is more thorough (but at the same time a little more complex / less clean due to hook insertion) because this "invalid canvas id error" wouldn't get caught by the proposal in this PR

sedsedus avatar Sep 24 '23 09:09 sedsedus

Another idea I had was to do it straight from the js side (like it was before), however, I don't see a way to receive events from WebRunner

This is what egui.rs does: https://github.com/emilk/egui/blob/dbf3405597619fb563a1d79926560c6ab3746442/docs/index.html#L132-L192

…but this PR seems a lot simpler imho. Less JavaScript is always a win in my book.

emilk avatar Sep 27 '23 12:09 emilk

Less JavaScript is always a win in my book.

I like this book :).

Before merging please also consider merging (I think you should be able to) this alternative into this branch: https://github.com/sedsedus/eframe_template/pull/1 ^ is potentially better because it catches all panics (unlike this PR), but I am not sure if main.rs hasn't become too complex for a "plain" template (panic hooks)

sedsedus avatar Sep 27 '23 17:09 sedsedus