From 549cbbc7841ac804714c569d94af3a1828ba93c7 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 2 Feb 2021 11:49:02 -0300 Subject: [PATCH 1/3] Hide attachment progress indicators if message send has failed. --- .../CVComponents/CVComponentBodyMedia.swift | 3 +- .../CVComponentGenericAttachment.swift | 31 ++++---- .../CV/CVComponents/CVComponentSticker.swift | 17 ++++- .../Cells/CVAttachmentProgressView.swift | 48 +++++++++++++ .../Cells/CVMediaAlbumView.swift | 6 +- .../ConversationView/Cells/CVMediaView.swift | 72 ++++++++----------- 6 files changed, 115 insertions(+), 62 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/CV/CVComponents/CVComponentBodyMedia.swift b/Signal/src/ViewControllers/ConversationView/CV/CVComponents/CVComponentBodyMedia.swift index c9735ed3bf..346b8d0347 100644 --- a/Signal/src/ViewControllers/ConversationView/CV/CVComponents/CVComponentBodyMedia.swift +++ b/Signal/src/ViewControllers/ConversationView/CV/CVComponents/CVComponentBodyMedia.swift @@ -68,9 +68,8 @@ public class CVComponentBodyMedia: CVComponentBase, CVComponent { let albumView = componentView.albumView albumView.configure(mediaCache: self.mediaCache, items: self.items, - isOutgoing: self.isOutgoing, + interaction: self.interaction, isBorderless: self.isBorderless, - isFromLinkedDevice: self.isFromLinkedDevice, cellMeasurement: cellMeasurement, conversationStyle: conversationStyle) diff --git a/Signal/src/ViewControllers/ConversationView/CV/CVComponents/CVComponentGenericAttachment.swift b/Signal/src/ViewControllers/ConversationView/CV/CVComponents/CVComponentGenericAttachment.swift index 689d0393c6..af47f3c5d3 100644 --- a/Signal/src/ViewControllers/ConversationView/CV/CVComponents/CVComponentGenericAttachment.swift +++ b/Signal/src/ViewControllers/ConversationView/CV/CVComponents/CVComponentGenericAttachment.swift @@ -177,26 +177,31 @@ public class CVComponentGenericAttachment: CVComponentBase, CVComponent { private func tryToBuildDownloadView() -> UIView? { - guard let attachmentPointer = self.attachmentPointer else { + let direction: CVAttachmentProgressView.Direction + switch CVAttachmentProgressView.progressType(forAttachment: attachment, + interaction: interaction) { + case .none: return nil - } - - switch attachmentPointer.pointerType { + case .uploading: + // We currently only show progress for downloads here. + return nil + case .pendingDownload(let attachmentPointer): + direction = .download(attachmentPointer: attachmentPointer) + case .downloading(let attachmentPointer): + direction = .download(attachmentPointer: attachmentPointer) case .restoring: - // TODO: Show "restoring" indicator and possibly progress. + // TODO: We could easily show progress for restores. + owsFailDebug("Restoring progress type.") return nil - case .unknown, .incoming: - break - @unknown default: - owsFailDebug("Invalid value.") + case .unknown: + owsFailDebug("Unknown progress type.") return nil } let downloadViewSize = min(iconSize.width, iconSize.height) - let progressView = CVAttachmentProgressView(direction: .download(attachmentPointer: attachmentPointer), - style: .withoutCircle(diameter: downloadViewSize), - conversationStyle: conversationStyle) - return progressView + return CVAttachmentProgressView(direction: direction, + style: .withoutCircle(diameter: downloadViewSize), + conversationStyle: conversationStyle) } private let hSpacing: CGFloat = 8 diff --git a/Signal/src/ViewControllers/ConversationView/CV/CVComponents/CVComponentSticker.swift b/Signal/src/ViewControllers/ConversationView/CV/CVComponents/CVComponentSticker.swift index 12d6ac788c..e2aacd4617 100644 --- a/Signal/src/ViewControllers/ConversationView/CV/CVComponents/CVComponentSticker.swift +++ b/Signal/src/ViewControllers/ConversationView/CV/CVComponents/CVComponentSticker.swift @@ -70,13 +70,28 @@ public class CVComponentSticker: CVComponentBase, CVComponent { containerView.addArrangedSubview(mediaView) componentView.layoutConstraints.append(contentsOf: mediaView.autoSetDimensions(to: .square(stickerSize))) - if isOutgoing, !attachmentStream.isUploaded, !isFromLinkedDevice { + switch CVAttachmentProgressView.progressType(forAttachment: attachmentStream, + interaction: interaction) { + case .none: + break + case .uploading: let progressView = CVAttachmentProgressView(direction: .upload(attachmentStream: attachmentStream), style: .withCircle, conversationStyle: conversationStyle) containerView.addSubview(progressView) progressView.autoAlignAxis(.horizontal, toSameAxisOf: mediaView) progressView.autoAlignAxis(.vertical, toSameAxisOf: mediaView) + case .pendingDownload: + break + case .downloading: + break + case .restoring: + // TODO: We could easily show progress for restores. + owsFailDebug("Restoring progress type.") + break + case .unknown: + owsFailDebug("Unknown progress type.") + break } } else if let attachmentPointer = self.attachmentPointer { let placeholderView = UIView() diff --git a/Signal/src/ViewControllers/ConversationView/Cells/CVAttachmentProgressView.swift b/Signal/src/ViewControllers/ConversationView/Cells/CVAttachmentProgressView.swift index 45a4bc93af..05b5b5e397 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/CVAttachmentProgressView.swift +++ b/Signal/src/ViewControllers/ConversationView/Cells/CVAttachmentProgressView.swift @@ -451,4 +451,52 @@ public class CVAttachmentProgressView: UIView { stateView.state = .uploadUnknownProgress } } + + public enum ProgressType { + case none + case uploading(attachmentStream: TSAttachmentStream) + case pendingDownload(attachmentPointer: TSAttachmentPointer) + case downloading(attachmentPointer: TSAttachmentPointer) + case restoring(attachmentPointer: TSAttachmentPointer) + case unknown + } + + public static func progressType(forAttachment attachment: TSAttachment, + interaction: TSInteraction) -> ProgressType { + + if let attachmentStream = attachment as? TSAttachmentStream { + if let outgoingMessage = interaction as? TSOutgoingMessage { + let hasSendFailed = outgoingMessage.messageState == .failed + let isFromLinkedDevice = outgoingMessage.isFromLinkedDevice + guard !attachmentStream.isUploaded, + !isFromLinkedDevice, + !hasSendFailed else { + return .none + } + return .uploading(attachmentStream: attachmentStream) + } else if interaction is TSIncomingMessage { + return .none + } else { + owsFailDebug("Unexpected interaction: \(type(of: interaction))") + return .unknown + } + } else if let attachmentPointer = attachment as? TSAttachmentPointer { + guard attachmentPointer.pointerType == .incoming else { + return .restoring(attachmentPointer: attachmentPointer) + } + switch attachmentPointer.state { + case .pendingMessageRequest, .pendingManualDownload: + return .pendingDownload(attachmentPointer: attachmentPointer) + case .failed, .enqueued, .downloading: + return .downloading(attachmentPointer: attachmentPointer) + @unknown default: + owsFailDebug("Invalid value.") + return .unknown + } + + } else { + owsFailDebug("Unexpected attachment: \(type(of: attachment))") + return .unknown + } + } } diff --git a/Signal/src/ViewControllers/ConversationView/Cells/CVMediaAlbumView.swift b/Signal/src/ViewControllers/ConversationView/Cells/CVMediaAlbumView.swift index 0b9a127443..919b294ec0 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/CVMediaAlbumView.swift +++ b/Signal/src/ViewControllers/ConversationView/Cells/CVMediaAlbumView.swift @@ -26,9 +26,8 @@ public class CVMediaAlbumView: UIStackView { public func configure(mediaCache: CVMediaCache, items: [CVMediaAlbumItem], - isOutgoing: Bool, + interaction: TSInteraction, isBorderless: Bool, - isFromLinkedDevice: Bool, cellMeasurement: CVCellMeasurement, conversationStyle: ConversationStyle) { @@ -41,10 +40,9 @@ public class CVMediaAlbumView: UIStackView { self.itemViews = CVMediaAlbumView.itemsToDisplay(forItems: items).map { CVMediaView(mediaCache: mediaCache, attachment: $0.attachment, - isOutgoing: isOutgoing, + interaction: interaction, maxMessageWidth: maxMessageWidth, isBorderless: isBorderless, - isFromLinkedDevice: isFromLinkedDevice, conversationStyle: conversationStyle) } self.isBorderless = isBorderless diff --git a/Signal/src/ViewControllers/ConversationView/Cells/CVMediaView.swift b/Signal/src/ViewControllers/ConversationView/Cells/CVMediaView.swift index 193c295201..b916a7c7ab 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/CVMediaView.swift +++ b/Signal/src/ViewControllers/ConversationView/Cells/CVMediaView.swift @@ -16,11 +16,10 @@ public class CVMediaView: UIView { private let mediaCache: CVMediaCache public let attachment: TSAttachment + private let interaction: TSInteraction private let conversationStyle: ConversationStyle - private let isOutgoing: Bool private let maxMessageWidth: CGFloat private let isBorderless: Bool - private let isFromLinkedDevice: Bool private var reusableMediaView: ReusableMediaView? @@ -28,17 +27,15 @@ public class CVMediaView: UIView { public required init(mediaCache: CVMediaCache, attachment: TSAttachment, - isOutgoing: Bool, + interaction: TSInteraction, maxMessageWidth: CGFloat, isBorderless: Bool, - isFromLinkedDevice: Bool, conversationStyle: ConversationStyle) { self.mediaCache = mediaCache self.attachment = attachment - self.isOutgoing = isOutgoing + self.interaction = interaction self.maxMessageWidth = maxMessageWidth self.isBorderless = isBorderless - self.isFromLinkedDevice = isFromLinkedDevice self.conversationStyle = conversationStyle super.init(frame: .zero) @@ -77,49 +74,40 @@ public class CVMediaView: UIView { private func configureForUndownloadedMedia() { tryToConfigureForBlurHash(attachment: attachment) - guard let attachmentPointer = attachment as? TSAttachmentPointer else { - owsFailDebug("Attachment has unexpected type.") - configure(forError: .invalid) - return - } - guard attachmentPointer.pointerType == .incoming else { - // TODO: Show "restoring" indicator and possibly progress. - configure(forError: .missing) - return - } - switch attachmentPointer.state { - case .pendingMessageRequest, .pendingManualDownload: - // We don't need to add a download indicator for pending - // attachments; CVComponentBodyMedia will add a download - // button if any media in the gallery is pending. - return - case .failed, .enqueued, .downloading: - break - @unknown default: - owsFailDebug("Invalid value.") - } - - backgroundColor = (Theme.isDarkThemeEnabled ? .ows_gray90 : .ows_gray05) - let progressView = CVAttachmentProgressView(direction: .download(attachmentPointer: attachmentPointer), - style: .withCircle, - conversationStyle: conversationStyle) - self.addSubview(progressView) - progressView.autoCenterInSuperview() + _ = addProgressIfNecessary() } private func addUploadProgressIfNecessary(_ subview: UIView) -> Bool { - guard let attachmentStream = attachment as? TSAttachmentStream else { + return addProgressIfNecessary() + } + + private func addProgressIfNecessary() -> Bool { + + let direction: CVAttachmentProgressView.Direction + switch CVAttachmentProgressView.progressType(forAttachment: attachment, + interaction: interaction) { + case .none: + return false + case .uploading(let attachmentStream): + direction = .upload(attachmentStream: attachmentStream) + case .pendingDownload(let attachmentPointer): + direction = .download(attachmentPointer: attachmentPointer) + case .downloading(let attachmentPointer): + direction = .download(attachmentPointer: attachmentPointer) + case .restoring: + // TODO: We could easily show progress for restores. + owsFailDebug("Restoring progress type.") + return false + case .unknown: + owsFailDebug("Unknown progress type.") return false } - guard isOutgoing, - !attachmentStream.isUploaded, - !isFromLinkedDevice else { - return false - } - let progressView = CVAttachmentProgressView(direction: .upload(attachmentStream: attachmentStream), + + backgroundColor = (Theme.isDarkThemeEnabled ? .ows_gray90 : .ows_gray05) + let progressView = CVAttachmentProgressView(direction: direction, style: .withCircle, conversationStyle: conversationStyle) - self.addSubview(progressView) + addSubview(progressView) progressView.autoCenterInSuperview() return true } From 55f339e2093078b9b4bf1a29ee34e18dd56d9131 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 2 Feb 2021 11:52:57 -0300 Subject: [PATCH 2/3] Hide attachment progress indicators if message send has failed. --- .../ConversationView/Cells/CVMediaView.swift | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/Cells/CVMediaView.swift b/Signal/src/ViewControllers/ConversationView/Cells/CVMediaView.swift index b916a7c7ab..8ae4386c53 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/CVMediaView.swift +++ b/Signal/src/ViewControllers/ConversationView/Cells/CVMediaView.swift @@ -90,8 +90,11 @@ public class CVMediaView: UIView { return false case .uploading(let attachmentStream): direction = .upload(attachmentStream: attachmentStream) - case .pendingDownload(let attachmentPointer): - direction = .download(attachmentPointer: attachmentPointer) + case .pendingDownload: + // We don't need to add a download indicator for pending + // attachments; CVComponentBodyMedia will add a download + // button if any media in the gallery is pending. + return false case .downloading(let attachmentPointer): direction = .download(attachmentPointer: attachmentPointer) case .restoring: From 8366f019483488a2235de9ef4216e697470947eb Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 3 Feb 2021 09:54:49 -0300 Subject: [PATCH 3/3] Respond to CR. --- .../ConversationView/Cells/CVMediaView.swift | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/Cells/CVMediaView.swift b/Signal/src/ViewControllers/ConversationView/Cells/CVMediaView.swift index 8ae4386c53..73927c8388 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/CVMediaView.swift +++ b/Signal/src/ViewControllers/ConversationView/Cells/CVMediaView.swift @@ -77,10 +77,6 @@ public class CVMediaView: UIView { _ = addProgressIfNecessary() } - private func addUploadProgressIfNecessary(_ subview: UIView) -> Bool { - return addProgressIfNecessary() - } - private func addProgressIfNecessary() -> Bool { let direction: CVAttachmentProgressView.Direction @@ -96,6 +92,8 @@ public class CVMediaView: UIView { // button if any media in the gallery is pending. return false case .downloading(let attachmentPointer): + backgroundColor = (Theme.isDarkThemeEnabled ? .ows_gray90 : .ows_gray05) + direction = .download(attachmentPointer: attachmentPointer) case .restoring: // TODO: We could easily show progress for restores. @@ -106,7 +104,6 @@ public class CVMediaView: UIView { return false } - backgroundColor = (Theme.isDarkThemeEnabled ? .ows_gray90 : .ows_gray05) let progressView = CVAttachmentProgressView(direction: direction, style: .withCircle, conversationStyle: conversationStyle) @@ -136,7 +133,7 @@ public class CVMediaView: UIView { } mediaView.backgroundColor = isBorderless ? .clear : Theme.washColor - if !addUploadProgressIfNecessary(mediaView) { + if !addProgressIfNecessary() { if reusableMediaView.isVideo { let videoPlayButton = Self.buildVideoPlayButton() addSubview(videoPlayButton)