-
Notifications
You must be signed in to change notification settings - Fork 266
Conversation
view.frame = frame | ||
// If we are preparing an animation, then we don't want to update frames or configure views. | ||
if let view = layout.makeView(from: recycler, configure: !prepareAnimation) { | ||
if !prepareAnimation { |
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.
Looks like there are conceptually two tree walks here, one to create or update the sub/superview tree and another to set the frames and other configured properties. Sometimes we want to run them together and sometimes separately. Perhaps this logic would be cleaner as separate functions instead of passing down the flag throughout?
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.
It is certainly possible to do this in two passes, but I think the code would be more complicated due to view collapsing. It is nice that we can do it incrementally, e.g.:
if !prepareAnimation {
for subview in subviews {
subview.frame.offsetInPlace(dx: frame.origin.x, dy: frame.origin.y)
}
}
ee80fb1
to
85174ac
Compare
|
||
## Requirements | ||
|
||
1. Use the `tag` parameter to identify layouts involved in the 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.
rename tag
to id
|
||
/** | ||
An identifier for the layout. | ||
It is used to identify which views should be reused when animating from one layout to another. |
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'd add a bit more detail here, talking about how applying two layouts reuses views by this id. Maybe even rename it to viewReuseId?
Also somewhere we should describe what happens if there is a duplicate.
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.
viewId
Going to merge this into master. This is probably not the final API since we want to do #24 before releasing 2.0. |
No description provided.