-
Notifications
You must be signed in to change notification settings - Fork 120
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
Improve Layout and Constraint Implementation (so that the control works well in Interface Builder.) #118
Comments
This article offered a red herring but ultimately led me closer to my solution: |
What Went Wrong
So it turns out™ that the cells were incorrectly spaced because the layout engine was doing cell arithmetic instead of properly pinning the views to each other. In a pre-autolayout world, this was accurate and fast. Once autolayout came into play, I made changes to account for This increasingly complex layout environment meant that somewhere, the dimensions of each cell, which almost all of the old layout code was based on, ended up wrong in one of the layout passes. Trying on Autolayout Constraints for SizeMy first solution was to pin the cells' trailing and leading edges using constraints. With this approach, I'm still using manually recycled Using constraints fixed the layout inconsistencies by leaning on auto layout to guarantee correct pinning, instead of basing it on the width of the calendar view. It also had the added benefit of correct rendering in localized right-to-left environments. The tradeoff here was an unacceptable performance hit. Running in the simulator on a quad core machine, the demo app performed fine, but an iPod touch running iOS 9 took well over a minute to switch between months. (Instruments verified this.) Layout Constraints, Memory Constraints, Performance ConstraintsI tried iterating on this by caching constraints on the cells, using This wasn't how it worked out. Caching constraints reduces the time spent creating new constraints on subsequent layout passes (by allocating memory for them only once) but it doesn't solve for the first time a cell is pinned to another cell. So if Cell A is pinned to Cell B the first time around, and then is pinned to Cell C, the constraint is created a second time. This is even worse from a memory standpoint, because even though we have a cache with pretty good access, memory usage is factorial - we have one constraint on each cell for every other cell. A grid requires pinning in both the horizontal and the vertical direction, so, not that it makes a huge difference, but the performance becomes More importantly, creating the constraints isn't the expensive part of the layout operation - activating the constraints is. "Activating" a constraint essentially causes the auto layout engine to solve the layout again. Calling Doing this about 60 times per transition is what was causing the delay. 🤦♂️ I also want to call out that the documentation for NSLayoutConstraint activateConstraints:` says that this method is "usually more efficient" than manually activating each constraint, but profiling has shown that it didn't seem to make a difference in this case. Collecting the Pieces with Collection ViewFinally, I began to try laying out the cells with This brings me to the point of this comment, which is that this issue is now blocked by #109 Upgrade to UICollectionView as well. It's a work in progress, but so far I'm happy with it. Final ThoughtsHow
|
Collection View is a Go 👍I got this to work nicely with BenefitsThis actually enabled a large number of enhancements to the framework by itself, including proper right-to-left support, and week view animation. What Actually Went Wrong! 🤦♂️While I was working on this, though, I came across something really interesting. The reason the why the cell spacing was wrong initially was because views that are instantiated in a storyboard have a default bounds size of 600 pixels by 600 pixels. The size doesn't change until the view moves to the window, by which time the cells have already been laid out. The simplest fix would have been to reload the cells in Known BugsThere is one slightly noticeable issue here, which is that when transitioning between months that have a different number of weeks visible, there are gaps in the animation. This is less notificable if the background color and the cell color is the same. (which it is by default, so I'm not worrying about it for 5.0.0) |
Known Bugs ReduxThe issue with the cells being slightly off in transitions from month to month with different week lengths is because we're using the |
Going to open an issue for that transition bug, and then close this. |
Opened as #125 |
Summary
Right now
CKCalendarView
on the 5.0.0 branch works pretty decently with auto layout when instantiated with code. The problem is that the way layout is defined, we do all of the work inupdateConstraints
. Views are installed, constraints are created, and constraints are updated. This is far from ideal, for a number of reasons. (See the sources at the bottom for more on this.)Expected Behavior
The control should work correctly when instantiated from code and from Interface Builder.
Actual Behavior
The control works almost correctly in code (except for initial layout) and completely incorrectly in Interface Builder.
Notes
I noticed that when I moved my subview installation logic from
layoutSubviews
toupdateConstraints
, constraint based cell animations became much smoother. However, when I tried to add Interface Builder support, I noticed that things weren't going right.Instances of the calendar view that were created in code initially were a little too short. Calendars created in Interface Builder had too much space between the cells until the user forced a layout pass.
Interface Builder itself showed a skewed version of the calendar, with a similar problem. Here, though, the bottom border rendered fine, but the calendar itself occupies the top left corner of the control, instead of the full size. The same cell spacing problem occurs as when the code is run.
Digging in a little more deeply, I realized that my layout setup was probably incorrect. So it turns out, that even though it made sense to me to have a single method for installing each view, (
_installTable
,_installHeader
, etc) this design ignores the structure ofUIView
layout. I need to rethink this.The fix here is probably to go through the view lifecycle and properly set up all of the parts at the appropriate times.
Sources:
updateConstraints
(Ole Begemann)layoutSubviews()
documentation (Apple)updateConstraints()
documentation (Apple)The text was updated successfully, but these errors were encountered: