Order Details: Add a note #176
Conversation
…te` object. Should return an `OrderNote?` instead of an `[OrderNote]?`
Add nav bar, display nav buttons
- grouped table style - section titles
Generated by 🚫 Danger |
jleandroperez
left a comment
jleandroperez
left a comment
There was a problem hiding this comment.
Looking great @mindgraffiti !!! few incoming nipticky comments, please, let me know if anything!.
|
|
||
| @objc func addButtonTapped() { | ||
| let indexPath = IndexPath(row: 0, section: 0) | ||
| guard let cell = tableView.cellForRow(at: indexPath) as? WriteCustomerNoteTableViewCell else { |
There was a problem hiding this comment.
Two notes here!!
- Instead of having the IndexPath for the TextView row, you can just do something like this:
WriteCustomerNoteTableViewCell.swift
class WriteCustomerNoteTableViewCell {
var onTextChange: ((String) -> Void)?
}
extension WriteCustomerNoteTableViewCell : UITextViewDelegate: {
func textViewDidChange() {
onTextChange?(textView.text)
}
}
|
|
||
| @objc func addButtonTapped() { | ||
| let indexPath = IndexPath(row: 0, section: 0) | ||
| guard let cell = tableView.cellForRow(at: indexPath) as? WriteCustomerNoteTableViewCell else { |
There was a problem hiding this comment.
- Having the mechanism mentioned above would let you disable the
addbutton, whenever there is no text (OR enable it whenisEmpty != false)
| target: self, | ||
| action: #selector(addButtonTapped)) | ||
| rightBarButton.tintColor = .white | ||
| navigationItem.setRightBarButton(rightBarButton, animated: false) |
There was a problem hiding this comment.
It'd be neat if we could disable the Add button whenever there is no actual text entered.
(More details below in addButtonTapped!!)
| } | ||
|
|
||
| let action = OrderNoteAction.addOrderNote(siteID: viewModel.order.siteID, orderID: viewModel.order.orderID, isCustomerNote: isCustomerNote, note: note) { [weak self] (orderNote, error) in | ||
| guard orderNote != nil else { |
There was a problem hiding this comment.
This one should probably check on error rather than the note. (if let error = error { ... })
| // | ||
| extension AddANoteViewController: UITableViewDataSource { | ||
| func numberOfSections(in tableView: UITableView) -> Int { | ||
| return 2 |
There was a problem hiding this comment.
return sections.count
| } | ||
|
|
||
| func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int { | ||
| return 1 |
There was a problem hiding this comment.
return sections[section].rows.count
| @@ -0,0 +1,25 @@ | |||
| import UIKit | |||
|
|
|||
| class ToggleEmailCustomerTableViewCell: UITableViewCell { | |||
There was a problem hiding this comment.
- Can we rename this one to something like
SwitchTableViewCellperhaps?. - The
topLabel.text+bottomLabel.textsetup should probably be done insetupEmailCustomerCell. That would effectively let you turn this one into a reusable cell (and not just for ToggleEmail)
| } | ||
|
|
||
| override func awakeFromNib() { | ||
| super.awakeFromNib() |
There was a problem hiding this comment.
Please nuke this one, since it's not really needed! 😄
| import UIKit | ||
| import Gridicons | ||
|
|
||
| class WriteCustomerNoteTableViewCell: UITableViewCell { |
There was a problem hiding this comment.
How aboooout... TextViewTableViewCell (or something like that?). Plus:
var iconImage: UIImage? {
set {
noteIconButton.setImage(newValue, for: .normal)
}
get {
return noteIconButton.image (...
}
The goal is: turn this cell into a generic one (and not just for this specific viewController).
There was a problem hiding this comment.
(And you could also move this one, and the second new cell, over to ReusableViews perhaps?)
| } | ||
|
|
||
| self?.upsertStoredOrderNotes(readOnlyOrderNotes: [note], orderID: orderID) | ||
| onCompletion(note, nil) |
There was a problem hiding this comment.
We should proooobably do an optimistic update here. @bummytime thoughts?
There was a problem hiding this comment.
@jleandroperez @mindgraffiti seems to make sense. Similar to the pattern here.
There was a problem hiding this comment.
@jleandroperez @bummytime not sure how to apply optimistic insert here. Do these steps make sense?
- Create an OrderNote object with a fake noteID and upsert it.
- If POSTing the note fails, write a function that deletes the fake note by noteID
- If POSTing is successful, delete the fake note by noteID and upsert the new note from the response
jleandroperez
left a comment
jleandroperez
left a comment
There was a problem hiding this comment.
LOVED this PR!!!. (As discussed over Slack!!):
- First Responder is no longer getting dismissed, whenever the Note Type is switched
- selectionStyle is set to .none in the two Cells
- Updated the firstResponder behavior to automatically toggle upon viewWillAppear / viewWillDisappear.
|
Thank you Jorge! |
Closes #75.
In this PR, an
addOrderNoteaction has been added to Yosemite. The action triggers apostnetwork request. On completion, if successful the new order note is inserted into core data.The UI button
Add a notedisplays a new modal view and is connected to the action. Upon successful completion, the order details view refreshes and displays the new order note.cc: @bummytime @jleandroperez