feat(mobile) - Add better offline support #3279
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
|
I don't think this should be set as a setting. It should be handled by default by fixing, and implementing better support for offline cases. |
This PR basically supports for a user to stay authenticated even if they're offline (currently it will hit multiple long timeouts waiting for several API endpoints to complete resolving). I can take out the setting option but this way users have the option to choose if their immich app shows images even if they're offline and their token cannot be validated. I can see some users not wanting to allow that to happen... |
|
Google photos and most photo apps don't have that as far as I am aware though. If we added a setting to restrict app access, it seems like a fingerprint option would make more sense IMO. |
|
Appreciate the feedback from both, I'll remove the setting and we can revisit a restriction feature later on |
|
Removed setting option |
mobile/lib/modules/login/providers/authentication.provider.dart
Outdated
Show resolved
Hide resolved
|
@CodiumAI-Agent /review |
PR Analysis
PR Feedback
How to use
|
| bool retResult = false; | ||
|
|
||
| if (offlineLogin) { | ||
| User? offlineUser = Store.tryGet(StoreKey.currentUser); |
There was a problem hiding this comment.
Consider adding error handling for the case when the user is offline and the stored user data is null. This could prevent potential crashes or undefined behavior. [important]
| if (accessToken != null && serverUrl != null) { | ||
| try { | ||
| // Resolve API server endpoint from user provided serverUrl | ||
| await apiService.resolveAndSetEndpoint(serverUrl); | ||
| } catch (e) { | ||
| // okay, try to continue anyway if offline | ||
| deviceIsOffline = true; |
There was a problem hiding this comment.
Consider adding a fallback or error message for the user in the case that the device is offline and login is unsuccessful. This would improve user experience by providing clear feedback. [medium]
There was a problem hiding this comment.
If the device is offline and the login is unsuccessful (i.e. there is no user information stored locally) then it will try to do an online login. If that also fails then it will redirect user back to the login page
| @@ -70,6 +76,29 @@ class ApiService { | |||
| return url; | |||
| } | |||
|
|
|||
| Future<bool> _isEndpointAvailable(String serverUrl) async { | |||
There was a problem hiding this comment.
I believe we would hit the .well-known endpoint to get the /api if the user forgot to put it in. Maybe only applicable to https but not local IP, can you help me double check
There was a problem hiding this comment.
Lmk if you have any more question after this after the discussion on Discord
| } else { | ||
| debugPrint("Error [onNavigation] ${e.toString()}"); | ||
| router.replaceAll([const LoginRoute()]); | ||
| return; |
There was a problem hiding this comment.
If there is no return statement here, what would happen?
Should fix #3239, #3199- would like Android testers to confirm. Was able to recreate the same issue on iOS and tested the fix to work.