Refactor PetitionsController to use programmatic UI - #26
Conversation
|
I am doing something very similar in https://github.com/Say-Their-Name/ios/tree/HomeController-programmatic-Layout. Main difference is I broke out a protocol "ServiceClient" from BaseViewController. |
|
I like the approach. One question, do you think we should rename PetitionUI to be something more general so that it can be re-used by the dontations UI. they seem to be very similar according to the mocks |
|
it's middle of the night at @Kilo-Loco 's location, @unnamedd would you like to take over and implement the fixes? |
Even if I can do it @hybridcattt, I guess the benefit of the code review is the sharing knowledge. I can do, but then I won't have his opinion about it. I rather wait for him. |
|
Your suggestions are minor and very reasonable so I'm sure @Kilo-Loco would not disagree! This project is collaborative so we just try to do our best to move the project forward and release it as soon as reasonably possible 🙂 |
hybridcattt
added
the
please take over
hybridcattt
left a comment
hybridcattt
left a comment
There was a problem hiding this comment.
Noticed an issue with implementation of PetitionsViewCode. Please have a look 🙏
| override func layoutSubviews() { | ||
| super.layoutSubviews() | ||
|
|
||
| setupSubviews() |
There was a problem hiding this comment.
layoutSubviews is called quite often, and the setup needs to happen only once. So it's better to put it in setupSelf method instead
| /// The UI for Petitions | ||
| final class PetitionsViewCode: UIView { | ||
|
|
||
| private lazy var petitionsLabel: UILabel = { |
There was a problem hiding this comment.
| private lazy var petitionsLabel: UILabel = { | |
| private let petitionsLabel: UILabel = { |
will work the same, just without the ability to accidentally overwrite the variable 🙂
| return label | ||
| }() | ||
|
|
||
| init() { |
There was a problem hiding this comment.
init(frame: CGRect) will work exactly the same, allowing to use both init() and init(frame:). It's due to how initializers are inherited, can read more here (though it's a long read) : https://docs.swift.org/swift-book/LanguageGuide/Initialization.html
hybridcattt
removed
the
please take over
6 participants
This commit is being used to propose a potential MVC design pattern.
PLEASE COMMENT ON THIS PR IF YOU DISAGREE WITH THIS PROPOSAL