From 42762ad9070dbfe6b8037ec04e6502b7593d8d9a Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 11 Jan 2019 15:23:06 -0500 Subject: [PATCH 1/3] Allow taps in albums with failed images. --- .../ColorPickerViewController.swift | 7 +------ .../Cells/OWSMessageBubbleView.m | 14 +++++++++----- .../ConversationView/ConversationViewItem.h | 4 +--- .../ConversationView/ConversationViewItem.m | 18 +----------------- 4 files changed, 12 insertions(+), 31 deletions(-) diff --git a/Signal/src/ViewControllers/ColorPickerViewController.swift b/Signal/src/ViewControllers/ColorPickerViewController.swift index 429a80cc7..cec956a9f 100644 --- a/Signal/src/ViewControllers/ColorPickerViewController.swift +++ b/Signal/src/ViewControllers/ColorPickerViewController.swift @@ -1,5 +1,5 @@ // -// Copyright (c) 2018 Open Whisper Systems. All rights reserved. +// Copyright (c) 2019 Open Whisper Systems. All rights reserved. // import Foundation @@ -434,11 +434,6 @@ private class MockConversationViewItem: NSObject, ConversationViewItem { owsFailDebug("unexpected invocation") return nil } - - func mediaAlbumHasFailedAttachment() -> Bool { - owsFailDebug("unexpected invocation") - return false - } } private class MockIncomingMessage: TSIncomingMessage { diff --git a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m index 73adb67f4..d9a40f81b 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m +++ b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m @@ -1,5 +1,5 @@ // -// Copyright (c) 2018 Open Whisper Systems. All rights reserved. +// Copyright (c) 2019 Open Whisper Systems. All rights reserved. // #import "OWSMessageBubbleView.h" @@ -1376,10 +1376,6 @@ const UIDataDetectorTypes kOWSAllowedDataDetectorTypes OWSAssertDebug(self.bodyMediaView); OWSAssertDebug(self.viewItem.mediaAlbumItems.count > 0); - if (self.viewItem.mediaAlbumHasFailedAttachment) { - [self.delegate didTapFailedIncomingAttachment:self.viewItem]; - return; - } if (![self.bodyMediaView isKindOfClass:[OWSMediaAlbumCellView class]]) { OWSFailDebug(@"Unexpected body media view: %@", self.bodyMediaView.class); return; @@ -1394,6 +1390,14 @@ const UIDataDetectorTypes kOWSAllowedDataDetectorTypes TSAttachment *attachment = mediaView.attachment; if (![attachment isKindOfClass:[TSAttachmentStream class]]) { + if ([attachment isKindOfClass:[TSAttachmentPointer class]]) { + TSAttachmentPointer *attachmentPointer = (TSAttachmentPointer *)attachment; + if (attachmentPointer.state == TSAttachmentPointerStateFailed) { + [self.delegate didTapFailedIncomingAttachment:self.viewItem]; + return; + } + } + OWSLogWarn(@"Media attachment not yet downloaded."); return; } diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewItem.h b/Signal/src/ViewControllers/ConversationView/ConversationViewItem.h index d106c8249..8bccb0604 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewItem.h +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewItem.h @@ -1,5 +1,5 @@ // -// Copyright (c) 2018 Open Whisper Systems. All rights reserved. +// Copyright (c) 2019 Open Whisper Systems. All rights reserved. // #import "ConversationViewLayout.h" @@ -144,8 +144,6 @@ NSString *NSStringForOWSMessageCellType(OWSMessageCellType cellType); - (nullable TSAttachmentStream *)firstValidAlbumAttachment; -- (BOOL)mediaAlbumHasFailedAttachment; - @end #pragma mark - diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m b/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m index d0783da92..11d520eb9 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m @@ -1,5 +1,5 @@ // -// Copyright (c) 2018 Open Whisper Systems. All rights reserved. +// Copyright (c) 2019 Open Whisper Systems. All rights reserved. // #import "ConversationViewItem.h" @@ -1074,22 +1074,6 @@ NSString *NSStringForOWSMessageCellType(OWSMessageCellType cellType) } } -- (BOOL)mediaAlbumHasFailedAttachment -{ - OWSAssertDebug(self.messageCellType == OWSMessageCellType_MediaAlbum); - OWSAssertDebug(self.mediaAlbumItems.count > 0); - - for (ConversationMediaAlbumItem *mediaAlbumItem in self.mediaAlbumItems) { - if ([mediaAlbumItem.attachment isKindOfClass:[TSAttachmentPointer class]]) { - TSAttachmentPointer *attachmentPointer = (TSAttachmentPointer *)mediaAlbumItem.attachment; - if (attachmentPointer.state == TSAttachmentPointerStateFailed) { - return YES; - } - } - } - return NO; -} - @end NS_ASSUME_NONNULL_END From 04a300784f9ef73ead6a0c41e652fcf005e75fe5 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 14 Jan 2019 16:17:59 -0500 Subject: [PATCH 2/3] Respond to CR. --- .../Cells/MediaAlbumCellView.swift | 16 +++++++++----- .../Cells/OWSMessageBubbleView.m | 22 ++++++++++++++----- .../ConversationView/ConversationViewItem.h | 2 ++ .../ConversationView/ConversationViewItem.m | 10 +++++++++ 4 files changed, 40 insertions(+), 10 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/Cells/MediaAlbumCellView.swift b/Signal/src/ViewControllers/ConversationView/Cells/MediaAlbumCellView.swift index 9319a2938..146dd9075 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/MediaAlbumCellView.swift +++ b/Signal/src/ViewControllers/ConversationView/Cells/MediaAlbumCellView.swift @@ -1,5 +1,5 @@ // -// Copyright (c) 2018 Open Whisper Systems. All rights reserved. +// Copyright (c) 2019 Open Whisper Systems. All rights reserved. // import Foundation @@ -11,6 +11,9 @@ public class MediaAlbumCellView: UIStackView { @objc public let itemViews: [ConversationMediaView] + @objc + public var moreItemsView: ConversationMediaView? + private static let kSpacingPts: CGFloat = 2 private static let kMaxItems = 5 @@ -41,8 +44,6 @@ public class MediaAlbumCellView: UIStackView { } private func createContents(maxMessageWidth: CGFloat) { - var moreItemViews = [ConversationMediaView]() - switch itemViews.count { case 0: owsFailDebug("No item views.") @@ -129,7 +130,7 @@ public class MediaAlbumCellView: UIStackView { return } - moreItemViews.append(lastView) + moreItemsView = lastView let tintView = UIView() tintView.backgroundColor = UIColor(white: 0, alpha: 0.4) @@ -151,7 +152,7 @@ public class MediaAlbumCellView: UIStackView { } for itemView in itemViews { - guard !moreItemViews.contains(itemView) else { + guard moreItemsView == itemView else { // Don't display the caption indicator on // the "more" item, if any. continue @@ -277,4 +278,9 @@ public class MediaAlbumCellView: UIStackView { } return bestMediaView } + + @objc + public func isMoreItemsView(mediaView: ConversationMediaView) -> Bool { + return moreItemsView == mediaView + } } diff --git a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m index d9a40f81b..72291134b 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m +++ b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m @@ -1388,16 +1388,28 @@ const UIDataDetectorTypes kOWSAllowedDataDetectorTypes return; } - TSAttachment *attachment = mediaView.attachment; - if (![attachment isKindOfClass:[TSAttachmentStream class]]) { - if ([attachment isKindOfClass:[TSAttachmentPointer class]]) { - TSAttachmentPointer *attachmentPointer = (TSAttachmentPointer *)attachment; - if (attachmentPointer.state == TSAttachmentPointerStateFailed) { + if ([mediaAlbumCellView isMoreItemsViewWithMediaView:mediaView]) { + for (ConversationMediaAlbumItem *mediaAlbumItem in self.viewItem.mediaAlbumItems) { + if (mediaAlbumItem.isFailedDownload) { + // Treat the tap as a "retry" tap if: + // + // a) the user tapped on the "more items" cell. + // b) there are any failed downloads in the album. [self.delegate didTapFailedIncomingAttachment:self.viewItem]; return; } } + } + TSAttachment *attachment = mediaView.attachment; + if ([attachment isKindOfClass:[TSAttachmentPointer class]]) { + TSAttachmentPointer *attachmentPointer = (TSAttachmentPointer *)attachment; + if (attachmentPointer.state == TSAttachmentPointerStateFailed) { + // Treat the tap as a "retry" tap if the user taps on a failed download. + [self.delegate didTapFailedIncomingAttachment:self.viewItem]; + return; + } + } else if (![attachment isKindOfClass:[TSAttachmentStream class]]) { OWSLogWarn(@"Media attachment not yet downloaded."); return; } diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewItem.h b/Signal/src/ViewControllers/ConversationView/ConversationViewItem.h index 8bccb0604..9d88e7e37 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewItem.h +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewItem.h @@ -47,6 +47,8 @@ NSString *NSStringForOWSMessageCellType(OWSMessageCellType cellType); @property (nonatomic, readonly, nullable) NSString *caption; +@property (nonatomic, readonly) BOOL isFailedDownload; + @end #pragma mark - diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m b/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m index 11d520eb9..7733b873f 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m @@ -64,6 +64,16 @@ NSString *NSStringForOWSMessageCellType(OWSMessageCellType cellType) return self; } + +- (BOOL)isFailedDownload +{ + if (![self.attachment isKindOfClass:[TSAttachmentPointer class]]) { + return NO; + } + TSAttachmentPointer *attachmentPointer = (TSAttachmentPointer *)self.attachment; + return attachmentPointer.state == TSAttachmentPointerStateFailed; +} + @end #pragma mark - From e91195385be0720e67d231f8e75132f56ed49b77 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 15 Jan 2019 16:39:01 -0500 Subject: [PATCH 3/3] Respond to CR. --- .../ColorPickerViewController.swift | 5 +++++ .../Cells/MediaAlbumCellView.swift | 2 +- .../ConversationView/Cells/OWSMessageBubbleView.m | 15 ++++----------- .../ConversationView/ConversationViewItem.h | 2 ++ .../ConversationView/ConversationViewItem.m | 13 +++++++++++++ 5 files changed, 25 insertions(+), 12 deletions(-) diff --git a/Signal/src/ViewControllers/ColorPickerViewController.swift b/Signal/src/ViewControllers/ColorPickerViewController.swift index cec956a9f..a1bcac6c4 100644 --- a/Signal/src/ViewControllers/ColorPickerViewController.swift +++ b/Signal/src/ViewControllers/ColorPickerViewController.swift @@ -434,6 +434,11 @@ private class MockConversationViewItem: NSObject, ConversationViewItem { owsFailDebug("unexpected invocation") return nil } + + func mediaAlbumHasFailedAttachment() -> Bool { + owsFailDebug("unexpected invocation") + return false + } } private class MockIncomingMessage: TSIncomingMessage { diff --git a/Signal/src/ViewControllers/ConversationView/Cells/MediaAlbumCellView.swift b/Signal/src/ViewControllers/ConversationView/Cells/MediaAlbumCellView.swift index 146dd9075..5e4beb8d0 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/MediaAlbumCellView.swift +++ b/Signal/src/ViewControllers/ConversationView/Cells/MediaAlbumCellView.swift @@ -152,7 +152,7 @@ public class MediaAlbumCellView: UIStackView { } for itemView in itemViews { - guard moreItemsView == itemView else { + guard moreItemsView != itemView else { // Don't display the caption indicator on // the "more" item, if any. continue diff --git a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m index 72291134b..2cb3d6a67 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m +++ b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m @@ -1388,17 +1388,10 @@ const UIDataDetectorTypes kOWSAllowedDataDetectorTypes return; } - if ([mediaAlbumCellView isMoreItemsViewWithMediaView:mediaView]) { - for (ConversationMediaAlbumItem *mediaAlbumItem in self.viewItem.mediaAlbumItems) { - if (mediaAlbumItem.isFailedDownload) { - // Treat the tap as a "retry" tap if: - // - // a) the user tapped on the "more items" cell. - // b) there are any failed downloads in the album. - [self.delegate didTapFailedIncomingAttachment:self.viewItem]; - return; - } - } + if ([mediaAlbumCellView isMoreItemsViewWithMediaView:mediaView] + && self.viewItem.mediaAlbumHasFailedAttachment) { + [self.delegate didTapFailedIncomingAttachment:self.viewItem]; + return; } TSAttachment *attachment = mediaView.attachment; diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewItem.h b/Signal/src/ViewControllers/ConversationView/ConversationViewItem.h index 9d88e7e37..487d5a7f5 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewItem.h +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewItem.h @@ -146,6 +146,8 @@ NSString *NSStringForOWSMessageCellType(OWSMessageCellType cellType); - (nullable TSAttachmentStream *)firstValidAlbumAttachment; +- (BOOL)mediaAlbumHasFailedAttachment; + @end #pragma mark - diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m b/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m index 7733b873f..60b94a184 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m @@ -1084,6 +1084,19 @@ NSString *NSStringForOWSMessageCellType(OWSMessageCellType cellType) } } +- (BOOL)mediaAlbumHasFailedAttachment +{ + OWSAssertDebug(self.messageCellType == OWSMessageCellType_MediaAlbum); + OWSAssertDebug(self.mediaAlbumItems.count > 0); + + for (ConversationMediaAlbumItem *mediaAlbumItem in self.mediaAlbumItems) { + if (mediaAlbumItem.isFailedDownload) { + return YES; + } + } + return NO; +} + @end NS_ASSUME_NONNULL_END