Call Script Screen Detail #16
Conversation
|
|
.gitignore
Outdated
| @@ -1,6 +1,7 @@ | |||
| # Xcode | |||
| # | |||
| # gitignore contributors: remember to update Global/Xcode.gitignore, Objective-C.gitignore & Swift.gitignore | |||
| .DS_Store | |||
There was a problem hiding this comment.
This should be in your system gitignore ~/.gitignore_global
There was a problem hiding this comment.
TIL that is a thing
| @IBOutlet weak var resultUnavailableBtn: UIButton! | ||
| @IBOutlet weak var resultVoicemailBtn: UIButton! | ||
| @IBOutlet weak var resultContactedBtn: UIButton! | ||
| @IBOutlet weak var resultSkipBtn: UIButton! |
There was a problem hiding this comment.
In the interest of consistency, can you rename these to ...button ?
| print("unknown button pressed") | ||
| } | ||
|
|
||
| // Struct passing around problems. This needs to be refactored / cleaned up. |
| if issuesManager.issuesList!.issues[i].id == self.issue.id { | ||
| issuesManager.issuesList!.issues[i].madeCalls = true | ||
| } | ||
| } |
There was a problem hiding this comment.
So I'm not married to structs tbh, it's just what I start with for JSON models. But the case could be made that we're using IssueManager.issuesList as an in memory data store, and that it should be mutable. That would avoid us having this code anywhere (helper or wherever).
There was a problem hiding this comment.
Another thing to consider is we will need some form of persistence soon, right now we use UserDefaults for your location settings which seems appropriate, but when it comes to saving these stats locally, we'll have to probably put them somewhere else (I was thinking of using @nickoneill's Pantry). However if we decided to just leverage a data store we could go with Realm or CoreData also. Food for thought.
There was a problem hiding this comment.
Oooh Pantry looks nice. All we really need to store is if we call certain issue IDs and contact IDs for those issues. Then we just need to pull in data. May try to implement that in a separate PR tonight or tomorrow.
| } | ||
|
|
||
| func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell { | ||
|
|
FiveCalls/FiveCalls/Contact.swift
Outdated
| var hasContacted: Bool | ||
| } | ||
|
|
||
| extension Contact: Hashable { |
There was a problem hiding this comment.
do we get Equatable for free w/ this?
There was a problem hiding this comment.
Yeah I forgot to pull these, I made it Hashable to put it in a Set but removed that code, but yes would make it Equatable as well.
| @@ -42,6 +43,6 @@ extension Issue : JSONSerializable { | |||
|
|
|||
| let contacts = contactsJSON.flatMap({ Contact(dictionary: $0) }) | |||
|
|
|||
| self.init(id: id, name: name, reason: reason, script: script, contacts: contacts) | |||
| self.init(id: id, name: name, reason: reason, script: script, contacts: contacts, madeCalls: false) | |||
There was a problem hiding this comment.
Here's another thing worth considering:
- JSON response -> structs -> immutable
- Local app data -> classes -> mutable
Then you just merge these in view models when dealing w/ the UI. You have a lightweight set of mutable data that just refers to issue ids.
There was a problem hiding this comment.
Yeah I think that's the way to do it, very simple local app data that references the JSON Structs.
There was a problem hiding this comment.
Do you want to handle that as a separate PR so we can merge this in? (last things I think are the padding + status bar style, which should be pretty quick)
There was a problem hiding this comment.
Yeah going to those separately. Padding is done.
|
Padding improved! |
First stab at the call script detail screen as well as hooks into the other systems.
trueNOTE: Because we decided to pass around structs, we end up passing the values not references. So you'll see some messy loops through the Manager in the screen that I'll refactor into helpers on the Manager later this weekend. Wanted to make it work first.