minibrowser: for each tab, show a spinning wheel while loading #38878
Conversation
|
It looks like the below 
I put the spinning wheel outside of the button, mainly because it doesn't seem to implement |
91df49c to
c8d4000
Compare
|
|
||
| let selected = webview.focused(); | ||
| if webview.has_pending_load() { | ||
| // Show a spinner while the tab is loading. |
There was a problem hiding this comment.
| // Show a spinner while the tab is loading. |
This comment is self-explanatory from the condition and function next line ui.spinner()
There was a problem hiding this comment.
I don't know, I find this update method quite hard to parse, so an extra comment wouldn't hurt.
| focused: false, | ||
| animating: false, | ||
| cursor: Cursor::Pointer, | ||
| pending_load: true, |
There was a problem hiding this comment.
| pending_load: true, | |
| pending_load: false, |
So that it shows the load button when actually start to load after set_load_status.
Right now it shows spinner at the beginning of creating new tab. After this change, there would be a short delay.
There was a problem hiding this comment.
When a Webview is created, it is always loaded to a url, so I think it makes sense to immediately show the spinner, as opposed to only show it after a short delay.
I think this looks odd. For example, in second screenshot its not immediately obvious which tab the spinning wheel belongs to. |
Do you have an actionable suggestion? I find it quite obvious thanks to the "close the tab" button acting as a kind of delimiter. Also, if you first open one tab, the spinner is on the left of it, so by the time you open a second tab, it's quite obvious for which tab the spinner is. Id say give it a try if the screenshot seems odd. |
I agree with @simonwuelker . The spinner being out of the (title + X) zone makes it look detached from any tab. Why not move it into that (title + X) container? |
|
#36680 adds favicons to the tab bars and will likely merge soon. My suggestion is to replace the favicon with the spinner while the tab is loading. |
Now that's actionable feedback, I've taken your tab ui code and used it to put the spinner inside the tab. Thank you. 
I'll rebase once the other merges. |
… still loading its document Signed-off-by: gterzian <2792687+gterzian@users.noreply.github.com>
Signed-off-by: gterzian <2792687+gterzian@users.noreply.github.com>
7dc314f to
71b2b3d
Compare
|
It works but there are a couple of things related to when to show the spinner and or the favicon(which loads before the page has finished loading so currently you see both a favicon and a spinner), and whether to clear the favicon on reload, that needs to be to looked into... |
I think it's a good idea to follow standard browser practice, and show some visual indicator while a tab is still loading it's top-level document. This PR adds the visual element in the minibrowser and the backing state in webviews and app state, as well as a boolean used to request a redraw of the native UI(but not of the web page).
Testing: Manual testing.
Fixes: N/A