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

Do not warn users about multihop extra latency, add info button instead #6535

Merged
merged 2 commits into from
Jul 30, 2024

Conversation

buggmagnet
Copy link
Contributor

@buggmagnet buggmagnet commented Jul 29, 2024

This PR changes the UI behaviour to remove the alert when enabling multihop, for a less disruptive experience.
Instead, an info button is added next to the setting name that indicates what the option does.

This PR also orders by name the Pages UITest subfolder.


This change is Reviewable

@buggmagnet buggmagnet added the iOS Issues related to iOS label Jul 29, 2024
@buggmagnet buggmagnet requested review from mojganii and acb-mv July 29, 2024 14:23
@buggmagnet buggmagnet self-assigned this Jul 29, 2024
Copy link

linear bot commented Jul 29, 2024

Copy link
Contributor

@acb-mv acb-mv left a comment

Choose a reason for hiding this comment

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

Reviewed 7 of 8 files at r1, all commit messages.
Reviewable status: 7 of 8 files reviewed, 1 unresolved discussion (waiting on @buggmagnet)


ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsViewController.swift line 183 at r1 (raw file):

                "MULTIHOP_INFORMATION_TEXT",
                tableName: "Multihop",
                value: "This setting increases latency. Use only if needed.",

Is this the final copy?

Copy link
Collaborator

@pinkisemils pinkisemils left a comment

Choose a reason for hiding this comment

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

Reviewable status: 7 of 8 files reviewed, 1 unresolved discussion (waiting on @acb-mv)


ios/MullvadVPN/View controllers/VPNSettings/VPNSettingsViewController.swift line 183 at r1 (raw file):

Previously, acb-mv wrote…

Is this the final copy?

It is for now. We can improve it, but we currently don't need to change it, because it's the exact same text we're already showing.

Copy link
Contributor

@acb-mv acb-mv left a comment

Choose a reason for hiding this comment

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

Reviewable status: 7 of 8 files reviewed, all discussions resolved

Copy link
Collaborator

@pinkisemils pinkisemils left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@buggmagnet buggmagnet force-pushed the change-multihop-toggle-behavior-ios-770 branch from 553efc7 to 16936b6 Compare July 30, 2024 09:45
@buggmagnet buggmagnet merged commit 5dd533c into main Jul 30, 2024
8 of 9 checks passed
@buggmagnet buggmagnet deleted the change-multihop-toggle-behavior-ios-770 branch July 30, 2024 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iOS Issues related to iOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants