Deprecate backgroundView for backgroundViewProvider #123
Conversation
| self.layoutMargins = layoutMargins | ||
| self.cornerRadius = cornerRadius | ||
|
|
||
| struct DefaultBackgroundProvider: BackgroundViewProvider { |
There was a problem hiding this comment.
The DefaultBackgroundProvider in the deprecated init simply wraps and returns the (optionally) provided backgroundView. So we'll still get the functionality in this case.
| public init(topSeparator: Separator.Style? = nil, | ||
| bottomSeparator: Separator.Style? = nil, | ||
| separatorColor: UIColor? = nil, | ||
| highlight: Bool? = nil, | ||
| accessoryType: UITableViewCell.AccessoryType = .none, | ||
| selectionColor: UIColor? = CellStyle.defaultSelectionColor, | ||
| backgroundColor: UIColor? = CellStyle.defaultBackgroundColor, | ||
| backgroundView: UIView? = nil, | ||
| backgroundView: UIView?, |
There was a problem hiding this comment.
Removed the default value here to avoid Ambiguous use of 'init'. This also means CellStyle initializers without an explicitly set backgroundView will default to the new implementation.
scottybobandy
requested review from
dannbeau,
raulriera,
krbarnes and
Caiopia
and removed request for
dannbeau
There was a problem hiding this comment.
LGTM, there is a small typo and I think we can get rid of
else {
let backgroundView = UIView()
backgroundView.backgroundColor = cell.backgroundColor
cell.backgroundView = backgroundView
}This seems to be completely new and probably unrelated to the current changes?
Don't forget the regenerate the documentation again? 😉
| /// An implementation should maintain some internal state about the contents of the view. | ||
| public protocol BackgroundViewProvider { | ||
| /// This is where the background view should be instantiated, since this | ||
| /// function is only called when a cell is being prepared to be deqeueued. |
There was a problem hiding this comment.
Small typo in the last word
scottybobandy
force-pushed
the
deprecate-background-view
branch
from
176c28a
to
93b12ca
Compare
|
The documentation should have been another PR 😬 sorry for not clarifying that |
|
@raulriera No worries - i'll separate them 👍 |
scottybobandy
force-pushed
the
deprecate-background-view
branch
from
93b12ca
to
878e74f
Compare
FunctionalTableData/CellStyle.swift
Outdated
| equality = equality && lhs.tintColor == rhs.tintColor | ||
| equality = equality && lhs.layoutMargins == rhs.layoutMargins | ||
| equality = equality && lhs.cornerRadius == rhs.cornerRadius | ||
| equality = equality && lhs.backgroundViewProvider?.isEqualTo(rhs.backgroundViewProvider) ?? true |
There was a problem hiding this comment.
What if lhs.backgroundViewProvider is nil and rhs.backgroundViewProvider is not?
| equality = equality && lhs.backgroundViewProvider?.isEqualTo(rhs.backgroundViewProvider) ?? true | |
| equality = equality && lhs.backgroundViewProvider?.isEqualTo(rhs.backgroundViewProvider) ?? ((lhs.backgroundViewProvider == nil) == (rhs.backgroundViewProvider == nil)) |
There was a problem hiding this comment.
I think in this case it is sufficient to write the equality check as:
lhs.backgroundViewProvider?.isEqualTo(rhs.backgroundViewProvider) ?? (rhs.backgroundViewProvider == nil)
scottybobandy
force-pushed
the
deprecate-background-view
branch
from
878e74f
to
cbd62f8
Compare
Re-opening this as changes have been pushed to the old branch since
stalebotkilled the PR.The
backgroundViewproperty onCellStyleis expensive - background views are created on every render pass, even though they are rarely used.The
BackgroundViewProviderallowsCellStyleto request an optional background view when its being dequeued; the only time it's actually needed.Fixes #105