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: Add View extension based on CompatibilityNavigationStack #4677

Merged
merged 1 commit into from
Jan 17, 2025

Conversation

facumenzella
Copy link
Contributor

Motivation

To avoid having conditional navigation based on iOS versions everywhere, this adds a wrapper to navigate smoothly. Showing here how it is used

Copy link
Member

@ajpallares ajpallares left a comment

Choose a reason for hiding this comment

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

This is nice. However, I don't think we need to provide the second method that allows passing a Binding<Item?>, as it is the exact same implementation but only providing a bridge from Binding<Item?> to Binding<Bool>. IMHO, that bridge can be done by the caller of compatibleNavigation. As an alternative, we could provide a convenient bridging from Binding<Item?> to Binding<Bool> independent from this compatibleNavigation method, as it is very common to have this bridging need. WDYT @facumenzella?

@facumenzella facumenzella force-pushed the feat/add-compatibility-nav branch from 80d0def to 3154623 Compare January 17, 2025 12:19
Copy link
Member

@ajpallares ajpallares left a comment

Choose a reason for hiding this comment

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

I think it's much cleaner now. Thanks @facumenzella!

@facumenzella facumenzella enabled auto-merge (squash) January 17, 2025 12:22
@facumenzella facumenzella merged commit ee1f749 into main Jan 17, 2025
9 of 10 checks passed
@facumenzella facumenzella deleted the feat/add-compatibility-nav branch January 17, 2025 12:31
This was referenced Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants