Skip to content

Conversation

kyleve
Copy link
Contributor

@kyleve kyleve commented Mar 23, 2023

Based on the work done in #106, so that way you can do an adaptation before, eg you map something to a Screen.

Checklist

  • Unit Tests
  • UI Tests
  • Snapshot Tests (iOS only)
  • I have made corresponding changes to the documentation

@@ -30,7 +30,7 @@ import ViewEnvironment
/// .adaptedEnvironment(keyPath: \.myValue, to: newValue)
/// ```
///
public struct AdaptedEnvironmentScreen<Content: Screen>: Screen {
Copy link
Contributor

Choose a reason for hiding this comment

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

thinking out loud: this change just makes things more permissive – you can now adapt any content type, not just screens (in practice would this be some form of Element?).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll defer to @bencochran here since he's the one who usually structures things this way, but eg, it's common to build a back stack wrapping some type of view model, etc, and then map that into a screen type after you fetch it. This allows you to use environment adaptation alongside that.

@@ -30,7 +30,7 @@ import ViewEnvironment
/// .adaptedEnvironment(keyPath: \.myValue, to: newValue)
/// ```
///
public struct AdaptedEnvironmentScreen<Content: Screen>: Screen {
public struct AdaptedEnvironmentScreen<Content> {
/// The screen wrapped by this screen.
public var wrapped: Content
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: the parameter names & comments now seem slightly mis-matched if the generic type is unconstrained

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think in reality, this is going to be 99% screens / mapped to one eventually, so I left it as is.

@kyleve kyleve merged commit 3e703ce into main Mar 24, 2023
@kyleve kyleve deleted the kve/adapted-screen-conformance branch March 24, 2023 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants