Add ViewEnvironment (#739) #999
Conversation
|
|
||
| /// Returns a new screen that will render the contents of this screen, with | ||
| /// the given environment key path set to the given value. | ||
| public func withEnvironment<Value>(_ keyPath: WritableKeyPath<ViewEnvironment, Value>, value: Value) -> EnvironmentScreen<Value, Self> { |
There was a problem hiding this comment.
This extension is getting a little SwiftUI-y. Thoughts?
There was a problem hiding this comment.
Can we make it more SwiftUI-y?
There was a problem hiding this comment.
For the full effect I could spell it this way:
public func environment<Value>(_ keyPath: WritableKeyPath<ViewEnvironment, Value>, _ value: Value) -> EnvironmentScreen<Value, Self>There was a problem hiding this comment.
I tend to shy away from unnamed parameters, but in the case of this one I think it’s actually alright?
ex:
someScreen
.environment(\.theme, DarkTheme())
.environment(\.padding, 24)| } | ||
|
|
||
| private enum SplitScreenPositionKey: ViewEnvironmentKey { | ||
| static var defaultValue: SplitScreenPosition = .none |
There was a problem hiding this comment.
This is cool! I'm kinda blown away that a private type's internal static var is accessible from outside this file.
There was a problem hiding this comment.
SplitScreenPositionKey is not accessed from outside this file.
There was a problem hiding this comment.
But it's defaultValue var is.
|
|
||
|
|
||
| /// A wrapper screen that overrides environment values | ||
| public struct EnvironmentScreen<Value, Content> { |
There was a problem hiding this comment.
Is there value in this? Without this ViewEnvironment is fully contained within viewControllerDescription and UIViewControllers.
There was a problem hiding this comment.
ViewControllerDescription itself doesn’t have any knowledge of ViewEnvironment.
My desire for this is for wrapper workflows to be able to provide environment values without having to implement a wrapper screen themselves. Let’s use automatic day/night theme switching as an example (ignoring iOS’s dark mode support for a moment). I could have a workflow that has itself subscribes to some signal for time of day, changing its state to .day or .night as appropriate. With this extension, it’s render could be:
func render(state: State, context: RenderContext<AutoNightModeWorkflow>) -> Rendering {
return wrappedWorkflow
.mapRendering { $0.environment(\.theme, Theme(style: state.timeOfDay) }
.rendered(with: context)
}
There was a problem hiding this comment.
Doesn't this break the UI/Workflow decoupling? ViewEnvironment is purely a view concern, so the workflows shouldn't know about it.
There was a problem hiding this comment.
IMO, it’s okay for a workflow to put values into the environment. But not to take them out. We absolutely could make a workflow like the one described above do the same job by implementing a wrapper screen type, but it’s effectively going to do the same thing and be the same flow of information.
There was a problem hiding this comment.
Can we have this discussion in a separate PR, please? It's a big change from the Kotlin behavior, for a speculative use case. And it's not required by the general machinery here, right?
There was a problem hiding this comment.
Sure. I’ll kick it out of this PR when I update to fix the tutorial code.
There was a problem hiding this comment.
While I'm thinking about it, though: one thing to note in defense of this class is that container views themselves can transform child renderings into other screen classes. I've already done that a few times, very handy.
That said, I still don't think this class should be a part of this PR.
There was a problem hiding this comment.
Actually, thinking more about this - this might be the primary way for us to set environment values.
| } | ||
|
|
||
| set { | ||
| storage[ObjectIdentifier(key)] = newValue |
There was a problem hiding this comment.
Given struct's value semantics, does this actually mutate the current value? In the Kotlin API ContainerHints is entirely immutable, we don't provide any mutating operators.
There was a problem hiding this comment.
In a struct this is effectively a mutating func, so the mutability follows our standard value semantics (isn’t shared).
So for example you cannot do:
func update(screen: MyScreen, environment: ViewEnvironment) {
environment[SomeKey.self] = 14
}ViewEnvironment is a container of strongly-typed key/value pairs that can be passed down the view-side of a workflow tree. This allows containers to provide view-oriented hinting to their children as well as allows for view-side changes to view-only environment without re-rendering the workflow tree (for example, runtime theme changes).
436627d to
6a46f0e
Compare
|
Removed |
(ViewEnvironment is the Swift implementation of Kotlin’s ContainerHints. I included the straw-man rename of them to ViewEnvironment.)
ViewEnvironment is a container of strongly-typed key/value pairs that can be passed down the view-side of a workflow tree. This allows containers to provide view-oriented hinting to their children as well as allows for view-side changes to view-only environment without re-rendering the workflow tree (for example, runtime theme changes).