Conversation
mtnbarreto
left a comment
mtnbarreto
left a comment
There was a problem hiding this comment.
Hi @chamitha. The code looks ok to me! Let's me use it a little bit to make sure it works.
Have you tested it adding the cocoapods dependency?
Yes I did. |
| super.customDidSelect() | ||
|
|
||
| guard let controller = makeController() else { return } | ||
| guard !isDisabled else { return } |
There was a problem hiding this comment.
Maybe we can allow the user to set up a title and use the detailTextView as the value. By doing that the user can opt by leaving empty the title and we will get the same result as now.
I would recommend to add the cell clear button as cell accessoryView.
We also have to change cell color title when row is disabled in order to indicate that.
Take a look at switchCell to see how we do it.
There was a problem hiding this comment.
I would recommend to add the cell clear button as cell accessoryView.
Done in f04a8b0
There was a problem hiding this comment.
We also have to change cell color title when row is disabled in order to indicate that.
Done in ccd09f6
There was a problem hiding this comment.
Maybe we can allow the user to set up a title and use the detailTextView as the value. By doing that the user can opt by leaving empty the title and we will get the same result as now.
@mtnbarreto There are a couple of reasons I would prefer the leave this as is (at least for the moment), where only the textLabel is used to display the location value / placeholder.
- For a consistent look and feel with the Apple location field in the Calendar app.
- Setting the title (
textLabel) text to empty / nil does not auto left align thedetailTextLabelinUITableViewCell. I can achieve this by adding constraints however the look and feel of the cell can then dynamically change from a left aligned title and right aligned value to no title and a left aligned value depending on the value ofrow.title. I don't particularly like this dynamic change in look and feel. This too of course can be further enhanced by allowing the user to choose the layout style, say title and value or value only but I rather implement this as a future improvement.
Let me know if these reasonings are acceptable.
|
@mtnbarreto |
1 similar comment
|
@mtnbarreto |
2 participants
Present the location selector view controller as modal ala the iOS Calendar app.