diff --git a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageCell.m b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageCell.m index 9946d632d..14469c223 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageCell.m +++ b/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 diff --git a/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m b/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m index a4c3a098e..b7f38ac45 100644 --- a/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m +++ b/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]; diff --git a/Signal/src/ViewControllers/MediaDetailViewController.m b/Signal/src/ViewControllers/MediaDetailViewController.m index cb0e18445..98c251a1b 100644 --- a/Signal/src/ViewControllers/MediaDetailViewController.m +++ b/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 = - [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]; + [self.delegate mediaDetailViewController:self requestDeleteConversationViewItem:self.viewItem]; } - (BOOL)canPerformAction:(SEL)action withSender:(nullable id)sender diff --git a/Signal/src/ViewControllers/MediaGalleryViewController.swift b/Signal/src/ViewControllers/MediaGalleryViewController.swift index 9b2c82ac9..9f1d5c2d7 100644 --- a/Signal/src/ViewControllers/MediaGalleryViewController.swift +++ b/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 = 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.mediaTileViewController.collectionView!.layoutIfNeeded() + self.dataSourceDelegate?.mediaGalleryDataSource(self, willDelete: message) 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 - - // notify tile view - self.mediaTileViewController.updatedDataSource(deletedSections: deletedSections, deletedItems: deletedIndexPaths) + self.dataSourceDelegate?.mediaGalleryDataSource(self, 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 } } diff --git a/Signal/src/ViewControllers/MediaPageViewController.swift b/Signal/src/ViewControllers/MediaPageViewController.swift index 07f8fc564..5dfe651d7 100644 --- a/Signal/src/ViewControllers/MediaPageViewController.swift +++ b/Signal/src/ViewControllers/MediaPageViewController.swift @@ -47,16 +47,20 @@ class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSou return currentViewController.galleryItemBox.value } set { - guard let galleryPage = self.buildGalleryPage(galleryItem: newValue) else { - owsFail("unexpetedly unable to build new gallery page") - return - } - - self.updateTitle(item: newValue) - self.setViewControllers([galleryPage], direction: .forward, animated: false, completion: nil) + setCurrentItem(newValue, direction: .forward, animated: false) } } + private func setCurrentItem(_ item: MediaGalleryItem, direction: UIPageViewControllerNavigationDirection, animated isAnimated: Bool) { + 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 private let showAllMediaButton: Bool @@ -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 diff --git a/Signal/src/ViewControllers/MediaTileViewController.swift b/Signal/src/ViewControllers/MediaTileViewController.swift index 0c123c7f5..0a0ec40d7 100644 --- a/Signal/src/ViewControllers/MediaTileViewController.swift +++ b/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 diff --git a/Signal/src/ViewControllers/MessageDetailViewController.swift b/Signal/src/ViewControllers/MessageDetailViewController.swift index 5008438c2..0a14b5b4d 100644 --- a/Signal/src/ViewControllers/MessageDetailViewController.swift +++ b/Signal/src/ViewControllers/MessageDetailViewController.swift @@ -12,10 +12,7 @@ enum MessageMetadataViewMode: UInt { case focusOnMetadata } -class MessageDetailViewController: OWSViewController, UIScrollViewDelegate, MediaDetailPresenter { - - static let TAG = "[MessageDetailViewController]" - let TAG = "[MessageDetailViewController]" +class MessageDetailViewController: OWSViewController, UIScrollViewDelegate, MediaDetailPresenter, MediaGalleryDataSourceDelegate { // 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) } } diff --git a/Signal/translations/en.lproj/Localizable.strings b/Signal/translations/en.lproj/Localizable.strings index e62085a68..7e28c1985 100644 --- a/Signal/translations/en.lproj/Localizable.strings +++ b/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...";