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(mobile): Auto switching server URLs #14437

Merged
merged 40 commits into from
Dec 5, 2024
Merged

Conversation

alextran1502
Copy link
Contributor

@alextran1502 alextran1502 commented Dec 1, 2024

This PR adds Network Settings, which allow users to specify additional endpoints for automatic endpoint switching when on and off the local network.

When enabling the feature, the user can manually specify the WiFi name and target endpoint or auto-populate the current connection information for Local Connection. As for External Connection, the user can specify a list of endpoints, ordered by priority. Those endpoints will be checked and verified, and the mobile app will establish the connection under the applicable network conditions.

The app will need the Location Permission to get the WiFi name to work correctly.

This PR also redesigned the settings page.

Setting Pages

Network Settings

TODO

  • Documentation

@fyfrey
Copy link
Contributor

fyfrey commented Dec 2, 2024

I really like the settings makeover.

Have you checked that all the connection/wifi settings work in the background backup?

mobile/lib/providers/app_life_cycle.provider.dart Outdated Show resolved Hide resolved
mobile/lib/services/auth.service.dart Show resolved Hide resolved
final entries =
useState([AuxilaryEndpoint(url: '', status: AuxCheckStatus.unknown)]);
final canSave =
useState(entries.value.every((e) => e.status == AuxCheckStatus.valid));
Copy link
Contributor

Choose a reason for hiding this comment

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

Not resolved

@alextran1502
Copy link
Contributor Author

alextran1502 commented Dec 5, 2024

Alright @mertalev resolved the comments and also removed the blur. Please give it another spin

@alextran1502 alextran1502 changed the title feat(mobile): Auto switching server endpoints feat(mobile): Auto switching server URLs Dec 5, 2024
mobile/assets/i18n/en-US.json Outdated Show resolved Hide resolved
mobile/assets/i18n/en-US.json Outdated Show resolved Hide resolved
mobile/assets/i18n/en-US.json Outdated Show resolved Hide resolved
mobile/assets/i18n/en-US.json Outdated Show resolved Hide resolved
mobile/assets/i18n/en-US.json Outdated Show resolved Hide resolved
mobile/assets/i18n/en-US.json Outdated Show resolved Hide resolved
mobile/assets/i18n/en-US.json Outdated Show resolved Hide resolved
@@ -8,36 +8,69 @@ import 'package:immich_mobile/widgets/settings/asset_list_settings/asset_list_se
import 'package:immich_mobile/widgets/settings/asset_viewer_settings/asset_viewer_settings.dart';
import 'package:immich_mobile/widgets/settings/backup_settings/backup_settings.dart';
import 'package:immich_mobile/widgets/settings/language_settings.dart';
import 'package:immich_mobile/widgets/settings/networking_settings/networking_settings.dart';
import 'package:immich_mobile/widgets/settings/notification_setting.dart';
import 'package:immich_mobile/widgets/settings/preference_settings/preference_setting.dart';
import 'package:immich_mobile/routing/router.dart';

enum SettingSection {
Copy link
Contributor

@mertalev mertalev Dec 5, 2024

Choose a reason for hiding this comment

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

I was initially thinking about shortening the subtitles, but now I'm looking at it more closely and the subtitles are all just restating the title. There's no new info. The sections are all self-explanatory, so I think you can just get rid of the subtitles. It'll look cleaner and allow it to be more compact.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, some of the entries are thicker than others because the subtitle is two lines vs. one, so removing them will make the thickness consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't look good without subtitles. I like having subtitles in this UI element as a design preference

Copy link
Contributor

Choose a reason for hiding this comment

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

That's okay then. I think it'd still be better to bring the two-line subtitles down to one line, but I don't think this is blocking

@alextran1502 alextran1502 merged commit 055f1fc into main Dec 5, 2024
36 checks passed
@alextran1502 alextran1502 deleted the networking-settings branch December 5, 2024 15:11
@SleepInfinity
Copy link

SleepInfinity commented Dec 8, 2024

This is fantastic!
I have a suggestion: could you consider adding support for multiple Wi-Fi SSIDs, separated by a comma? On my router, I have two Wi-Fi networks (2.4GHz and 5GHz), and it would be great to connect to the local IP address of the Immich server from either of these networks.

Also, I noticed that when I open the Networking tab in the app, the scroll FPS drops to around 30. Could you explain why this happens?

Thank you for your hard work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants