Fix delete from message details

// FREEBIE

Signal/src/ViewControllers/ConversationView/Cells/OWSMessageCell.m

@@ -999,7 +999,6 @@ const CGFloat OWSMessageCellCornerRadius = 17;
     self.customView.userInteractionEnabled = NO;
     [self.payloadView addSubview:self.customView];
     [self.contentConstraints addObjectsFromArray:[self.customView autoPinToSuperviewEdges]];
-    [self cropMediaViewToBubbbleShape:self.customView];
 }
 
 - (CGSize)textBubbleSizeForContentWidth:(int)contentWidth

Signal/src/ViewControllers/DebugUI/DebugUIMessages.m

@@ -123,6 +123,10 @@ NS_ASSUME_NONNULL_BEGIN
                         actionBlock:^{
                             [DebugUIMessages createFakeThreads:1000 withFakeMessages:1];
                         }],
+        [OWSTableItem itemWithTitle:@"🖼 fake media messages: 5"
+                        actionBlock:^{
+                            [DebugUIMessages sendFakeMediaMessages:5 thread:thread];
+                        }],
         [OWSTableItem itemWithTitle:@"🖼 fake media messages: 100"
                         actionBlock:^{
                             [DebugUIMessages sendFakeMediaMessages:100 thread:thread];

Signal/src/ViewControllers/MediaDetailViewController.m

@@ -62,6 +62,7 @@ NS_ASSUME_NONNULL_BEGIN
 
 @property (nonatomic) TSAttachmentStream *attachmentStream;
 @property (nonatomic, nullable) ConversationViewItem *viewItem;
+@property (nonatomic, readonly) UIImage *image;
 
 @property (nonatomic, nullable) OWSVideoPlayer *videoPlayer;
 @property (nonatomic, nullable) UIButton *playVideoButton;
@@ -94,6 +95,8 @@ NS_ASSUME_NONNULL_BEGIN
 
     _galleryItemBox = galleryItemBox;
     _viewItem = viewItem;
+    // We cache the image data in case the attachment stream is deleted.
+    _image = galleryItemBox.attachmentStream.image;
 
     return self;
 }
@@ -119,11 +122,6 @@ NS_ASSUME_NONNULL_BEGIN
     return _fileData;
 }
 
-- (UIImage *)image
-{
-    return self.attachmentStream.image;
-}
-
 - (BOOL)isAnimated
 {
     return self.attachmentStream.isAnimated;
@@ -407,35 +405,7 @@ NS_ASSUME_NONNULL_BEGIN
         return;
     }
 
-    UIAlertController *actionSheet =
+    [self.delegate mediaDetailViewController:self requestDeleteConversationViewItem:self.viewItem];
-        [UIAlertController alertControllerWithTitle:nil message:nil preferredStyle:UIAlertControllerStyleActionSheet];
-
-    [actionSheet
-        addAction:[UIAlertAction
-                      actionWithTitle:NSLocalizedString(@"TXT_DELETE_TITLE", nil)
-                                style:UIAlertActionStyleDestructive
-                              handler:^(UIAlertAction *action) {
-                                  [self.delegate mediaDetailViewController:self
-                                         requestDeleteConversationViewItem:self.viewItem];
-
-                                  // TODO maybe this would be more straight forward if the MessageDetailVC was the
-                                  // deleteDelegate
-                                  if ([self.presentingViewController isKindOfClass:[UINavigationController class]]) {
-                                      UINavigationController *navController
-                                          = (UINavigationController *)self.presentingViewController;
-                                      if ([navController.topViewController
-                                              isKindOfClass:[MessageDetailViewController class]]) {
-                                          MessageDetailViewController *messageDetailViewController
-                                              = (MessageDetailViewController *)navController.topViewController;
-                                          messageDetailViewController.wasDeleted = YES;
-                                          [navController popViewControllerAnimated:YES];
-                                      }
-                                  }
-                              }]];
-
-    [actionSheet addAction:[OWSAlerts cancelAction]];
-
-    [self presentViewController:actionSheet animated:YES completion:nil];
 }
 
 - (BOOL)canPerformAction:(SEL)action withSender:(nullable id)sender

Signal/src/ViewControllers/MediaGalleryViewController.swift

@@ -179,6 +179,11 @@ protocol MediaGalleryDataSource: class {
     func delete(message: TSMessage)
 }
 
+protocol MediaGalleryDataSourceDelegate: class {
+    func mediaGalleryDataSource(_ mediaGalleryDataSource: MediaGalleryDataSource, willDelete message: TSMessage)
+    func mediaGalleryDataSource(_ mediaGalleryDataSource: MediaGalleryDataSource, deletedSections: IndexSet, deletedItems: [IndexPath])
+}
+
 class MediaGalleryViewController: UINavigationController, MediaGalleryDataSource, MediaTileViewControllerDelegate {
 
     private var pageViewController: MediaPageViewController?
@@ -555,6 +560,12 @@ class MediaGalleryViewController: UINavigationController, MediaGalleryDataSource
     lazy var mediaTileViewController: MediaTileViewController = {
         let vc = MediaTileViewController(mediaGalleryDataSource: self, uiDatabaseConnection: self.uiDatabaseConnection)
         vc.delegate = self
+
+        // dataSourceDelegate will either be this tile view, or the MessageDetailView, but they should
+        // be mutually exclusive
+        assert(self.dataSourceDelegate == nil)
+        self.dataSourceDelegate = vc
+
         return vc
     }()
 
@@ -712,12 +723,12 @@ class MediaGalleryViewController: UINavigationController, MediaGalleryDataSource
         }
     }
 
+    weak var dataSourceDelegate: MediaGalleryDataSourceDelegate?
     var deletedMessages: Set<TSMessage> = Set()
     func delete(message: TSMessage) {
         Logger.info("\(logTag) in \(#function) with message: \(String(describing: message.uniqueId)) attachmentId: \(String(describing: message.attachmentIds.firstObject))")
 
-        // TODO put this somewhere reasonable...
+        self.dataSourceDelegate?.mediaGalleryDataSource(self, willDelete: message)
-        self.mediaTileViewController.collectionView!.layoutIfNeeded()
 
         self.editingDatabaseConnection.asyncReadWrite { transaction in
             message.remove(with: transaction)
@@ -765,10 +776,7 @@ class MediaGalleryViewController: UINavigationController, MediaGalleryDataSource
             deletedIndexPaths.append(IndexPath(row: sectionRowIndex, section: sectionIndex + 1))
         }
 
-        // TODO? notify pager view
+        self.dataSourceDelegate?.mediaGalleryDataSource(self, deletedSections: deletedSections, deletedItems: deletedIndexPaths)
-
-        // notify tile view    
-        self.mediaTileViewController.updatedDataSource(deletedSections: deletedSections, deletedItems: deletedIndexPaths)
     }
 
     let kGallerySwipeLoadBatchSize: UInt = 5
@@ -806,6 +814,6 @@ class MediaGalleryViewController: UINavigationController, MediaGalleryDataSource
         self.uiDatabaseConnection.read { (transaction: YapDatabaseReadTransaction) in
             count = self.mediaGalleryFinder.mediaCount(transaction: transaction)
         }
-        return Int(count)
+        return Int(count) - deletedMessages.count
     }
 }

Signal/src/ViewControllers/MediaPageViewController.swift

@@ -47,14 +47,18 @@ class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSou
             return currentViewController.galleryItemBox.value
         }
         set {
-            guard let galleryPage = self.buildGalleryPage(galleryItem: newValue) else {
+            setCurrentItem(newValue, direction: .forward, animated: false)
-                owsFail("unexpetedly unable to build new gallery page")
+        }
-                return
+    }
-            }
 
-            self.updateTitle(item: newValue)
+    private func setCurrentItem(_ item: MediaGalleryItem, direction: UIPageViewControllerNavigationDirection, animated isAnimated: Bool) {
-            self.setViewControllers([galleryPage], direction: .forward, animated: false, completion: nil)
+        guard let galleryPage = self.buildGalleryPage(galleryItem: item) else {
+            owsFail("unexpetedly unable to build new gallery page")
+            return
         }
+
+        self.updateTitle(item: item)
+        self.setViewControllers([galleryPage], direction: direction, animated: isAnimated)
     }
 
     private let uiDatabaseConnection: YapDatabaseConnection
@@ -302,7 +306,33 @@ class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSou
             owsFail("\(logTag) in \(#function) currentViewController was unexpectedly nil")
             return
         }
-        currentViewController.didPressDelete(sender)
+
+        guard let mediaGalleryDataSource = self.mediaGalleryDataSource else {
+            owsFail("\(logTag) in \(#function) mediaGalleryDataSource was unexpectedly nil")
+            return
+        }
+
+        let actionSheet = UIAlertController(title: nil, message: nil, preferredStyle: .actionSheet)
+        let deleteAction = UIAlertAction(title: NSLocalizedString("TXT_DELETE_TITLE", comment: ""),
+                                         style: .destructive) { _ in
+                                            let deletedItem = currentViewController.galleryItem
+
+                                            if !self.sliderEnabled {
+                                                // In message details, which doesn't use the slider, so don't swap pages.
+                                            } else if let nextItem = mediaGalleryDataSource.galleryItem(after: deletedItem) {
+                                                self.setCurrentItem(nextItem, direction: .forward, animated: true)
+                                            } else if let previousItem = mediaGalleryDataSource.galleryItem(before: deletedItem) {
+                                                self.setCurrentItem(previousItem, direction: .reverse, animated: true)
+                                            } else {
+                                                // else we deleted the last piece of media, return to the conversation view
+                                                self.dismissSelf(animated: true)
+                                            }
+                                            mediaGalleryDataSource.delete(message: deletedItem.message)
+        }
+        actionSheet.addAction(OWSAlerts.cancelAction)
+        actionSheet.addAction(deleteAction)
+
+        self.present(actionSheet, animated: true)
     }
 
     @objc

Signal/src/ViewControllers/MediaTileViewController.swift

@@ -8,7 +8,7 @@ public protocol MediaTileViewControllerDelegate: class {
     func mediaTileViewController(_ viewController: MediaTileViewController, didTapView tappedView: UIView, mediaGalleryItem: MediaGalleryItem)
 }
 
-public class MediaTileViewController: UICollectionViewController, MediaGalleryCellDelegate {
+public class MediaTileViewController: UICollectionViewController, MediaGalleryCellDelegate, MediaGalleryDataSourceDelegate {
 
     private weak var mediaGalleryDataSource: MediaGalleryDataSource?
 
@@ -143,14 +143,31 @@ public class MediaTileViewController: UICollectionViewController, MediaGalleryCe
         }
     }
 
-    // MARK: UIColletionViewDataSource
+    // MARK: MediaGalleryDataSourceDelegate
 
-    public func updatedDataSource(deletedSections: IndexSet, deletedItems: [IndexPath]) {
+    func mediaGalleryDataSource(_ mediaGalleryDataSource: MediaGalleryDataSource, willDelete message: TSMessage) {
+        guard let collectionView = self.collectionView else {
+            owsFail("\(logTag) in \(#function) collectionView was unexpectedly nil")
+            return
+        }
+
+        // We've got to lay out the collectionView before any changes are made to the date source
+        // otherwise we'll fail when we try to remove the deleted sections/rows
+        collectionView.layoutIfNeeded()
+    }
+
+    func mediaGalleryDataSource(_ mediaGalleryDataSource: MediaGalleryDataSource, deletedSections: IndexSet, deletedItems: [IndexPath]) {
         guard let collectionView = self.collectionView else {
             owsFail("\(logTag) in \(#function) collectionView was unexpetedly nil")
             return
         }
 
+        guard mediaGalleryDataSource.galleryItemCount > 0  else {
+            // Show Empty
+            self.collectionView?.reloadData()
+            return
+        }
+
         // If collectionView hasn't been laid out yet, it won't have the sections/rows to remove.
         collectionView.performBatchUpdates({
             collectionView.deleteSections(deletedSections)
@@ -158,6 +175,8 @@ public class MediaTileViewController: UICollectionViewController, MediaGalleryCe
         })
     }
 
+    // MARK: UICollectionViewDataSource
+
     override public func numberOfSections(in collectionView: UICollectionView) -> Int {
         guard galleryDates.count > 0 else {
             // empty gallery

Signal/src/ViewControllers/MessageDetailViewController.swift

@@ -12,10 +12,7 @@ enum MessageMetadataViewMode: UInt {
     case focusOnMetadata
 }
 
-class MessageDetailViewController: OWSViewController, UIScrollViewDelegate, MediaDetailPresenter {
+class MessageDetailViewController: OWSViewController, UIScrollViewDelegate, MediaDetailPresenter, MediaGalleryDataSourceDelegate {
-
-    static let TAG = "[MessageDetailViewController]"
-    let TAG = "[MessageDetailViewController]"
 
     // MARK: Properties
 
@@ -172,7 +169,7 @@ class MessageDetailViewController: OWSViewController, UIScrollViewDelegate, Medi
 
     private func updateContent() {
         guard let contentView = contentView else {
-            owsFail("\(TAG) Missing contentView")
+            owsFail("\(logTag) Missing contentView")
             return
         }
 
@@ -393,7 +390,7 @@ class MessageDetailViewController: OWSViewController, UIScrollViewDelegate, Medi
 
         if rows.count == 0 {
             // Neither attachment nor body.
-            owsFail("\(self.TAG) Message has neither attachment nor body.")
+            owsFail("\(self.logTag) 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: ""))
@@ -412,7 +409,7 @@ class MessageDetailViewController: OWSViewController, UIScrollViewDelegate, Medi
         }
 
         guard let attachment = TSAttachment.fetch(uniqueId: attachmentId, transaction: transaction) else {
-            Logger.warn("\(TAG) Missing attachment. Was it deleted?")
+            Logger.warn("\(logTag) Missing attachment. Was it deleted?")
             return nil
         }
 
@@ -423,7 +420,7 @@ class MessageDetailViewController: OWSViewController, UIScrollViewDelegate, Medi
         var rows = [UIView]()
 
         guard let attachment = self.attachment else {
-            Logger.warn("\(TAG) Missing attachment. Was it deleted?")
+            Logger.warn("\(logTag) Missing attachment. Was it deleted?")
             return rows
         }
 
@@ -556,7 +553,7 @@ class MessageDetailViewController: OWSViewController, UIScrollViewDelegate, Medi
 
     func shareButtonPressed() {
         guard let attachmentStream = attachmentStream else {
-            Logger.error("\(TAG) Share button should only be shown with attachment, but no attachment found.")
+            Logger.error("\(logTag) Share button should only be shown with attachment, but no attachment found.")
             return
         }
         AttachmentSharing.showShareUI(forAttachment: attachmentStream)
@@ -569,15 +566,15 @@ class MessageDetailViewController: OWSViewController, UIScrollViewDelegate, Medi
         }
 
         guard let attachmentStream = attachmentStream else {
-            Logger.error("\(TAG) Message has neither attachment nor message body.")
+            Logger.error("\(logTag) Message has neither attachment nor message body.")
             return
         }
         guard let utiType = MIMETypeUtil.utiType(forMIMEType: attachmentStream.contentType) else {
-            Logger.error("\(TAG) Attachment has invalid MIME type: \(attachmentStream.contentType).")
+            Logger.error("\(logTag) Attachment has invalid MIME type: \(attachmentStream.contentType).")
             return
         }
         guard let dataSource = dataSource else {
-            Logger.error("\(TAG) Attachment missing data source.")
+            Logger.error("\(logTag) Attachment missing data source.")
             return
         }
         let data = dataSource.data()
@@ -593,11 +590,11 @@ class MessageDetailViewController: OWSViewController, UIScrollViewDelegate, Medi
 
         self.uiDatabaseConnection.read { transaction in
             guard let uniqueId = self.message.uniqueId else {
-                Logger.error("\(self.TAG) Message is missing uniqueId.")
+                Logger.error("\(self.logTag) Message is missing uniqueId.")
                 return
             }
             guard let newMessage = TSInteraction.fetch(uniqueId: uniqueId, transaction: transaction) as? TSMessage else {
-                Logger.error("\(self.TAG) Couldn't reload message.")
+                Logger.error("\(self.logTag) Couldn't reload message.")
                 return
             }
             self.message = newMessage
@@ -608,25 +605,25 @@ class MessageDetailViewController: OWSViewController, UIScrollViewDelegate, Medi
     internal func yapDatabaseModified(notification: NSNotification) {
         AssertIsOnMainThread()
 
+        guard !wasDeleted else {
+            // Item was deleted. Don't bother re-rendering, it will fail and we'll soon be dismissed.
+            return
+        }
+
         let notifications = self.uiDatabaseConnection.beginLongLivedReadTransaction()
 
         guard let uniqueId = self.message.uniqueId else {
-            Logger.error("\(self.TAG) Message is missing uniqueId.")
+            Logger.error("\(self.logTag) Message is missing uniqueId.")
             return
         }
         guard self.uiDatabaseConnection.hasChange(forKey: uniqueId,
                                                  inCollection: TSInteraction.collection(),
                                                  in: notifications) else {
-                                                    Logger.debug("\(TAG) No relevant changes.")
+                                                    Logger.debug("\(logTag) No relevant changes.")
                                                     return
         }
 
         updateDBConnectionAndMessageToLatest()
-
-        guard !wasDeleted else {
-            // Item was deleted. Don't bother re-rendering, it will fail and we'll soon be dismissed.
-            return
-        }
         updateContent()
     }
 
@@ -674,44 +671,44 @@ class MessageDetailViewController: OWSViewController, UIScrollViewDelegate, Medi
             return
         }
         guard let messageTextProxyView = messageTextProxyView else {
-            owsFail("\(TAG) Missing messageTextProxyView")
+            owsFail("\(logTag) Missing messageTextProxyView")
             return
         }
         guard let scrollView = scrollView else {
-            owsFail("\(TAG) Missing scrollView")
+            owsFail("\(logTag) Missing scrollView")
             return
         }
         guard let contentView = contentView else {
-            owsFail("\(TAG) Missing contentView")
+            owsFail("\(logTag) Missing contentView")
             return
         }
         guard let bubbleView = bubbleView else {
-            owsFail("\(TAG) Missing bubbleView")
+            owsFail("\(logTag) Missing bubbleView")
             return
         }
         guard let bubbleSuperview = bubbleView.superview else {
-            owsFail("\(TAG) Missing bubbleSuperview")
+            owsFail("\(logTag) Missing bubbleSuperview")
             return
         }
         guard let messageTextTopConstraint = messageTextTopConstraint else {
-            owsFail("\(TAG) Missing messageTextTopConstraint")
+            owsFail("\(logTag) Missing messageTextTopConstraint")
             return
         }
         guard let messageTextHeightLayoutConstraint = messageTextHeightLayoutConstraint else {
-            owsFail("\(TAG) Missing messageTextHeightLayoutConstraint")
+            owsFail("\(logTag) Missing messageTextHeightLayoutConstraint")
             return
         }
         guard let messageTextProxyViewHeightConstraint = messageTextProxyViewHeightConstraint else {
-            owsFail("\(TAG) Missing messageTextProxyViewHeightConstraint")
+            owsFail("\(logTag) Missing messageTextProxyViewHeightConstraint")
             return
         }
         guard let bubbleViewWidthConstraint = bubbleViewWidthConstraint else {
-            owsFail("\(TAG) Missing bubbleViewWidthConstraint")
+            owsFail("\(logTag) Missing bubbleViewWidthConstraint")
             return
         }
 
         if messageTextView.width() != messageTextProxyView.width() {
-            owsFail("\(TAG) messageTextView.width \(messageTextView.width) != messageTextProxyView.width \(messageTextProxyView.width)")
+            owsFail("\(logTag) messageTextView.width \(messageTextView.width) != messageTextProxyView.width \(messageTextProxyView.width)")
         }
 
         let maxBubbleWidth = bubbleSuperview.width() - (bubbleViewHMargin * 2)
@@ -754,11 +751,30 @@ class MessageDetailViewController: OWSViewController, UIScrollViewDelegate, Medi
     }
 
     public func scrollViewDidScroll(_ scrollView: UIScrollView) {
-        Logger.verbose("\(TAG) scrollViewDidScroll")
+        Logger.verbose("\(logTag) scrollViewDidScroll")
 
         updateTextLayout()
     }
 
+    // MediaGalleryDataSourceDelegate
+
+    func mediaGalleryDataSource(_ mediaGalleryDataSource: MediaGalleryDataSource, willDelete message: TSMessage) {
+        Logger.info("\(self.logTag) in \(#function)")
+        guard message == self.message else {
+            // Should only be one message we can delete when viewing message details
+            owsFail("\(logTag) in \(#function) Unexpectedly informed of irrelevant message deletion")
+            return
+        }
+
+        self.wasDeleted = true
+    }
+
+    func mediaGalleryDataSource(_ mediaGalleryDataSource: MediaGalleryDataSource, deletedSections: IndexSet, deletedItems: [IndexPath]) {
+        self.dismiss(animated: true) {
+            self.navigationController?.popViewController(animated: true)
+        }
+    }
+
     // MARK: MediaDetailPresenter
 
     public func presentDetails(mediaMessageView: MediaMessageView, fromView: UIView) {
@@ -768,6 +784,7 @@ class MessageDetailViewController: OWSViewController, UIScrollViewDelegate, Medi
         }
 
         let mediaGalleryViewController = MediaGalleryViewController(thread: self.thread, uiDatabaseConnection: self.uiDatabaseConnection)
+        mediaGalleryViewController.dataSourceDelegate = self
         mediaGalleryViewController.presentDetailView(fromViewController: self, mediaMessage: self.message, replacingView: fromView)
     }
 }

Signal/translations/en.lproj/Localizable.strings

@@ -774,7 +774,7 @@
 "FINISH_GROUP_CREATION_LABEL" = "Finish creating group";
 
 /* Label indicating media gallery is empty */
-"GALLERY_TILES_EMPTY_GALLERY" = "You haven't sent or received any media in this conversation yet.";
+"GALLERY_TILES_EMPTY_GALLERY" = "You don't have any media in this conversation.";
 
 /* Label indicating loading is in progress */
 "GALLERY_TILES_LOADING_MORE_RECENT_LABEL" = "Loading Newer Media...";