-
Notifications
You must be signed in to change notification settings - Fork 372
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
Animate expansion of connection details view #7548
base: main
Are you sure you want to change the base?
Conversation
bb3c5ae
to
96ea06d
Compare
2bc4ae5
to
f113e11
Compare
This reverts commit f113e11. This solution results in black bars on iOS version <18
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a black backround behind the buttons that should not be there:
Reviewable status: 0 of 10 files reviewed, 1 unresolved discussion (waiting on @SteffenErn)
ios/MullvadVPN/View controllers/Tunnel/ConnectionView/ConnectionView.swift
line 18 at r3 (raw file):
@State private(set) var showConnectionDetailsAnimated = false @State private(set) var isExpandedAnimatied = false
Nit: Spelling: "Animatied"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 10 files reviewed, 1 unresolved discussion (waiting on @SteffenErn)
ios/MullvadVPN/View controllers/Tunnel/ConnectionView/ConnectionView.swift
line 17 at r3 (raw file):
@State private(set) var isExpanded = false @State private(set) var showConnectionDetailsAnimated = false
I think we always want to do these things animated, so we don't need to specify that in the variable names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 3 of 10 files at r1, 6 of 7 files at r2, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @SteffenErn)
ios/MullvadVPN/Extensions/View+Modifier.swift
line 12 at r4 (raw file):
extension View { func apply<V: View>(@ViewBuilder _ block: (Self) -> V) -> V { block(self) }
We should add documentation here to clarify what the function is used for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @SteffenErn)
ios/MullvadVPN/View controllers/Tunnel/ConnectionView/ConnectionView.swift
line 18 at r4 (raw file):
@State private(set) var showConnectionDetailsAnimated = false @State private(set) var isExpandedAnimatied = false
typo : isExpandedAnimated
ios/MullvadVPN/View controllers/Tunnel/ConnectionView/ConnectionView.swift
line 40 at r4 (raw file):
.background(BlurView(style: .dark)) .cornerRadius(12) .padding(EdgeInsets(top: 16, leading: 16, bottom: 24, trailing: 16))
why did you remove constants?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 3 of 10 files at r1, 6 of 7 files at r2, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @SteffenErn)
It is finally time for some fancy animations of the connection view. A few changes to the structure, lots of pain and finally the salvation through
.transformEffect(.identity)
was required.Some of the newly introduced
State
variables seem unnecessary, but it doesn't work properly without them.Enjoy!
This change is