diff --git a/Signal/src/ViewControllers/MediaGalleryViewController.swift b/Signal/src/ViewControllers/MediaGalleryViewController.swift index e1d715436..d5278c558 100644 --- a/Signal/src/ViewControllers/MediaGalleryViewController.swift +++ b/Signal/src/ViewControllers/MediaGalleryViewController.swift @@ -176,11 +176,11 @@ protocol MediaGalleryDataSource: class { func showAllMedia(focusedItem: MediaGalleryItem) func dismissMediaDetailViewController(_ mediaDetailViewController: MediaPageViewController, animated isAnimated: Bool, completion: (() -> Void)?) - func delete(items: [MediaGalleryItem]) + func delete(items: [MediaGalleryItem], initiatedBy: MediaGalleryDataSourceDelegate) } protocol MediaGalleryDataSourceDelegate: class { - func mediaGalleryDataSource(_ mediaGalleryDataSource: MediaGalleryDataSource, willDelete items: [MediaGalleryItem]) + func mediaGalleryDataSource(_ mediaGalleryDataSource: MediaGalleryDataSource, willDelete items: [MediaGalleryItem], initiatedBy: MediaGalleryDataSourceDelegate) func mediaGalleryDataSource(_ mediaGalleryDataSource: MediaGalleryDataSource, deletedSections: IndexSet, deletedItems: [IndexPath]) } @@ -280,6 +280,7 @@ class MediaGalleryViewController: UINavigationController, MediaGalleryDataSource self.initialDetailItem = initialDetailItem let pageViewController = MediaPageViewController(initialItem: initialDetailItem, mediaGalleryDataSource: self, uiDatabaseConnection: self.uiDatabaseConnection, options: self.options) + self.addDataSourceDelegate(pageViewController) self.pageViewController = pageViewController self.setViewControllers([pageViewController], animated: false) @@ -467,7 +468,7 @@ class MediaGalleryViewController: UINavigationController, MediaGalleryDataSource if isAnimated { UIView.animate(withDuration: changedItems ? 0.25 : 0.18, delay: 0.0, - options:.curveEaseOut, + options: .curveEaseOut, animations: { // Move back over it's original location self.presentationView.superview?.layoutIfNeeded() @@ -535,8 +536,8 @@ class MediaGalleryViewController: UINavigationController, MediaGalleryDataSource self.presentationViewConstraints += self.presentationView.autoSetDimensions(to: convertedRect.size) self.presentationViewConstraints += [ - self.presentationView.autoPinEdge(toSuperviewEdge: .top, withInset:convertedRect.origin.y), - self.presentationView.autoPinEdge(toSuperviewEdge: .left, withInset:convertedRect.origin.x) + self.presentationView.autoPinEdge(toSuperviewEdge: .top, withInset: convertedRect.origin.y), + self.presentationView.autoPinEdge(toSuperviewEdge: .left, withInset: convertedRect.origin.x) ] } @@ -573,10 +574,7 @@ class MediaGalleryViewController: UINavigationController, MediaGalleryDataSource 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 + self.addDataSourceDelegate(vc) return vc }() @@ -735,15 +733,21 @@ class MediaGalleryViewController: UINavigationController, MediaGalleryDataSource } } - weak var dataSourceDelegate: MediaGalleryDataSourceDelegate? - var deletedMessages: Set = Set() + var dataSourceDelegates: [Weak] = [] + func addDataSourceDelegate(_ dataSourceDelegate: MediaGalleryDataSourceDelegate) { + dataSourceDelegates.append(Weak(value: dataSourceDelegate)) + } - func delete(items: [MediaGalleryItem]) { + var deletedMessages: Set = Set() + var deletedGalleryItems: Set = Set() + + func delete(items: [MediaGalleryItem], initiatedBy: MediaGalleryDataSourceDelegate) { AssertIsOnMainThread() - Logger.info("\(logTag) in \(#function) with items: \(items)") + Logger.info("\(logTag) in \(#function) with items: \(items.map { ($0.attachmentStream, $0.message.timestamp) })") - dataSourceDelegate?.mediaGalleryDataSource(self, willDelete: items) + deletedGalleryItems.formUnion(items) + dataSourceDelegates.forEach { $0.value?.mediaGalleryDataSource(self, willDelete: items, initiatedBy: initiatedBy) } self.editingDatabaseConnection.asyncReadWrite { transaction in for item in items { @@ -812,7 +816,7 @@ class MediaGalleryViewController: UINavigationController, MediaGalleryDataSource } } - dataSourceDelegate?.mediaGalleryDataSource(self, deletedSections: deletedSections, deletedItems: deletedIndexPaths) + dataSourceDelegates.forEach { $0.value?.mediaGalleryDataSource(self, deletedSections: deletedSections, deletedItems: deletedIndexPaths) } } let kGallerySwipeLoadBatchSize: UInt = 5 @@ -828,7 +832,17 @@ class MediaGalleryViewController: UINavigationController, MediaGalleryDataSource } let index: Int = galleryItems.index(after: currentIndex) - return galleryItems[safe: index] + guard let nextItem = galleryItems[safe: index] else { + // already at last item + return nil + } + + guard !deletedGalleryItems.contains(nextItem) else { + Logger.debug("\(self.logTag) in \(#function) nextItem was deleted - Recursing.") + return galleryItem(after: nextItem) + } + + return nextItem } internal func galleryItem(before currentItem: MediaGalleryItem) -> MediaGalleryItem? { @@ -842,7 +856,17 @@ class MediaGalleryViewController: UINavigationController, MediaGalleryDataSource } let index: Int = galleryItems.index(before: currentIndex) - return galleryItems[safe: index] + guard let previousItem = galleryItems[safe: index] else { + // already at first item + return nil + } + + guard !deletedGalleryItems.contains(previousItem) else { + Logger.debug("\(self.logTag) in \(#function) previousItem was deleted - Recursing.") + return galleryItem(before: previousItem) + } + + return previousItem } var galleryItemCount: Int { diff --git a/Signal/src/ViewControllers/MediaPageViewController.swift b/Signal/src/ViewControllers/MediaPageViewController.swift index ef0540c2e..20add8558 100644 --- a/Signal/src/ViewControllers/MediaPageViewController.swift +++ b/Signal/src/ViewControllers/MediaPageViewController.swift @@ -18,7 +18,7 @@ public class GalleryItemBox: NSObject { } } -fileprivate class Box { +private class Box { var value: A init(_ val: A) { self.value = val @@ -31,7 +31,7 @@ fileprivate extension MediaDetailViewController { } } -class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSource, UIPageViewControllerDelegate, MediaDetailViewControllerDelegate { +class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSource, UIPageViewControllerDelegate, MediaDetailViewControllerDelegate, MediaGalleryDataSourceDelegate { private weak var mediaGalleryDataSource: MediaGalleryDataSource? @@ -171,7 +171,7 @@ class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSou self.view.addSubview(footerBar) footerBar.autoPinWidthToSuperview() footerBar.autoPin(toBottomLayoutGuideOf: self, withInset: 0) - footerBar.autoSetDimension(.height, toSize:kFooterHeight) + footerBar.autoSetDimension(.height, toSize: kFooterHeight) // Gestures @@ -244,7 +244,7 @@ class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSou // Hiding the status bar affects the positioning of the navbar. We don't want to show that in an animation, it's // better to just have everythign "flit" in/out. - UIApplication.shared.setStatusBarHidden(shouldHideToolbars, with:.none) + UIApplication.shared.setStatusBarHidden(shouldHideToolbars, with: .none) self.navigationController?.setNavigationBarHidden(shouldHideToolbars, animated: false) // We don't animate the background color change because the old color shows through momentarily @@ -267,20 +267,20 @@ class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSou } var toolbarItems: [UIBarButtonItem] = [ - UIBarButtonItem(barButtonSystemItem: .action, target:self, action: #selector(didPressShare)), - UIBarButtonItem(barButtonSystemItem: .flexibleSpace, target:nil, action:nil) + UIBarButtonItem(barButtonSystemItem: .action, target: self, action: #selector(didPressShare)), + UIBarButtonItem(barButtonSystemItem: .flexibleSpace, target: nil, action: nil) ] if (self.currentItem.isVideo) { toolbarItems += [ isPlayingVideo ? self.videoPauseBarButton : self.videoPlayBarButton, - UIBarButtonItem(barButtonSystemItem: .flexibleSpace, target:nil, action:nil) + UIBarButtonItem(barButtonSystemItem: .flexibleSpace, target: nil, action: nil) ] } toolbarItems.append(UIBarButtonItem(barButtonSystemItem: .trash, - target:self, - action:#selector(didPressDelete))) + target: self, + action: #selector(didPressDelete))) self.footerBar.setItems(toolbarItems, animated: false) } @@ -317,18 +317,7 @@ class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSou 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(items: [deletedItem]) + mediaGalleryDataSource.delete(items: [deletedItem], initiatedBy: self) } actionSheet.addAction(OWSAlerts.cancelAction) actionSheet.addAction(deleteAction) @@ -336,6 +325,43 @@ class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSou self.present(actionSheet, animated: true) } + // MARK: MediaGalleryDataSourceDelegate + + func mediaGalleryDataSource(_ mediaGalleryDataSource: MediaGalleryDataSource, willDelete items: [MediaGalleryItem], initiatedBy: MediaGalleryDataSourceDelegate) { + Logger.debug("\(self.logTag) in \(#function)") + + guard let currentItem = self.currentItem else { + owsFail("\(logTag) in \(#function) currentItem was unexpectedly nil") + return + } + + guard items.contains(currentItem) else { + Logger.debug("\(self.logTag) in \(#function) irrelevant item") + return + } + + // If we setCurrentItem with (animated: true) while this VC is in the background, then + // the next/previous cache isn't expired, and we're able to swipe back to the just-deleted vc. + // So to get the correct behavior, we should only animate these transitions when this + // vc is in the foreground + let isAnimated = initiatedBy === self + + if !self.sliderEnabled { + // In message details, which doesn't use the slider, so don't swap pages. + } else if let nextItem = mediaGalleryDataSource.galleryItem(after: currentItem) { + self.setCurrentItem(nextItem, direction: .forward, animated: isAnimated) + } else if let previousItem = mediaGalleryDataSource.galleryItem(before: currentItem) { + self.setCurrentItem(previousItem, direction: .reverse, animated: isAnimated) + } else { + // else we deleted the last piece of media, return to the conversation view + self.dismissSelf(animated: true) + } + } + + func mediaGalleryDataSource(_ mediaGalleryDataSource: MediaGalleryDataSource, deletedSections: IndexSet, deletedItems: [IndexPath]) { + // no-op + } + @objc public func didPressPlayBarButton(_ sender: Any) { guard let currentViewController = self.viewControllers?[0] as? MediaDetailViewController else { @@ -512,7 +538,7 @@ class MediaPageViewController: UIPageViewController, UIPageViewControllerDataSou } dismissSelf(animated: true) { - mediaGalleryDataSource.delete(items: [galleryItem]) + mediaGalleryDataSource.delete(items: [galleryItem], initiatedBy: self) } } diff --git a/Signal/src/ViewControllers/MediaTileViewController.swift b/Signal/src/ViewControllers/MediaTileViewController.swift index b3462da81..f6a5ecf02 100644 --- a/Signal/src/ViewControllers/MediaTileViewController.swift +++ b/Signal/src/ViewControllers/MediaTileViewController.swift @@ -94,13 +94,13 @@ public class MediaTileViewController: UICollectionViewController, MediaGalleryDa let footerBar = UIToolbar() self.footerBar = footerBar let deleteButton = UIBarButtonItem(barButtonSystemItem: .trash, - target:self, - action:#selector(didPressDelete)) + target: self, + action: #selector(didPressDelete)) self.deleteButton = deleteButton let footerItems = [ - UIBarButtonItem(barButtonSystemItem: .flexibleSpace, target:nil, action:nil), + UIBarButtonItem(barButtonSystemItem: .flexibleSpace, target: nil, action: nil), deleteButton, - UIBarButtonItem(barButtonSystemItem: .flexibleSpace, target:nil, action:nil), + UIBarButtonItem(barButtonSystemItem: .flexibleSpace, target: nil, action: nil) ] footerBar.setItems(footerItems, animated: false) @@ -111,9 +111,6 @@ public class MediaTileViewController: UICollectionViewController, MediaGalleryDa self.footerBarBottomConstraint = footerBar.autoPinEdge(toSuperviewEdge: .bottom, withInset: -kFooterBarHeight) updateSelectButton() - - self.view.layoutIfNeeded() - scrollToBottom(animated: false) } private func indexPath(galleryItem: MediaGalleryItem) -> IndexPath? { @@ -140,7 +137,9 @@ public class MediaTileViewController: UICollectionViewController, MediaGalleryDa } Logger.debug("\(logTag) scrolling to focused item at indexPath: \(indexPath)") + self.view.layoutIfNeeded() self.collectionView?.scrollToItem(at: indexPath, at: .centeredVertically, animated: false) + self.autoLoadMoreIfNecessary() } // MARK: UICollectionViewDelegate @@ -482,6 +481,10 @@ public class MediaTileViewController: UICollectionViewController, MediaGalleryDa @objc func didCancelSelect(_ sender: Any) { + endSelectMode() + } + + func endSelectMode() { isInBatchSelectMode = false guard let collectionView = self.collectionView else { @@ -536,7 +539,8 @@ public class MediaTileViewController: UICollectionViewController, MediaGalleryDa }() let deleteAction = UIAlertAction(title: confirmationTitle, style: .destructive) { _ in - mediaGalleryDataSource.delete(items: items) + mediaGalleryDataSource.delete(items: items, initiatedBy: self) + self.endSelectMode() } let actionSheet = UIAlertController(title: nil, message: nil, preferredStyle: .actionSheet) @@ -553,7 +557,7 @@ public class MediaTileViewController: UICollectionViewController, MediaGalleryDa // MARK: MediaGalleryDataSourceDelegate - func mediaGalleryDataSource(_ mediaGalleryDataSource: MediaGalleryDataSource, willDelete items: [MediaGalleryItem]) { + func mediaGalleryDataSource(_ mediaGalleryDataSource: MediaGalleryDataSource, willDelete items: [MediaGalleryItem], initiatedBy: MediaGalleryDataSourceDelegate) { Logger.debug("\(self.logTag) in \(#function)") guard let collectionView = self.collectionView else { @@ -709,20 +713,6 @@ public class MediaTileViewController: UICollectionViewController, MediaGalleryDa } } } - - // MARK: Util - - private func scrollToBottom(animated isAnimated: Bool) { - guard let collectionView = self.collectionView else { - owsFail("\(self.logTag) in \(#function) collectionView was unexpectedly nil") - return - } - - let yOffset: CGFloat = collectionView.contentSize.height - collectionView.bounds.size.height + collectionView.contentInset.bottom - let offset: CGPoint = CGPoint(x: 0, y: yOffset) - - collectionView.setContentOffset(offset, animated: isAnimated) - } } // MARK: - Private Helper Classes @@ -730,7 +720,7 @@ public class MediaTileViewController: UICollectionViewController, MediaGalleryDa // Accomodates remaining scrolled to the same "apparent" position when new content is insterted // into the top of a collectionView. There are multiple ways to solve this problem, but this // is the only one which avoided a perceptible flicker. -fileprivate class MediaTileViewLayout: UICollectionViewFlowLayout { +private class MediaTileViewLayout: UICollectionViewFlowLayout { fileprivate var isInsertingCellsToTop: Bool = false fileprivate var contentSizeBeforeInsertingToTop: CGSize? @@ -751,7 +741,7 @@ fileprivate class MediaTileViewLayout: UICollectionViewFlowLayout { } } -fileprivate class MediaGallerySectionHeader: UICollectionReusableView { +private class MediaGallerySectionHeader: UICollectionReusableView { static let reuseIdentifier = "MediaGallerySectionHeader" @@ -813,7 +803,7 @@ fileprivate class MediaGallerySectionHeader: UICollectionReusableView { } } -fileprivate class MediaGalleryStaticHeader: UICollectionViewCell { +private class MediaGalleryStaticHeader: UICollectionViewCell { static let reuseIdentifier = "MediaGalleryStaticHeader" @@ -843,7 +833,7 @@ fileprivate class MediaGalleryStaticHeader: UICollectionViewCell { } } -fileprivate class MediaGalleryCell: UICollectionViewCell { +private class MediaGalleryCell: UICollectionViewCell { static let reuseIdentifier = "MediaGalleryCell" diff --git a/Signal/src/ViewControllers/MessageDetailViewController.swift b/Signal/src/ViewControllers/MessageDetailViewController.swift index 6eb86c153..1f9e964cb 100644 --- a/Signal/src/ViewControllers/MessageDetailViewController.swift +++ b/Signal/src/ViewControllers/MessageDetailViewController.swift @@ -758,7 +758,7 @@ class MessageDetailViewController: OWSViewController, UIScrollViewDelegate, Medi // MediaGalleryDataSourceDelegate - func mediaGalleryDataSource(_ mediaGalleryDataSource: MediaGalleryDataSource, willDelete items: [MediaGalleryItem]) { + func mediaGalleryDataSource(_ mediaGalleryDataSource: MediaGalleryDataSource, willDelete items: [MediaGalleryItem], initiatedBy: MediaGalleryDataSourceDelegate) { Logger.info("\(self.logTag) in \(#function)") guard (items.map({ $0.message }) == [self.message]) else { @@ -785,7 +785,7 @@ class MessageDetailViewController: OWSViewController, UIScrollViewDelegate, Medi } let mediaGalleryViewController = MediaGalleryViewController(thread: self.thread, uiDatabaseConnection: self.uiDatabaseConnection) - mediaGalleryViewController.dataSourceDelegate = self + mediaGalleryViewController.addDataSourceDelegate(self) mediaGalleryViewController.presentDetailView(fromViewController: self, mediaMessage: self.message, replacingView: fromView) } }