-
-
Notifications
You must be signed in to change notification settings - Fork 113
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Add View Traits and transitions #426
Conversation
dom: DOMNode, | ||
additionalAttributes: [HTMLAttribute: String], | ||
transaction: Transaction | ||
) { |
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.
Would it be possible to split the body of this function into multiple functions or some helper types to resolve the linter warning?
@@ -98,24 +98,29 @@ public class MountedElement<R: Renderer> { | |||
} | |||
} | |||
|
|||
public internal(set) var viewTraits: _ViewTraitStore |
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.
Are these traits somehow different from SwiftUI accessibility traits? I can't find anything else about SwiftUI traits, just wondering what's used as a reference here.
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.
SwiftUI has _ViewTraitKey
internally, and uses them for transition
, onDelete
, onMove
and some other modifiers (I think drag & drop?). From what I can tell, a trait is data attached to a single view, and in many cases seems to be a way to get the trait out of SwiftUI and into the host platform (for instance, for UITableView swipe actions or NSItemProvider-based drag & drop).
Although I'm really just speculating as to how SwiftUI uses them by looking at the swiftinterface
. In Tokamak they are meant to be passed to the nearest host element so renderers can access them.
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.
Traits are also used for zIndex(_:)
and layoutPriority(_:)
. I haven't look into whether Tokamak has implemented these modifiers. If they're not implemented, traits would be really helpful.
Text(".slide")
.transition(AnyTransition.slide)
VStack {
Text(".slide").transition(AnyTransition.slide)
Text(".slide").transition(AnyTransition.slide)
}
Text(".slide")
.transition(AnyTransition.slide)
VStack {
Text(".slide")
Text(".slide")
}.transition(AnyTransition.slide)
|
_ style: String, | ||
on dom: DOMNode, | ||
with animation: Animation | ||
) { |
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.
I realize these linter warnings are a bit annoying, but is it still possible to get it a bit slimmer and under 50 lines? Maybe would work better from code readability perspective too.
1-3 should be fixed. |
My idea to fix 4 atm is to keep a list of |
1 and 2 look good. For 3, the slide-out looks good. However, for the slide-in, lines 2-3 just appear rather than sliding. |
@State private var showText = "Show"
...
Button(isVisible ? "Hide" : showText) {
...
Text(".opacity")
.transition(AnyTransition.opacity)
.onDisappear(perform: {
showText = "Show Again"
}) In SwiftUI, the button text changes to "Show" when you click "Hide", then changes to "Show Again" when the animation is complete. With your code, it changes from "Hide" to "Show Again" immediately upon clicking "Hide". |
Seems like it only affects |
4 should be fixed now too. |
There's already a |
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.
For 3, the slide-in is still behaving incorrectly.
For 4, the fix is causing another problem. If you switch from Transition to a different section of the demo while a transition is happening, the animating HTML will appear in the new section for the remaining duration of the transition animation.
Also, I'm now getting an error when rapidly pressing the inner button:
|
The slide-in bug should be fixed. Seems like the |
Looks good! You are getting a couple of linter warnings now, so you may want to refactor a bit before merge. |
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.
👏
This adds
_ViewTraitKey
and related internal modifiers. Then transition builds on top of these traits.Here's the transition demo:
https://user-images.githubusercontent.com/13581484/126044318-b2856cbb-56ed-4196-a0fd-6ea5c6c606c7.mov
It works by applying the transition's modifier to the a View on mount/update/unmount.