From 19e010645ee6f4be7df8ae04171484e62b7d6ded Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 26 Sep 2017 09:32:48 -0400 Subject: [PATCH] Respond to CR. // FREEBIE --- .../AttachmentApprovalViewController.swift | 6 +- .../MessageMetadataViewController.swift | 61 +++++++++++-------- .../src/Messages/OWSReadReceiptManager.m | 4 ++ .../src/Util/NSDate+millisecondTimeStamp.h | 15 ----- 4 files changed, 44 insertions(+), 42 deletions(-) delete mode 100644 SignalServiceKit/src/Util/NSDate+millisecondTimeStamp.h diff --git a/Signal/src/ViewControllers/AttachmentApprovalViewController.swift b/Signal/src/ViewControllers/AttachmentApprovalViewController.swift index a306b21ef..06bdf2ec9 100644 --- a/Signal/src/ViewControllers/AttachmentApprovalViewController.swift +++ b/Signal/src/ViewControllers/AttachmentApprovalViewController.swift @@ -22,7 +22,7 @@ class AttachmentApprovalViewController: OWSViewController { @available(*, unavailable, message:"use attachment: constructor instead.") required init?(coder aDecoder: NSCoder) { self.attachment = SignalAttachment.empty() - mediaMessageView = MediaMessageView(attachment:attachment) + self.mediaMessageView = MediaMessageView(attachment:attachment) super.init(coder: aDecoder) owsFail("\(self.TAG) invalid constructor") } @@ -31,9 +31,9 @@ class AttachmentApprovalViewController: OWSViewController { assert(!attachment.hasError) self.attachment = attachment self.successCompletion = successCompletion - mediaMessageView = MediaMessageView(attachment:attachment) + self.mediaMessageView = MediaMessageView(attachment:attachment) super.init(nibName: nil, bundle: nil) -} + } // MARK: View Lifecycle diff --git a/Signal/src/ViewControllers/MessageMetadataViewController.swift b/Signal/src/ViewControllers/MessageMetadataViewController.swift index f6ae9bc86..cd1c4844d 100644 --- a/Signal/src/ViewControllers/MessageMetadataViewController.swift +++ b/Signal/src/ViewControllers/MessageMetadataViewController.swift @@ -21,6 +21,14 @@ class MessageMetadataViewController: OWSViewController { var attachmentStream: TSAttachmentStream? var messageBody: String? + static let dateFormatter: DateFormatter = CreateDateFormatter() + private class func CreateDateFormatter() -> DateFormatter { + let dateFormatter = DateFormatter() + dateFormatter.dateStyle = .short + dateFormatter.timeStyle = .long + return dateFormatter + } + // MARK: Initializers @available(*, unavailable, message:"use message: constructor instead.") @@ -69,19 +77,6 @@ class MessageMetadataViewController: OWSViewController { scrollView.autoPinWidthToSuperview(withMargin:0) scrollView.autoPin(toTopLayoutGuideOf: self, withInset:0) - let footer = UIToolbar() - footer.barTintColor = UIColor.ows_materialBlue() - view.addSubview(footer) - footer.autoPinWidthToSuperview(withMargin:0) - footer.autoPinEdge(.top, to:.bottom, of:scrollView) - footer.autoPin(toBottomLayoutGuideOf: self, withInset:0) - - footer.items = [ - UIBarButtonItem(barButtonSystemItem: .flexibleSpace, target: nil, action: nil), - UIBarButtonItem(barButtonSystemItem: .action, target: self, action: #selector(shareButtonPressed)), - UIBarButtonItem(barButtonSystemItem: .flexibleSpace, target: nil, action: nil) - ] - // See notes on how to use UIScrollView with iOS Auto Layout: // // https://developer.apple.com/library/content/releasenotes/General/RN-iOSSDK-6_0/ @@ -132,20 +127,16 @@ class MessageMetadataViewController: OWSViewController { } } - let dateFormatter = DateFormatter() - dateFormatter.dateStyle = .short - dateFormatter.timeStyle = .long - let sentDate = NSDate.ows_date(withMillisecondsSince1970:message.timestamp) rows.append(valueRow(name: NSLocalizedString("MESSAGE_METADATA_VIEW_SENT_DATE_TIME", comment: "Label for the 'sent date & time' field of the 'message metadata' view."), - value:dateFormatter.string(from:sentDate))) + value:MessageMetadataViewController.dateFormatter.string(from:sentDate))) if let _ = message as? TSIncomingMessage { let receivedDate = message.dateForSorting() rows.append(valueRow(name: NSLocalizedString("MESSAGE_METADATA_VIEW_RECEIVED_DATE_TIME", comment: "Label for the 'received date & time' field of the 'message metadata' view."), - value:dateFormatter.string(from:receivedDate))) + value:MessageMetadataViewController.dateFormatter.string(from:receivedDate))) } // TODO: We could include the "disappearing messages" state here. @@ -171,6 +162,7 @@ class MessageMetadataViewController: OWSViewController { rows.append(bodyLabel) } else { // Neither attachment nor body. + owsFail("\(self.TAG) Message has neither attachment nor body.") rows.append(valueRow(name: NSLocalizedString("MESSAGE_METADATA_VIEW_NO_ATTACHMENT_OR_BODY", comment: "Label for messages without a body or attachment in the 'message metadata' view."), value:"")) @@ -199,6 +191,25 @@ class MessageMetadataViewController: OWSViewController { mediaMessageView.autoPinToSquareAspectRatio() } + let hasAttachment = message.attachmentIds.count > 0 + + if hasAttachment { + let footer = UIToolbar() + footer.barTintColor = UIColor.ows_materialBlue() + view.addSubview(footer) + footer.autoPinWidthToSuperview(withMargin:0) + footer.autoPinEdge(.top, to:.bottom, of:scrollView) + footer.autoPin(toBottomLayoutGuideOf: self, withInset:0) + + footer.items = [ + UIBarButtonItem(barButtonSystemItem: .flexibleSpace, target: nil, action: nil), + UIBarButtonItem(barButtonSystemItem: .action, target: self, action: #selector(shareButtonPressed)), + UIBarButtonItem(barButtonSystemItem: .flexibleSpace, target: nil, action: nil) + ] + } else { + scrollView.autoPin(toBottomLayoutGuideOf: self, withInset:0) + } + // TODO: We might want to add a footer with share/save/copy/etc. } @@ -269,9 +280,9 @@ class MessageMetadataViewController: OWSViewController { } private func recipientStatus(forOutgoingMessage message: TSOutgoingMessage, recipientId: String) -> String { - let dateFormatter = DateFormatter() - dateFormatter.dateStyle = .short - dateFormatter.timeStyle = .long + // Legacy messages don't have "recipient read" state or "per-recipient delivery" state, + // so we fall back to `TSOutgoingMessageState` which is not per-recipient and therefore + // might be misleading. let recipientReadMap = message.recipientReadMap if let readTimestamp = recipientReadMap[recipientId] { @@ -279,11 +290,11 @@ class MessageMetadataViewController: OWSViewController { let readDate = NSDate.ows_date(withMillisecondsSince1970:readTimestamp.uint64Value) return String(format:NSLocalizedString("MESSAGE_STATUS_READ_WITH_TIMESTAMP_FORMAT", comment: "message status for messages read by the recipient. Embeds: {{the date and time the message was read}}."), - dateFormatter.string(from:readDate)) + MessageMetadataViewController.dateFormatter.string(from:readDate)) } // TODO: We don't currently track delivery state on a per-recipient basis. - // We should. + // We should. NOTE: This work is in PR. if message.wasDelivered { return NSLocalizedString("MESSAGE_STATUS_DELIVERED", comment:"message status for message delivered to their recipient.") @@ -297,6 +308,8 @@ class MessageMetadataViewController: OWSViewController { NSLocalizedString("MESSAGE_STATUS_SENT", comment:"message footer for sent messages") } else if message.hasAttachments() { + assert(message.messageState == .attemptingOut) + return NSLocalizedString("MESSAGE_STATUS_UPLOADING", comment:"message footer while attachment is uploading") } else { diff --git a/SignalServiceKit/src/Messages/OWSReadReceiptManager.m b/SignalServiceKit/src/Messages/OWSReadReceiptManager.m index 465014737..d5ecca6a3 100644 --- a/SignalServiceKit/src/Messages/OWSReadReceiptManager.m +++ b/SignalServiceKit/src/Messages/OWSReadReceiptManager.m @@ -55,6 +55,9 @@ NS_ASSUME_NONNULL_BEGIN - (void)addRecipientId:(NSString *)recipientId timestamp:(uint64_t)timestamp { + OWSAssert(recipientId.length > 0); + OWSAssert(timestamp > 0); + NSMutableDictionary *recipientMapCopy = [self.recipientMap mutableCopy]; recipientMapCopy[recipientId] = @(timestamp); _recipientMap = [recipientMapCopy copy]; @@ -434,6 +437,7 @@ NSString *const OWSReadReceiptManagerAreReadReceiptsEnabled = @"areReadReceiptsE OWSAssert(recipientMap.count > 0); for (NSString *recipientId in recipientMap) { NSNumber *nsReadTimestamp = recipientMap[recipientId]; + OWSAssert(nsReadTimestamp); uint64_t readTimestamp = [nsReadTimestamp unsignedLongLongValue]; [message updateWithReadRecipientId:recipientId readTimestamp:readTimestamp transaction:transaction]; diff --git a/SignalServiceKit/src/Util/NSDate+millisecondTimeStamp.h b/SignalServiceKit/src/Util/NSDate+millisecondTimeStamp.h deleted file mode 100644 index b1dea2392..000000000 --- a/SignalServiceKit/src/Util/NSDate+millisecondTimeStamp.h +++ /dev/null @@ -1,15 +0,0 @@ -// -// Copyright (c) 2017 Open Whisper Systems. All rights reserved. -// - -NS_ASSUME_NONNULL_BEGIN - -@interface NSDate (millisecondTimeStamp) - -+ (uint64_t)ows_millisecondTimeStamp; -+ (NSDate *)ows_dateWithMillisecondsSince1970:(uint64_t)milliseconds; -+ (uint64_t)ows_millisecondsSince1970ForDate:(NSDate *)date; - -@end - -NS_ASSUME_NONNULL_END