From ca1119e48033294370dd3729cbd5a1468dd7233c Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Tue, 27 Nov 2018 16:05:03 -0700 Subject: [PATCH 1/4] extract method for clarity --- .../PhotoCollectionPickerController.swift | 101 +++++++++--------- 1 file changed, 53 insertions(+), 48 deletions(-) diff --git a/Signal/src/ViewControllers/PhotoLibrary/PhotoCollectionPickerController.swift b/Signal/src/ViewControllers/PhotoLibrary/PhotoCollectionPickerController.swift index bbe0ebf58..bdb5436d1 100644 --- a/Signal/src/ViewControllers/PhotoLibrary/PhotoCollectionPickerController.swift +++ b/Signal/src/ViewControllers/PhotoLibrary/PhotoCollectionPickerController.swift @@ -81,63 +81,68 @@ class PhotoCollectionPickerController: OWSTableViewController, PhotoLibraryDeleg } } + // MARK: - + private func updateContents() { photoCollections = library.allPhotoCollections() - let section = OWSTableSection() - for collection in photoCollections { - section.add(OWSTableItem.init(customCellBlock: { () -> UITableViewCell in - let cell = OWSTableItem.newCell() - - cell.backgroundColor = .ows_gray95 - cell.contentView.backgroundColor = .ows_gray95 - cell.selectedBackgroundView?.backgroundColor = UIColor(white: 0.2, alpha: 1) - - let imageView = UIImageView() - let kImageSize = 50 - imageView.autoSetDimensions(to: CGSize(width: kImageSize, height: kImageSize)) - - let contents = collection.contents() - if contents.count > 0 { - let photoMediaSize = PhotoMediaSize(thumbnailSize: CGSize(width: kImageSize, height: kImageSize)) - let assetItem = contents.assetItem(at: 0, photoMediaSize: photoMediaSize) - imageView.image = assetItem.asyncThumbnail { [weak imageView] image in - guard let strongImageView = imageView else { - return - } - guard let image = image else { - return - } - strongImageView.image = image - } - } - - let titleLabel = UILabel() - titleLabel.text = collection.localizedTitle() - titleLabel.font = UIFont.ows_dynamicTypeBody - titleLabel.textColor = .ows_gray05 - - let stackView = UIStackView(arrangedSubviews: [imageView, titleLabel]) - stackView.axis = .horizontal - stackView.alignment = .center - stackView.spacing = 10 - - cell.contentView.addSubview(stackView) - stackView.ows_autoPinToSuperviewMargins() - - return cell - }, - customRowHeight: UITableViewAutomaticDimension, - actionBlock: { [weak self] in - guard let strongSelf = self else { return } - strongSelf.didSelectCollection(collection: collection) - })) + let sectionItems = photoCollections.map { collection in + return OWSTableItem(customCellBlock: { self.buildTableCell(collection: collection) }, + customRowHeight: UITableViewAutomaticDimension, + actionBlock: { [weak self] in + guard let strongSelf = self else { return } + strongSelf.didSelectCollection(collection: collection) + }) } + + let section = OWSTableSection(title: nil, items: sectionItems) let contents = OWSTableContents() contents.addSection(section) self.contents = contents } + func buildTableCell(collection: PhotoCollection) -> UITableViewCell { + let cell = OWSTableItem.newCell() + + cell.backgroundColor = .ows_gray95 + cell.contentView.backgroundColor = .ows_gray95 + cell.selectedBackgroundView?.backgroundColor = UIColor(white: 0.2, alpha: 1) + + let imageView = UIImageView() + let kImageSize = 50 + imageView.autoSetDimensions(to: CGSize(width: kImageSize, height: kImageSize)) + + let contents = collection.contents() + if contents.count > 0 { + let photoMediaSize = PhotoMediaSize(thumbnailSize: CGSize(width: kImageSize, height: kImageSize)) + let assetItem = contents.assetItem(at: 0, photoMediaSize: photoMediaSize) + imageView.image = assetItem.asyncThumbnail { [weak imageView] image in + guard let strongImageView = imageView else { + return + } + guard let image = image else { + return + } + strongImageView.image = image + } + } + + let titleLabel = UILabel() + titleLabel.text = collection.localizedTitle() + titleLabel.font = UIFont.ows_dynamicTypeBody + titleLabel.textColor = .ows_gray05 + + let stackView = UIStackView(arrangedSubviews: [imageView, titleLabel]) + stackView.axis = .horizontal + stackView.alignment = .center + stackView.spacing = 10 + + cell.contentView.addSubview(stackView) + stackView.ows_autoPinToSuperviewMargins() + + return cell + } + @objc public override func applyTheme() { view.backgroundColor = .ows_gray95 From 87d1338412d9778aeef060226ebd67596e5da9c7 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Tue, 27 Nov 2018 16:05:26 -0700 Subject: [PATCH 2/4] remove unused code --- .../PhotoLibrary/PhotoCollectionPickerController.swift | 7 ------- 1 file changed, 7 deletions(-) diff --git a/Signal/src/ViewControllers/PhotoLibrary/PhotoCollectionPickerController.swift b/Signal/src/ViewControllers/PhotoLibrary/PhotoCollectionPickerController.swift index bdb5436d1..5e89b3f64 100644 --- a/Signal/src/ViewControllers/PhotoLibrary/PhotoCollectionPickerController.swift +++ b/Signal/src/ViewControllers/PhotoLibrary/PhotoCollectionPickerController.swift @@ -104,7 +104,6 @@ class PhotoCollectionPickerController: OWSTableViewController, PhotoLibraryDeleg func buildTableCell(collection: PhotoCollection) -> UITableViewCell { let cell = OWSTableItem.newCell() - cell.backgroundColor = .ows_gray95 cell.contentView.backgroundColor = .ows_gray95 cell.selectedBackgroundView?.backgroundColor = UIColor(white: 0.2, alpha: 1) @@ -143,12 +142,6 @@ class PhotoCollectionPickerController: OWSTableViewController, PhotoLibraryDeleg return cell } - @objc - public override func applyTheme() { - view.backgroundColor = .ows_gray95 - tableView.backgroundColor = .ows_gray95 - } - // MARK: Actions @objc From 58eda67a792f241f0f94bdb4239835d811cc6d04 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Tue, 27 Nov 2018 16:41:04 -0700 Subject: [PATCH 3/4] show *most recent* thumbnail in album picker --- .../PhotoLibrary/ImagePickerController.swift | 4 +- .../PhotoCollectionPickerController.swift | 7 +-- .../PhotoLibrary/PhotoLibrary.swift | 48 +++++++++++++++---- 3 files changed, 46 insertions(+), 13 deletions(-) diff --git a/Signal/src/ViewControllers/PhotoLibrary/ImagePickerController.swift b/Signal/src/ViewControllers/PhotoLibrary/ImagePickerController.swift index df01402df..ca7ef180e 100644 --- a/Signal/src/ViewControllers/PhotoLibrary/ImagePickerController.swift +++ b/Signal/src/ViewControllers/PhotoLibrary/ImagePickerController.swift @@ -159,7 +159,7 @@ class ImagePickerGridController: UICollectionViewController, PhotoLibraryDelegat collectionView.reloadData() collectionView.layoutIfNeeded() - let count = photoCollectionContents.count + let count = photoCollectionContents.assetCount for index in 0.. Int { - return photoCollectionContents.count + return photoCollectionContents.assetCount } override func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell { diff --git a/Signal/src/ViewControllers/PhotoLibrary/PhotoCollectionPickerController.swift b/Signal/src/ViewControllers/PhotoLibrary/PhotoCollectionPickerController.swift index 5e89b3f64..4e6c83a1a 100644 --- a/Signal/src/ViewControllers/PhotoLibrary/PhotoCollectionPickerController.swift +++ b/Signal/src/ViewControllers/PhotoLibrary/PhotoCollectionPickerController.swift @@ -112,14 +112,15 @@ class PhotoCollectionPickerController: OWSTableViewController, PhotoLibraryDeleg imageView.autoSetDimensions(to: CGSize(width: kImageSize, height: kImageSize)) let contents = collection.contents() - if contents.count > 0 { - let photoMediaSize = PhotoMediaSize(thumbnailSize: CGSize(width: kImageSize, height: kImageSize)) - let assetItem = contents.assetItem(at: 0, photoMediaSize: photoMediaSize) + + let photoMediaSize = PhotoMediaSize(thumbnailSize: CGSize(width: kImageSize, height: kImageSize)) + if let assetItem = contents.lastAssetItem(photoMediaSize: photoMediaSize) { imageView.image = assetItem.asyncThumbnail { [weak imageView] image in guard let strongImageView = imageView else { return } guard let image = image else { + owsFailDebug("image was unexpectedly nil") return } strongImageView.image = image diff --git a/Signal/src/ViewControllers/PhotoLibrary/PhotoLibrary.swift b/Signal/src/ViewControllers/PhotoLibrary/PhotoLibrary.swift index bbcbc45dc..7d183aa0e 100644 --- a/Signal/src/ViewControllers/PhotoLibrary/PhotoLibrary.swift +++ b/Signal/src/ViewControllers/PhotoLibrary/PhotoLibrary.swift @@ -25,12 +25,12 @@ class PhotoMediaSize { class PhotoPickerAssetItem: PhotoGridItem { let asset: PHAsset - let album: PhotoCollectionContents + let photoCollectionContents: PhotoCollectionContents let photoMediaSize: PhotoMediaSize - init(asset: PHAsset, album: PhotoCollectionContents, photoMediaSize: PhotoMediaSize) { + init(asset: PHAsset, photoCollectionContents: PhotoCollectionContents, photoMediaSize: PhotoMediaSize) { self.asset = asset - self.album = album + self.photoCollectionContents = photoCollectionContents self.photoMediaSize = photoMediaSize } @@ -47,7 +47,7 @@ class PhotoPickerAssetItem: PhotoGridItem { } func asyncThumbnail(completion: @escaping (UIImage?) -> Void) -> UIImage? { - album.requestThumbnail(for: self.asset, thumbnailSize: photoMediaSize.thumbnailSize) { image, _ in + photoCollectionContents.requestThumbnail(for: self.asset, thumbnailSize: photoMediaSize.thumbnailSize) { image, _ in completion(image) } return nil @@ -70,19 +70,51 @@ class PhotoCollectionContents { self.localizedTitle = localizedTitle } - var count: Int { + private let imageManager = PHCachingImageManager() + + // MARK: - Asset Accessors + + var assetCount: Int { return fetchResult.count } - private let imageManager = PHCachingImageManager() + var lastAsset: PHAsset? { + guard assetCount > 0 else { + return nil + } + return asset(at: assetCount - 1) + } + + var firstAsset: PHAsset? { + guard assetCount > 0 else { + return nil + } + return asset(at: 0) + } func asset(at index: Int) -> PHAsset { return fetchResult.object(at: index) } + // MARK: - AssetItem Accessors + func assetItem(at index: Int, photoMediaSize: PhotoMediaSize) -> PhotoPickerAssetItem { let mediaAsset = asset(at: index) - return PhotoPickerAssetItem(asset: mediaAsset, album: self, photoMediaSize: photoMediaSize) + return PhotoPickerAssetItem(asset: mediaAsset, photoCollectionContents: self, photoMediaSize: photoMediaSize) + } + + func firstAssetItem(photoMediaSize: PhotoMediaSize) -> PhotoPickerAssetItem? { + guard let mediaAsset = firstAsset else { + return nil + } + return PhotoPickerAssetItem(asset: mediaAsset, photoCollectionContents: self, photoMediaSize: photoMediaSize) + } + + func lastAssetItem(photoMediaSize: PhotoMediaSize) -> PhotoPickerAssetItem? { + guard let mediaAsset = lastAsset else { + return nil + } + return PhotoPickerAssetItem(asset: mediaAsset, photoCollectionContents: self, photoMediaSize: photoMediaSize) } // MARK: ImageManager @@ -242,7 +274,7 @@ class PhotoLibrary: NSObject, PHPhotoLibraryChangeObserver { } let photoCollection = PhotoCollection(collection: assetCollection) // Hide empty collections. - guard photoCollection.contents().count > 0 else { + guard photoCollection.contents().assetCount > 0 else { return } collections.append(photoCollection) From 46102e57b7060784f1a57b761d4a9d455d40b819 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Tue, 27 Nov 2018 17:48:14 -0700 Subject: [PATCH 4/4] AlbumPicker cells to spec --- .../PhotoCollectionPickerController.swift | 53 ++++++++++++------- 1 file changed, 35 insertions(+), 18 deletions(-) diff --git a/Signal/src/ViewControllers/PhotoLibrary/PhotoCollectionPickerController.swift b/Signal/src/ViewControllers/PhotoLibrary/PhotoCollectionPickerController.swift index 4e6c83a1a..80a143722 100644 --- a/Signal/src/ViewControllers/PhotoLibrary/PhotoCollectionPickerController.swift +++ b/Signal/src/ViewControllers/PhotoLibrary/PhotoCollectionPickerController.swift @@ -38,6 +38,7 @@ class PhotoCollectionPickerController: OWSTableViewController, PhotoLibraryDeleg super.viewDidLoad() view.backgroundColor = .ows_gray95 + tableView.separatorColor = .clear if #available(iOS 11, *) { let titleLabel = UILabel() @@ -101,44 +102,60 @@ class PhotoCollectionPickerController: OWSTableViewController, PhotoLibraryDeleg self.contents = contents } - func buildTableCell(collection: PhotoCollection) -> UITableViewCell { + private let numberFormatter: NumberFormatter = NumberFormatter() + + private func buildTableCell(collection: PhotoCollection) -> UITableViewCell { let cell = OWSTableItem.newCell() - cell.contentView.backgroundColor = .ows_gray95 + cell.contentView.backgroundColor = Theme.darkThemeBackgroundColor cell.selectedBackgroundView?.backgroundColor = UIColor(white: 0.2, alpha: 1) + let contents = collection.contents() + + let titleLabel = UILabel() + titleLabel.text = collection.localizedTitle() + titleLabel.font = UIFont.ows_dynamicTypeBody + titleLabel.textColor = Theme.darkThemePrimaryColor + + let countLabel = UILabel() + countLabel.text = numberFormatter.string(for: contents.assetCount) + countLabel.font = UIFont.ows_dynamicTypeCaption1 + countLabel.textColor = Theme.darkThemePrimaryColor + + let textStack = UIStackView(arrangedSubviews: [titleLabel, countLabel]) + textStack.axis = .vertical + textStack.alignment = .leading + textStack.spacing = 2 + let imageView = UIImageView() - let kImageSize = 50 + let kImageSize = 80 imageView.autoSetDimensions(to: CGSize(width: kImageSize, height: kImageSize)) - let contents = collection.contents() + let hStackView = UIStackView(arrangedSubviews: [imageView, textStack]) + hStackView.axis = .horizontal + hStackView.alignment = .center + hStackView.spacing = 11 let photoMediaSize = PhotoMediaSize(thumbnailSize: CGSize(width: kImageSize, height: kImageSize)) if let assetItem = contents.lastAssetItem(photoMediaSize: photoMediaSize) { imageView.image = assetItem.asyncThumbnail { [weak imageView] image in - guard let strongImageView = imageView else { + AssertIsOnMainThread() + + guard let imageView = imageView else { return } + guard let image = image else { owsFailDebug("image was unexpectedly nil") return } - strongImageView.image = image + + imageView.image = image } } - let titleLabel = UILabel() - titleLabel.text = collection.localizedTitle() - titleLabel.font = UIFont.ows_dynamicTypeBody - titleLabel.textColor = .ows_gray05 - - let stackView = UIStackView(arrangedSubviews: [imageView, titleLabel]) - stackView.axis = .horizontal - stackView.alignment = .center - stackView.spacing = 10 - - cell.contentView.addSubview(stackView) - stackView.ows_autoPinToSuperviewMargins() + cell.contentView.addSubview(hStackView) + hStackView.ows_autoPinToSuperviewMargins() return cell }