jmshrv
✨ New Login Flow
This PR reworks the login flow for Finamp. Login is now split into multiple steps, that offer more features for selecting a server and authenticating with an account.
I'm not fully satisfied with the navigation transitions, would've preferred some more animations but could get it to work in a nice way yet.
This PR also fixes an issue with logging into a Jellyfin 10.9 server, because those recently stopped returning some JSON items with caused an exception when trying to log in with Finamp.
Features:
- Specify the server URL without requiring http(s) or a port, and allowing a trailing slash
- Discover servers on the local network using UDP broadcast packets
- Show all visible users for the selected server
- Selecting a user pre-fills the username
- Log in using Quick Connect
Screenshots:
I tried this out against my own server, and ran into a couple issues. My server is on port 8096, and I tried just putting in the IP without the port. The server was eventually found, but it took over 2 minutes with no indication anything was happening. The logs suggest that's just how long it took the initial https request to time out. Also, on one login attempt I ran into this:
The .99 IP is correct, and presumably what returned the info. Continuing to user selection with a custom user threw a connection error when pressing log in, although again with a multi-minute timeout delay before there was any response.
Thanks for testing! Interesting, I wouldn't have thought the timeout could be that long. On what OS/version did you test it?
I also had the bug with the wrong base URL before, I'll take another look later!
@Komodo5197 I just added some optimizations to the server connection test, could you try again? If I did it correctly, requests should now time out after 5 seconds, so if your server is running on the default port without https (last case that gets checked), and nothing aborts the request, the connection attempt will time out after 15 seconds.
On my devices/servers the connection always gets refused immediately, so it times out after a few milliseconds.
There also shouldn't be any race condition for the base url anymore 😅
Okay, I tested it out and it's working way nicer now. The server shows up in 5 seconds with a loading indicator while waiting, and I haven't been able to get the race error to recur with the new code.
I did notice a couple of visual hiccups if you want to correct them. First, the text in the sever info box under the URL entry is sort of off center vertically and looks a bit off.
Second, the quick connect stuff still shows on the login screen even though my server has quick connect disabled. I think it would probably be better to hide it or have a disabled message.
Ah, thanks for pointing that out. I'll fix it tomorrow.
I'm curious, why do you have quick connect disabled? 🤔
I don't really remember why I disabled quick connect. The logic was probably 'I don't use this' -> 'might as well disable it' without too much thought put into it.
I'll probably go ahead and merge this in tomorrow, so that we can tests this as part of the other PRs. I'm guessing no-one is testing this otherwise ^^
This is a nice improvement, I've just tested Quick Connect and it did what was supposed to do :smile: I'd suggest adding a horizontal divider between QC and login fields, e.g. using design similar to this:
On the other hand, if QC is enabled on server, maybe it'd make sense to have this option earlier than in last step? Perhaps that's where that divider would fit better - use QC code or go through flow of selecting a (custom) user - especially since web client doesn't require you select/type in a user before getting the code.
Yeah, in theory Quick Connect is not tied to a specific account. But I'm assuming that supplying the code in this step might make it easier to deal with. However, it might be unexpected for the user if they select on account in Finamp, and then use Quick Connect with a different account.
I guess I could move the Quick Connect instructions to the user selection page, but that would probably result in scrolling (if multiple accounts exist). But I'll see what I can come up with. Since it's mostly layout changes, it shouldn't be too hard to do...
Just pushed a tiny change to use the system's monospace font instead of Roboto, saves us shipping a font (and makes more sense outside of Android)
Hmm, I could've sworn I wanted to make the font bold and it didn't work with the built-in font 🤔
I guess the system font should be fine then :)
hmm, yeah it looks like the bold hasn't applied. I'd say it's prominent enough anyway
Okay, I'd appreciate a short round of testing, ideally not just the Quick Connect stuff. Either way, I'll probably merge this into redesign some time tomorrow...
Okay, I'd appreciate a short round of testing, ideally not just the Quick Connect stuff. Either way, I'll probably merge this into
redesignsome time tomorrow...
Is there something blocking that now? I can help test if needed.
I simply didn't get to it yet, but if you want to give it another test I'll wait for your feedback!
Yup, works great! Took me a bit, not really familiar with android dev tooling haha. LGTM
Info: Pixel 7 Android 14 (GrapheneOS)
One quick thing though, seems like the firefox password manager doesn't detect the login page as somewhere where login details can be autofilled. idk if that's a finamp issue though. This was like that before too.
Thanks for testing. Yeah I have a similar problem with Bitwarden, it works if I go back into the input field and select the credentials from there, without opening the password manager app again. Maybe that works for you too?
I'll take a look at what might be the issue, but since it's not a downgrade I'll merge this anyway :)
I played around with the autofill stuff a bit, but didn't get it to work. I have the same issue in the Flutter Autofill sample app, and also the same issue when using Google's autofill instead of Bitwarden. Seems like this is a limitation of Flutter that we can't get around for now 😕
@Chaphasilor On my local network with - while testing - only four devices including router and jellyfin server address http://192.168.178.21:8096, the jellyfin server is not found by server discovery, even after letting it run for several minutes.
When entering the URL manually, it finds and shows it though.
Don't know, it just doesn't work for me on a fresh redesign installation (Pixel, Android 14).
The logs do say "[JellyfinServerClientDiscovery] Sending discovery messages" and, as already said, when I provide the actual address, it connects, the auto-discovery just doesn't seem to work.
Did you try it with the official Jellyfin app? If that doesn't work either, it's likely a server config problem. You need to expose UDP port 7359: https://jellyfin.org/docs/general/networking/#static-ports
If the official client works but Finamp doesn't, I'd love to do more testing. Either way, thanks for letting me know about the issue! :)
Ah, okay, thank you! I'm using a device firewall, I was assuming it would just look at, for example, port 8096 (which is exposed) and wait for responses.
So, everything back, my mistake, sorry for the panic. After exposing port 7359, it now works.
Awesome!
Scanning ports would in theory work, but it would simply take too long, would be less reliable, and would be much slower for non-standard ports.
It's just a bummer that this isn't properly mentioned in the Jellyfin docs / setup guide. I'll probably add a small disclaimer to the release notes :)