-
Notifications
You must be signed in to change notification settings - Fork 118
[WCiOS17] Update PaymentMethodsView
to not use deprecated NavigationLink
init
#16196
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
base: trunk
Are you sure you want to change the base?
Conversation
Seems to crash the previews macro if we do not provide a rootViewController, despite being optional
|
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.
Works well, thanks for fixing these deprecated usage! 🚀
.previewDisplayName("Light") | ||
#Preview("Light") { | ||
@Previewable @State var rootViewController = UIViewController() | ||
NavigationView { |
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.
nit: as NavigationView
is deprecated, would be nice to replace it with NavigationStack
. Simply replacing it with NavigationStack
seems to work. Similarly for other previews in this file.
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.
That's very true, updated: 84d2e6a
) | ||
.navigationBarTitleDisplayMode(.inline) | ||
} | ||
.environment(\.colorScheme, .dark) |
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.
nit: unrelated to this PR, I think we could just keep one preview here since the differences are just the color scheme and accessibility font size which can be configured in SwiftUI previews. It's more worth having multiple previews when the data/states are different.
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.
Yeah, that sounds good to me. Updated: 84d2e6a
Closes WOOMOB-1406
Description
As with #16195, this PR updates the
PaymentMethodsView
component from Order > Collect Payment to not use deprecatedNavigationLink(isActive:)
API for navigation. It also fixes the file previews, which seem to crash in Xcode due missing dependencies.Changes
NavigationLink(isActive:
in favor of.navigationDestination(isPresented:
view modifierTesting information
Cash
as payment methodScreenshots