-
Notifications
You must be signed in to change notification settings - Fork 1.7k
🔸 Swift Rewrite 🔸 #450
base: master
Are you sure you want to change the base?
🔸 Swift Rewrite 🔸 #450
Conversation
…no luck. Will circle back to this later...
* Removes SnapKit from EmptyDataSet * Removes weak self from UIKit animation block as it's not required
…456) * Removes SnapKit from EmptyDataSet * Removes weak self from UIKit animation block as it's not required * Adds early exit to isEmpty function to prevent unnecessary calculation * Removes SnapKit from Bento
Co-Authored-By: Eytan Schulman <[email protected]>
Co-Authored-By: Eytan Schulman <[email protected]>
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.
In my opinion this Swift rewrite is almost ready, with some slight changes, but in a separate project, maybe DZNEmptyDataSet-Swift
as it does not have full feature parity with DZNEmptyDataSet
, like you noted. I think this could totally be released as a simple Swift Package
, and people could start using it 😄
} | ||
} | ||
|
||
extension ViewController: EmptyDataSetSource { |
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.
Does not conform to protocol EmptyDataSetSource
, needs to implement:
imageTintColor(forEmptyDataSet scrollView: UIScrollView)
|
||
protocol EmptyDataSetProtocol { | ||
func swizzle() -> Bool | ||
func isEmpty() -> Bool |
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 method isEmpty()
doesn't seem to be used anywhere, only implemented. Is this intended?
|
||
public var isEmptyDataSetVisible: Bool { | ||
// guard let view = emptyDataSetView else { return false } | ||
// return !view.isHidden |
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.
If this is the default implementation (returning true
) then this should be removed
I'm working on a new project that requires loads of empty data set, so might as well rewrite this lib in Swift. A rewrite has been long due.
Just putting the PR out there for visibility for now. I need eyes on this code, specially for reviewing and applying good practices. I'm looking to simplify the code as much as possible, while still being easy to read and inviting for contributions.
This rewrite won't be exactly how it was proposed on #174.
This won't be:
Initial prototype is looking crisp.