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

Improvements to loading animations #3386

Closed
FrancYescO opened this issue Nov 13, 2024 · 31 comments · Fixed by #3409
Closed

Improvements to loading animations #3386

FrancYescO opened this issue Nov 13, 2024 · 31 comments · Fixed by #3409
Labels
enhancement New feature or request

Comments

@FrancYescO
Copy link

FrancYescO commented Nov 13, 2024

Describe the feature you would like to see

When we set a filter on log level that have no content, or if the container have no logs the loading spinner
image
never disappear making UX vary bad (we don't know if is still loading or we just have no result)

another thing for a better UX: pretty often i use dozzle behind a VPN and when i disconnect i'll keep seeing the bouncing bar
image
giving me the bad appareance that the websocket is still connected and i'm still following the stream for new logs... will be nice to see an event to hide/change the color of this bar wen the websocket lose the connection, this case can be simulated with the throttling option of chrome:
image

opened a specific issue from #3385 (comment)

@FrancYescO FrancYescO added the enhancement New feature or request label Nov 13, 2024
@amir20
Copy link
Owner

amir20 commented Nov 13, 2024

Never disappearing makes the user experience very poor. We can't tell if the system is still loading or if there are simply no results.

This is the exact problem Dozzle faces. The Docker API returns logs only when they are available. Dozzle cannot distinguish between a lack of data and a slow response.

I'm unsure how to resolve this. I could "fake" it by sending a dummy log, but is that the right approach? If something goes wrong with the Docker API, the user won't see a loading indicator anymore. I can't think of a better solution and I'm open to ideas.

It would be nice to have an event that hides or changes the color of this bar.

I agree with this suggestion. I believe all the necessary code is already in place for handling a disconnect; I just need to change the color.

@FrancYescO
Copy link
Author

FrancYescO commented Nov 13, 2024

I'm unsure how to resolve this. I could "fake" it by sending a dummy log, but is that the right approach? If something goes wrong with the Docker API, the user won't see a loading indicator anymore. I can't think of a better solution and I'm open to ideas.

i'm not sure how it works on the BE side (maybe there is just a 6000ms timeout?), but looking on the frontend requests/network tab of the browser i think that removing the circular spinner and show the bouncing bar when the /stream endpoint fullfill the request (receive the :ping ?) is a good way to manage the situation, in this way we know at least that the endpoint is connected, maybe still loading, but connected

i.e in my case the request with an empty search params remains in pending for 6s than the /stream endpoint get connected start receiving the :pings... but on the frontend side nothing changes and just the circular spinner is shown

@amir20
Copy link
Owner

amir20 commented Nov 13, 2024

bouncing bar when the /stream endpoint fullfill the request (receive the :ping ?) is a good way to manage the situation, in this way we know at least that the endpoint is connected, maybe still loading, but connected

:ping is just a comment in web socket. I don't think Javascript actually see it. See here

So I am not sure 😕

@FrancYescO
Copy link
Author

EventSource.readyState

i think is just when changes from CONNECTING to OPEN is enougth

@amir20
Copy link
Owner

amir20 commented Nov 13, 2024

I could use that. But that just means the connection is open. It doesn't mean the logs have been fetched yet. But maybe that's good enough for now?

@FrancYescO
Copy link
Author

yep i think is still better than the actual "nothing"

@amir20
Copy link
Owner

amir20 commented Nov 14, 2024

I updated I fixed the first issue by using on open here #3388

The VPN issue is hard to reproduce. I used "offline" but the connection is still active I continue to get data. When I intentionally exit Dozzle I see:

Screenshot 2024-11-13 at 5 20 51 PM

So it might be possible that the connection just hangs. Because if I see eventsource.onerror I clear the logs and show that error.

@FrancYescO
Copy link
Author

testing the 3388 i still not see what i was expecting:
see here, when the first request get the 200 logs instantly popping (obviously as the resulting gived something..)
while when searching for "sssss" a new stream request is firing, but when the request gets a 200 i expect at least that the circular spinner changes in the boncing bar giving me the appareance that i'm connected to logs and not still waiting for the initial load
Nov-14-2024 09-36-02

@FrancYescO
Copy link
Author

So it might be possible that the connection just hangs

you are right... the onerror is misleading... seems we need a more complex solution to know if the eventstream is disconnected ... trying to figure out if somehow we can read the :ping event on the client side (so if no ping/events for x seconds we assume are disconnected)

@amir20
Copy link
Owner

amir20 commented Nov 14, 2024

testing the 3388 i still not see what i was expecting

Hmm, I think I may have found the weirdest bug. It seems as if SSE doesn't trigger onOpen until some data is sent. In this case, :ping is sent after a little while and it triggers the on open. However, this makes sense because when I do curl -v the response headers aren't sent until the first ping.

I am going to see if send :ping when connection first opens.

so if no ping/events for x seconds we assume are disconnected

I think the only way to do this is implement my own heartbeat ping. I'll have to think about it. Let me know if you find anything.

@amir20
Copy link
Owner

amir20 commented Nov 14, 2024

I have added :ping immediately.

I think i'll have to come back to heartbeat for a future release. It would require some modest amount of work.

@amir20
Copy link
Owner

amir20 commented Nov 14, 2024

Going to release tomorrow if everything looks good.

@amir20
Copy link
Owner

amir20 commented Nov 15, 2024

Going to merge. Haven't heard back though.

@FrancYescO
Copy link
Author

i can confirm that now is a lot better, idk if is wanted but when no logs are loaded also the bouncing bar is not visualised

@amir20
Copy link
Owner

amir20 commented Nov 16, 2024

idk if is wanted but when no logs are loaded also the bouncing bar is not visualised

I bet it's there. It's just hidden.

@amir20
Copy link
Owner

amir20 commented Nov 17, 2024

Looks like its hidden when there are no messages. I plan to come back to this later. There is something I want to do before working on it.

@amir20
Copy link
Owner

amir20 commented Nov 18, 2024

I made some improvements to the loader at #3404. Can you test it with amir20/dozzle:pr-3404?

I'm still unsure about it. I removed the circular loader and replaced it with a pulsing line. It starts in the center and moves to the bottom when data is loaded. I believe there should be a cleaner way to achieve this, as I don't find a loader in the middle of the screen very elegant.

@amir20
Copy link
Owner

amir20 commented Nov 18, 2024

@FrancYescO These are the states that I think a loader would need to handle correctly:

  1. State: Loading -- SSE is still connecting
  2. State: Loaded but no data -- SSE connected but there are no logs. This could be latency issue or just no logs
  3. State: more than zero logs -- SSE has returned logs
  4. State: error -- SSE onError triggered

For 1, I think it just makes sense to have gray boxes.
For 2, I don't know. This is a tough one. What do you think? Should it say no logs, or just empty?
For 3, already done.
For 4, I could just turn the bottom line to red. I currently do this.

@FrancYescO
Copy link
Author

testing pr-3404, personally i really don't like the bouncing bar in the center and having a different spinner like before was helping to better understand that different things were happening: so something like, circular center spinner in yellow while SSE connecting, that turn green while connected, as no logs can be also a delay showing no log may be not a good idea: just show the bouncing bar on top of the list (and maybe after ~5s of no log loaded showing something like "Container have no logs, or maybe we are stil loading")

@amir20
Copy link
Owner

amir20 commented Nov 18, 2024

I made some updates by showing fake gray boxes. I think it improves it a little bit. I don't like center either.

I wasn't a fan of the circular loader. It should just one style of loader IMO.

Try docker pull and try again in a few mins. I think it's still building.

@FrancYescO
Copy link
Author

I wasn't a fan of the circular loader. It should just one style of loader IMO.

probably you are right, just showing on top (or bottom?) instead of center maybe is enought?

just waited for the push, but i'm not seeing the gray boxes you are mentioning

@FrancYescO
Copy link
Author

just waited for the push, but i'm not seeing the gray boxes you are mentioning

nvm, got it (browser cache) ... yeah can be okay... maybe show that on full page (or use the rest of the page to show something other... like container name/image name/start time to put the loading on bottom 🤷)

@amir20
Copy link
Owner

amir20 commented Nov 18, 2024

I'd consider that as new work :P Let's get the loader right first. It all happens so quickly that is hard to see.

Right now the implementation shows the gray boxes if there are no logs. Might be confusing. I tried removing it, but it looked pretty bad because it would jump from an empty list to a big list between containers.

Let me know if you have any other ideas on the loaders. You'll probably have to play around with the network setting to see the error.

@FrancYescO
Copy link
Author

FrancYescO commented Nov 18, 2024

just noticed a bug: if the loading bar gets red and than the connection is restored (remaining on the page) it remain in red also after the logs get loaded

fast replicate: set offline, change container, than remove the offline throttling

@amir20
Copy link
Owner

amir20 commented Nov 18, 2024

Nice find. Yep, bug.

@amir20
Copy link
Owner

amir20 commented Nov 19, 2024

and maybe after ~5s of no log loaded showing something like "Container have no logs, or maybe we are stil loading"

I like that idea btw. I'll see if that makes more sense.

@amir20
Copy link
Owner

amir20 commented Nov 19, 2024

Alright #3409 should be the lat improvement. I think this issue can be closed now.

@FrancYescO
Copy link
Author

Just another minor issue maybe only on mobile while connection restored

trim.CD2463DF-BE08-483B-A242-9DDFA9F4C3F2.MOV

@amir20
Copy link
Owner

amir20 commented Nov 20, 2024

That has always been around. It's not an easy fix though.

@FrancYescO
Copy link
Author

Maybe just don't show "container not found" (or a spinner under it?) if the is and undergoing containers reload request

@amir20
Copy link
Owner

amir20 commented Nov 20, 2024

The problem is that Dozzle is unsure whether a container doesn't exist or if I'm just waiting for the data to reload. Both of those states are the same. I would have to look deeper into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants