Add foundation for new connection view

[rablador]

In order to work incrementally and in parallel we should first create a basic implementation that can act as a foundation for coming work. It doesn't have to be stateful.

---

This change is 

[linear]

IOS-959 Add foundation for new connection view

[buggmagnet]

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

---

ios/MullvadVPN.xcodeproj/project.pbxproj line 3913 at r1 (raw file):

			sourceTree = "<group>";
		};
		7A0EAE982D01B29E00D3EB8B /* Recovered References */ = {

This should be deleted

[buggmagnet]

Except for the hiccup in the project structure, looks good !

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

[rablador]

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

---

ios/MullvadVPN.xcodeproj/project.pbxproj line 3913 at r1 (raw file):

Previously, buggmagnet wrote…

This should be deleted

Done.

[buggmagnet]

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

[mojganii]

Reviewable status: all files reviewed, 13 unresolved discussions (waiting on @rablador)

---

ios/MullvadVPN/View controllers/Tunnel/FeatureIndicators/ConnectionView.swift line 18 at r2 (raw file):

            BlurView()

            VStack(alignment: .leading, spacing: 16) {

we already have property for that ' UIMetrics.padding16'

---

ios/MullvadVPN/View controllers/Tunnel/FeatureIndicators/ConnectionView.swift line 22 at r2 (raw file):

                ButtonPanel()
            }
            .padding(16)

same above for the rest

---

ios/MullvadVPN/View controllers/Tunnel/FeatureIndicators/ConnectionView.swift line 28 at r2 (raw file):

        // Importing UIView in SwitftUI (see BlurView) has sizing limitations, so we need to help the view
        // understand its width constraints.
        .frame(maxWidth: UIScreen.main.bounds.width)

does the comment imply that we can't use .frame(width: .infinity)

---

ios/MullvadVPN/View controllers/Tunnel/FeatureIndicators/ConnectionView.swift line 36 at r2 (raw file):

        VStack {
            Spacer()
            ConnectionView()

for making rendering faster I suggest to shorten preview code with :

Code snippet:

#Preview {
    ConnectionView()
        .background(UIColor.secondaryColor.color)
}

---

ios/MullvadVPN/View controllers/Tunnel/FeatureIndicators/ConnectionView.swift line 43 at r2 (raw file):

}

private struct BlurView: View {

I really recommend to make a separate file for this reusable class and make it public or internal.

---

ios/MullvadVPN/View controllers/Tunnel/FeatureIndicators/ConnectionView.swift line 60 at r2 (raw file):

                .font(.title3.weight(.semibold))
                .foregroundStyle(UIColor.successColor.color)
                .padding(.bottom, 4)

let's use UIMetrics's paddings when they are already there.

---

ios/MullvadVPN/View controllers/Tunnel/FeatureIndicators/ConnectionView.swift line 61 at r2 (raw file):

                .foregroundStyle(UIColor.successColor.color)
                .padding(.bottom, 4)
            Text("Country, City")

don't forget to use localized string and this goes for the rest strings :

Code snippet:

LocalizedStringKey("City")

---

ios/MullvadVPN/View controllers/Tunnel/FeatureIndicators/ConnectionView.swift line 75 at r2 (raw file):

        VStack(spacing: 16) {
            SplitMainButton(
                text: "Switch location",

Localization should be considered.

---

ios/MullvadVPN/Views/MainButtonStyle.swift line 20 at r2 (raw file):

    func makeBody(configuration: Configuration) -> some View {
        configuration.label
            .padding(.horizontal, 8)

let's use padding we already have in UIMetrics.

---

ios/MullvadVPN/Views/SplitMainButton.swift line 12 at r2 (raw file):

struct SplitMainButton: View {
    var text: String

Localization

---

ios/MullvadVPN/Views/SplitMainButton.swift line 35 at r2 (raw file):

                    .resizable()
                    .scaledToFit()
                    .padding(4)

there is a value for that in UIMetrics

---

ios/MullvadVPN/Views/MainButton.swift line 2 at r2 (raw file):

//
//  Untitled.swift

//  MainButton.swift

---

ios/MullvadVPN/Views/MainButton.swift line 12 at r2 (raw file):

struct MainButton: View {
    var text: String

localization is forgotten.

[rablador]

Reviewable status: 7 of 8 files reviewed, 13 unresolved discussions (waiting on @acb-mv, @buggmagnet, and @mojganii)

---

ios/MullvadVPN/View controllers/Tunnel/FeatureIndicators/ConnectionView.swift line 18 at r2 (raw file):

Previously, mojganii wrote…

we already have property for that ' UIMetrics.padding16'

I know, but I don't think .padding16 helps very much. The name implies that it can never be anything but 16, which means that it's basically just a hard coded value in the form of a constant (for the sake of being a constant). If it was UIMetrics.ConnectionViewSpacing or similar, we could have taken advantage of it being more "dynamic" and not using hard coded values.

Also, since ConnectionView is not generic and reused anywhere else it doesn't matter if we use specific hardcoded values in certain places I think.

I'm not totally against making changes, but I think it deserves a discussion. What does @acb-mv @buggmagnet think?

---

ios/MullvadVPN/View controllers/Tunnel/FeatureIndicators/ConnectionView.swift line 22 at r2 (raw file):

Previously, mojganii wrote…

same above for the rest

Discussion in https://reviewable.io/reviews/mullvad/mullvadvpn-app/7290#-ODotBA_0KqXZmMiRa2q above.

---

ios/MullvadVPN/View controllers/Tunnel/FeatureIndicators/ConnectionView.swift line 28 at r2 (raw file):

Previously, mojganii wrote…

does the comment imply that we can't use .frame(width: .infinity)

Yes, but thanks to the snippet you sent me before we can get rid of this. It will be changed in an upcoming PR. :)

---

ios/MullvadVPN/View controllers/Tunnel/FeatureIndicators/ConnectionView.swift line 36 at r2 (raw file):

Previously, mojganii wrote…

for making rendering faster I suggest to shorten preview code with :

We could, but it would not show the connection view at the bottom of the screen like in production. I'm fine either way and can change it if you insist. :)

---

ios/MullvadVPN/View controllers/Tunnel/FeatureIndicators/ConnectionView.swift line 43 at r2 (raw file):

Previously, mojganii wrote…

I really recommend to make a separate file for this reusable class and make it public or internal.

I agree. It has been fixed in an upcoming PR.

---

ios/MullvadVPN/View controllers/Tunnel/FeatureIndicators/ConnectionView.swift line 60 at r2 (raw file):

Previously, mojganii wrote…

let's use UIMetrics's paddings when they are already there.

Discussion in https://reviewable.io/reviews/mullvad/mullvadvpn-app/7290#-ODotBA_0KqXZmMiRa2q above.

---

ios/MullvadVPN/View controllers/Tunnel/FeatureIndicators/ConnectionView.swift line 61 at r2 (raw file):

Previously, mojganii wrote…

don't forget to use localized string and this goes for the rest strings :

There's a TODO at the top of the file addressing this. All hard coded texts will be replaced with proper localized texts in an upcoming PR.

---

ios/MullvadVPN/View controllers/Tunnel/FeatureIndicators/ConnectionView.swift line 75 at r2 (raw file):

Previously, mojganii wrote…

Localization should be considered.

See https://reviewable.io/reviews/mullvad/mullvadvpn-app/7290#-ODoyAG84xsZr6VBUXao

---

ios/MullvadVPN/Views/MainButton.swift line 2 at r2 (raw file):

Previously, mojganii wrote…

//  MainButton.swift

Done.

---

ios/MullvadVPN/Views/MainButton.swift line 12 at r2 (raw file):

Previously, mojganii wrote…

localization is forgotten.

See https://reviewable.io/reviews/mullvad/mullvadvpn-app/7290#-ODoyAG84xsZr6VBUXao

---

ios/MullvadVPN/Views/MainButtonStyle.swift line 20 at r2 (raw file):

Previously, mojganii wrote…

let's use padding we already have in UIMetrics.

Discussion in https://reviewable.io/reviews/mullvad/mullvadvpn-app/7290#-ODotBA_0KqXZmMiRa2q above.

---

ios/MullvadVPN/Views/SplitMainButton.swift line 12 at r2 (raw file):

Previously, mojganii wrote…

Localization

See https://reviewable.io/reviews/mullvad/mullvadvpn-app/7290#-ODoyAG84xsZr6VBUXao

---

ios/MullvadVPN/Views/SplitMainButton.swift line 35 at r2 (raw file):

Previously, mojganii wrote…

there is a value for that in UIMetrics

Discussion in https://reviewable.io/reviews/mullvad/mullvadvpn-app/7290#-ODotBA_0KqXZmMiRa2q above.

[acb-mv]

Reviewable status: 7 of 8 files reviewed, 13 unresolved discussions (waiting on @buggmagnet and @mojganii)

---

ios/MullvadVPN/View controllers/Tunnel/FeatureIndicators/ConnectionView.swift line 18 at r2 (raw file):

Previously, rablador (Jon Petersson) wrote…

I know, but I don't think .padding16 helps very much. The name implies that it can never be anything but 16, which means that it's basically just a hard coded value in the form of a constant (for the sake of being a constant). If it was UIMetrics.ConnectionViewSpacing or similar, we could have taken advantage of it being more "dynamic" and not using hard coded values.

Also, since ConnectionView is not generic and reused anywhere else it doesn't matter if we use specific hardcoded values in certain places I think.

I'm not totally against making changes, but I think it deserves a discussion. What does @acb-mv @buggmagnet think?

I think padding16 is a slightly silly name. If it's going to be a number, it should refer not to the number of pixels but something like a multiple of a grid unit or something, though if so, that would be a question for Carl. If we're not using numbers (and arguably we shouldn't be), the name should refer to where it's used: ConnectionViewSpacing could be a candidate, though finding other use cases we have something in common with and naming the constant after them would be better.

[buggmagnet]

Reviewable status: 7 of 8 files reviewed, 13 unresolved discussions (waiting on @mojganii)

---

ios/MullvadVPN/View controllers/Tunnel/FeatureIndicators/ConnectionView.swift line 18 at r2 (raw file):

Previously, acb-mv wrote…

I think padding16 is a slightly silly name. If it's going to be a number, it should refer not to the number of pixels but something like a multiple of a grid unit or something, though if so, that would be a question for Carl. If we're not using numbers (and arguably we shouldn't be), the name should refer to where it's used: ConnectionViewSpacing could be a candidate, though finding other use cases we have something in common with and naming the constant after them would be better.

At some point in the past, we decided that if a magic number appears in a single file, and within a specific context, we should leave it as that, since it's usually self explanatory.

Semantics are usually important, and I think calling a spacing "padding" would be semantically incorrect, therefore I agree that we should not use padding 16

@acb-mv we came up with the nomenclature paddingX (where X is a number) to add some context instead of using said number as a result of a discussion where we didn't want to have magic numbers peppered through the codebase.

I agree that the name is not the best, but sometimes we do need a generic number that we reuse in different contexts where ConnectionViewSpacing would be misleading instead of a more generic paddingX

Overall, I think we can leave that as is.

[mojganii]

Reviewable status: 7 of 8 files reviewed, 12 unresolved discussions (waiting on @buggmagnet and @rablador)

---

ios/MullvadVPN/View controllers/Tunnel/FeatureIndicators/ConnectionView.swift line 36 at r2 (raw file):

Previously, rablador (Jon Petersson) wrote…

We could, but it would not show the connection view at the bottom of the screen like in production. I'm fine either way and can change it if you insist. :)

I don't insist but I think for subviews we shouldn't have cared about the position it's gonna take in the future

[rablador]

Reviewable status: 7 of 8 files reviewed, 12 unresolved discussions (waiting on @buggmagnet and @mojganii)

---

ios/MullvadVPN/View controllers/Tunnel/FeatureIndicators/ConnectionView.swift line 36 at r2 (raw file):

Previously, mojganii wrote…

I don't insist but I think for subviews we shouldn't have cared about the position it's gonna take in the future

I changed it!

[mojganii]

Reviewable status: 6 of 8 files reviewed, 12 unresolved discussions (waiting on @buggmagnet and @rablador)

---

ios/MullvadVPN/View controllers/Tunnel/FeatureIndicators/ConnectionView.swift line 22 at r2 (raw file):

Previously, rablador (Jon Petersson) wrote…

Discussion in https://reviewable.io/reviews/mullvad/mullvadvpn-app/7290#-ODotBA_0KqXZmMiRa2q above.

I'm fully aware of that discussion, my point is if we are not going to use them, then we have to remove them.

[github-actions]

🚨 End to end tests failed. Please check the failed workflow run.