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

feat: show application as soon as possible #1499

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

pedrolamas
Copy link
Member

@pedrolamas pedrolamas commented Sep 30, 2024

Present Fluidd page as soon as possible instead of waiting for socket connection.

The changes here are significant and could lead to unexpected scenarios, but I have tested with and without authentication enabled and I believe everything is working as it should and no errors are currently being logged in the browser console.

Signed-off-by: Pedro Lamas pedrolamas@gmail.com

@pedrolamas pedrolamas added FR - Enhancement New feature or request UI - Change All matters related to a significant change in the UI labels Sep 30, 2024
@pedrolamas pedrolamas added this to the 1.30.5 milestone Sep 30, 2024
@pedrolamas pedrolamas requested a review from matmen October 1, 2024 09:10
Signed-off-by: Pedro Lamas <pedrolamas@gmail.com>
@matmen
Copy link
Member

matmen commented Oct 1, 2024

I tested the changes and they look fine to me (at least nothing appears to be broken 😛), but just wondering: is this solving any specific issue? I haven't noticed fluidd taking "too long" to load or anything - I believe it's always shown the app even if the socket wasn't connected yet?

@pedrolamas
Copy link
Member Author

The current behavior is to try and connect the socket before we even show the UI, and that translates in a 3s delay if the address is down.

The new behavior is to show the page with an "indeterminate" progress bar loading while we do the connection.

new

On the above, left is new behavior, right is the old behavior. I pressed F5 on both sides at the same time.

Copy link
Member

@matmen matmen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't really notice any difference on my end (granted I've never had issues with this), but it doesn't look like this broke anything, so LGTM ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FR - Enhancement New feature or request UI - Change All matters related to a significant change in the UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants