First draft of Undo Call Response#305
Conversation
Snackbar action color lightened Changed undo text Changed from toast to snackbar
dektar
left a comment
dektar
left a comment
There was a problem hiding this comment.
Looks really good! If you can add tests that'd be awesome, but I don't think this area has any tests yet anyway so it might not be in scope. Some other comments below.
| Outcome.getDisplayString(this, mPendingOutcome.status)); | ||
| String contactName = mIssue.contacts.get(mPendingContactIndex).name; | ||
| String message = getResources().getString( | ||
| R.string.call_reported_undo_format, outcomeLabel, contactName); |
There was a problem hiding this comment.
can we test this with a long rep name -- that it clips or wraps OK?
There was a problem hiding this comment.
I took Susan's advice and shortened the text (removed the rep's name).
Can definitely add it back if you feel strongly. But with it removed it obviates the need for this comment!
| int index = mPendingContactIndex; | ||
| mPendingContactIndex = null; | ||
| mPendingOutcome = null; | ||
| mPendingCallHandler.removeCallbacksAndMessages(null); |
There was a problem hiding this comment.
would we want to removeCallbacksAndMessages even if mPendingContactIndex == null, i.e. do this first in any case?
There was a problem hiding this comment.
yep, good catch.
| Outcome outcome = mPendingOutcome; | ||
| mPendingContactIndex = null; | ||
| mPendingOutcome = null; | ||
| mPendingCallHandler.removeCallbacksAndMessages(null); |
There was a problem hiding this comment.
would we want to removeCallbacksAndMessages even if mPendingContactIndex == null..., i.e. do this first in any case?
| if (s == null || s.isEmpty()) { | ||
| return s; | ||
| } | ||
| return Character.toUpperCase(s.charAt(0)) + s.substring(1); |
There was a problem hiding this comment.
will s.substring(1) work properly if the string is only 1 long?
There was a problem hiding this comment.
this won't throw unless beginIndex is > length. If they're equal it just returns ""
| showContactsUi(); | ||
| Intent intent = new Intent(getApplicationContext(), RepCallActivity.class); | ||
| intent.putExtra(KEY_ISSUE, mIssue); | ||
| intent.putExtra(RepCallActivity.KEY_ADDRESS, mAddress); |
There was a problem hiding this comment.
i don't think we need to pass address to RepCallActivity any more.
| finish(); | ||
| } | ||
|
|
||
| private void returnToIssueWithDemoCalled() { |
There was a problem hiding this comment.
can you check the demo call behavior is as expected (no "undo" and it all plays nicely)?
2 participants
What type of PR is this? (check all applicable)
Description
Related Issues
Were the changes tested?
have not been included