Respond to CR.

// FREEBIE
This commit is contained in:
Matthew Chen 2017-10-02 15:24:57 -04:00
parent fd9188415f
commit 8b7d34e51c
6 changed files with 99 additions and 99 deletions

View File

@ -27,19 +27,19 @@ class GifPickerCell: UICollectionViewCell {
}
}
// We do "progressive" loading by loading stills (jpg or gif) and "full" gifs.
// We do "progressive" loading by loading stills (jpg or gif) and "animated" gifs.
// This is critical on cellular connections.
var stillAssetRequest: GiphyAssetRequest?
var stillAsset: GiphyAsset?
var fullAssetRequest: GiphyAssetRequest?
var fullAsset: GiphyAsset?
var animatedAssetRequest: GiphyAssetRequest?
var animatedAsset: GiphyAsset?
var imageView: YYAnimatedImageView?
// MARK: Initializers
deinit {
stillAssetRequest?.cancel()
fullAssetRequest?.cancel()
animatedAssetRequest?.cancel()
}
override func prepareForReuse() {
@ -50,9 +50,9 @@ class GifPickerCell: UICollectionViewCell {
stillAsset = nil
stillAssetRequest?.cancel()
stillAssetRequest = nil
fullAsset = nil
fullAssetRequest?.cancel()
fullAssetRequest = nil
animatedAsset = nil
animatedAssetRequest?.cancel()
animatedAssetRequest = nil
imageView?.removeFromSuperview()
imageView = nil
}
@ -62,14 +62,14 @@ class GifPickerCell: UICollectionViewCell {
stillAssetRequest = nil
}
private func clearFullAssetRequest() {
fullAssetRequest?.cancel()
fullAssetRequest = nil
private func clearanimatedAssetRequest() {
animatedAssetRequest?.cancel()
animatedAssetRequest = nil
}
private func clearAssetRequests() {
clearStillAssetRequest()
clearFullAssetRequest()
clearanimatedAssetRequest()
}
public func ensureCellState() {
@ -84,12 +84,12 @@ class GifPickerCell: UICollectionViewCell {
clearAssetRequests()
return
}
guard self.fullAsset == nil else {
guard self.animatedAsset == nil else {
return
}
// The Giphy API returns a slew of "renditions" for a given image.
// It's critical that we carefully "pick" the best rendition to use.
guard let fullRendition = imageInfo.pickGifRendition() else {
guard let animatedRendition = imageInfo.pickAnimatedRendition() else {
Logger.warn("\(TAG) could not pick gif rendition: \(imageInfo.giphyId)")
clearAssetRequests()
return
@ -101,7 +101,7 @@ class GifPickerCell: UICollectionViewCell {
}
// Start still asset request if necessary.
if stillAsset == nil && fullAsset == nil && stillAssetRequest == nil {
if stillAsset == nil && animatedAsset == nil && stillAssetRequest == nil {
stillAssetRequest = GiphyDownloader.sharedInstance.requestAsset(rendition:stillRendition,
priority:.high,
success: { [weak self] assetRequest, asset in
@ -124,28 +124,28 @@ class GifPickerCell: UICollectionViewCell {
})
}
// Start full asset request if necessary.
if fullAsset == nil && fullAssetRequest == nil {
fullAssetRequest = GiphyDownloader.sharedInstance.requestAsset(rendition:fullRendition,
// Start animated asset request if necessary.
if animatedAsset == nil && animatedAssetRequest == nil {
animatedAssetRequest = GiphyDownloader.sharedInstance.requestAsset(rendition:animatedRendition,
priority:.low,
success: { [weak self] assetRequest, asset in
guard let strongSelf = self else { return }
if assetRequest != nil && assetRequest != strongSelf.fullAssetRequest {
if assetRequest != nil && assetRequest != strongSelf.animatedAssetRequest {
owsFail("Obsolete request callback.")
return
}
// If we have the full asset, we don't need the still asset.
// If we have the animated asset, we don't need the still asset.
strongSelf.clearAssetRequests()
strongSelf.fullAsset = asset
strongSelf.animatedAsset = asset
strongSelf.tryToDisplayAsset()
},
failure: { [weak self] assetRequest in
guard let strongSelf = self else { return }
if assetRequest != strongSelf.fullAssetRequest {
if assetRequest != strongSelf.animatedAssetRequest {
owsFail("Obsolete request callback.")
return
}
strongSelf.clearFullAssetRequest()
strongSelf.clearanimatedAssetRequest()
})
}
}
@ -173,12 +173,6 @@ class GifPickerCell: UICollectionViewCell {
}
private func pickBestAsset() -> GiphyAsset? {
if let fullAsset = fullAsset {
return fullAsset
}
if let stillAsset = stillAsset {
return stillAsset
}
return nil
return animatedAsset ?? stillAsset
}
}

View File

@ -54,31 +54,33 @@ class GifPickerLayout: UICollectionViewLayout {
return
}
let vMargin = UInt(5)
let hMargin = UInt(5)
let vInset = UInt(5)
let hInset = UInt(5)
let vSpacing = UInt(3)
let hSpacing = UInt(3)
// TODO: We probably want to use 2 or 3 columns.
// It might depend on the device.
// 2 columns will show fewer GIFs at a time,
// but use less network & be a more responsive experience.
let columnCount = UInt(3)
// We use 2 or 3 columns, depending on the device.
// 2 columns will show fewer GIFs at a time,
// but use less network & be a more responsive experience.
let screenSize = UIScreen.main.bounds.size
let screenWidth = min(screenSize.width, screenSize.height)
let columnCount = UInt(max(2, screenWidth / 130))
let totalViewWidth = UInt(collectionView.width())
let hTotalWhitespace = (2 * hMargin) + (hSpacing * (columnCount - 1))
let hTotalWhitespace = (2 * hInset) + (hSpacing * (columnCount - 1))
let hRemainderSpace = totalViewWidth - hTotalWhitespace
let columnWidth = UInt(hRemainderSpace / columnCount)
// We want to unevenly distribute the hSpacing between the columns
// so that the left and right margins are equal, which is non-trivial
// due to rounding error.
let totalHSpacing = totalViewWidth - ((2 * hMargin) + (columnCount * columnWidth))
let totalHSpacing = totalViewWidth - ((2 * hInset) + (columnCount * columnWidth))
// columnXs are the left edge of each column.
var columnXs = [UInt]()
// columnYs are the top edge of the next cell in each column.
var columnYs = [UInt]()
for columnIndex in 0...columnCount-1 {
var columnX = hMargin + (columnWidth * columnIndex)
var columnX = hInset + (columnWidth * columnIndex)
if columnCount > 1 {
// We want to unevenly distribute the hSpacing between the columns
// so that the left and right margins are equal, which is non-trivial
@ -86,12 +88,12 @@ class GifPickerLayout: UICollectionViewLayout {
columnX += ((totalHSpacing * columnIndex) / (columnCount - 1))
}
columnXs.append(columnX)
columnYs.append(vMargin)
columnYs.append(vInset)
}
// Always layout all items.
let imageInfos = delegate.imageInfosForLayout()
var contentBottom = vMargin
var contentBottom = vInset
for (cellIndex, imageInfo) in imageInfos.enumerated() {
// Select a column by finding the "highest, leftmost" column.
var column = 0
@ -117,18 +119,14 @@ class GifPickerLayout: UICollectionViewLayout {
}
// Add bottom margin.
let contentHeight = contentBottom + vMargin
let contentHeight = contentBottom + vInset
contentSize = CGSize(width:CGFloat(totalViewWidth), height:CGFloat(contentHeight))
}
override func layoutAttributesForElements(in rect: CGRect) -> [UICollectionViewLayoutAttributes]? {
var result = [UICollectionViewLayoutAttributes]()
for itemAttributes in itemAttributesMap.values {
if itemAttributes.frame.intersects(rect) {
result.append(itemAttributes)
}
return itemAttributesMap.values.filter { itemAttributes in
return itemAttributes.frame.intersects(rect)
}
return result
}
override func layoutAttributesForItem(at indexPath: IndexPath) -> UICollectionViewLayoutAttributes? {

View File

@ -98,7 +98,7 @@ class GifPickerViewController: OWSViewController, UISearchBarDelegate, UICollect
view.backgroundColor = UIColor.black
self.navigationItem.leftBarButtonItem = UIBarButtonItem(barButtonSystemItem:.stop,
self.navigationItem.leftBarButtonItem = UIBarButtonItem(barButtonSystemItem:.cancel,
target:self,
action:#selector(donePressed))
self.navigationItem.title = NSLocalizedString("GIF_PICKER_VIEW_TITLE",
@ -134,10 +134,12 @@ class GifPickerViewController: OWSViewController, UISearchBarDelegate, UICollect
searchBar.delegate = self
searchBar.placeholder = NSLocalizedString("GIF_VIEW_SEARCH_PLACEHOLDER_TEXT",
comment:"Placeholder text for the search field in gif view")
// Style the search bar so that it looks appropriate against a black background.
searchBar.isTranslucent = false
searchBar.backgroundImage = UIImage(color:UIColor.clear)
searchBar.barTintColor = UIColor.black
searchBar.tintColor = UIColor.white
self.view.addSubview(searchBar)
searchBar.autoPinWidthToSuperview()
searchBar.autoPin(toTopLayoutGuideOf: self, withInset:0)
@ -204,7 +206,7 @@ class GifPickerViewController: OWSViewController, UISearchBarDelegate, UICollect
owsFail("\(TAG) unexpected cell.")
return
}
guard let asset = cell.fullAsset else {
guard let asset = cell.animatedAsset else {
Logger.info("\(TAG) unload cell selected.")
return
}
@ -213,7 +215,7 @@ class GifPickerViewController: OWSViewController, UISearchBarDelegate, UICollect
owsFail("\(TAG) couldn't load asset.")
return
}
let attachment = SignalAttachment(dataSource : dataSource, dataUTI: asset.rendition.utiType())
let attachment = SignalAttachment(dataSource : dataSource, dataUTI: asset.rendition.utiType)
guard let thread = thread else {
owsFail("\(TAG) Missing thread.")
return
@ -284,9 +286,10 @@ class GifPickerViewController: OWSViewController, UISearchBarDelegate, UICollect
strongSelf.imageInfos = imageInfos
strongSelf.updateContents()
},
failure: { [weak self] in
failure: { [weak self] _ in
guard let strongSelf = self else { return }
Logger.info("\(strongSelf.TAG) search failed.")
// TODO: Present this error to the user.
})
}

View File

@ -3,7 +3,6 @@
//
import Foundation
import ObjectiveC
// There's no UTI type for webp!
enum GiphyFormat {
@ -37,7 +36,7 @@ enum GiphyFormat {
self.url = url
}
public func fileExtension() -> String {
public var fileExtension: String {
switch format {
case .gif:
return "gif"
@ -48,7 +47,7 @@ enum GiphyFormat {
}
}
public func utiType() -> String {
public var utiType: String {
switch format {
case .gif:
return kUTTypeGIF as String
@ -59,6 +58,14 @@ enum GiphyFormat {
}
}
public var isStill: Bool {
return name.hasSuffix("_still")
}
public var isDownsampled: Bool {
return name.hasSuffix("_downsampled")
}
public func log() {
Logger.verbose("\t \(format), \(name), \(width), \(height), \(fileSize)")
}
@ -101,7 +108,7 @@ enum GiphyFormat {
return pickRendition(isStill:true, pickingStrategy:.smallerIsBetter, maxFileSize:kMaxFileSize)
}
public func pickGifRendition() -> GiphyRendition? {
public func pickAnimatedRendition() -> GiphyRendition? {
// Try to pick a small file...
if let rendition = pickRendition(isStill:false, pickingStrategy:.largerIsBetter, maxFileSize:kMaxFileSize) {
return rendition
@ -129,10 +136,12 @@ enum GiphyFormat {
continue
}
// Only consider still renditions.
guard rendition.name.hasSuffix("_still") else {
guard rendition.isStill else {
continue
}
// Accept renditions without a valid file size.
// Accept still renditions without a valid file size. Note that fileSize
// will be zero for renditions without a valid file size, so they will pass
// the maxFileSize test.
//
// Don't worry about max content size; still images are tiny in comparison
// with animated renditions.
@ -148,11 +157,11 @@ enum GiphyFormat {
continue
}
// Ignore stills.
guard !rendition.name.hasSuffix("_still") else {
guard !rendition.isStill else {
continue
}
// Ignore "downsampled" renditions which skip frames, etc.
guard !rendition.name.hasSuffix("_downsampled") else {
guard !rendition.isDownsampled else {
continue
}
guard rendition.width >= kMinDimension &&
@ -200,6 +209,7 @@ enum GiphyFormat {
// MARK: - Properties
static let TAG = "[GiphyAPI]"
let TAG = "[GiphyAPI]"
static let sharedInstance = GiphyAPI()
@ -214,7 +224,7 @@ enum GiphyFormat {
private func giphyAPISessionManager() -> AFHTTPSessionManager? {
guard let baseUrl = NSURL(string:kGiphyBaseURL) else {
Logger.error("\(GiphyAPI.TAG) Invalid base URL.")
Logger.error("\(TAG) Invalid base URL.")
return nil
}
// TODO: We need to verify that this session configuration properly
@ -236,15 +246,15 @@ enum GiphyFormat {
// MARK: Search
public func search(query: String, success: @escaping (([GiphyImageInfo]) -> Void), failure: @escaping (() -> Void)) {
public func search(query: String, success: @escaping (([GiphyImageInfo]) -> Void), failure: @escaping ((NSError?) -> Void)) {
guard let sessionManager = giphyAPISessionManager() else {
Logger.error("\(GiphyAPI.TAG) Couldn't create session manager.")
failure()
Logger.error("\(TAG) Couldn't create session manager.")
failure(nil)
return
}
guard NSURL(string:kGiphyBaseURL) != nil else {
Logger.error("\(GiphyAPI.TAG) Invalid base URL.")
failure()
Logger.error("\(TAG) Invalid base URL.")
failure(nil)
return
}
@ -255,8 +265,8 @@ enum GiphyFormat {
let kGiphyPageSize = 200
let kGiphyPageOffset = 0
guard let queryEncoded = query.addingPercentEncoding(withAllowedCharacters: .urlPathAllowed) else {
Logger.error("\(GiphyAPI.TAG) Could not URL encode query: \(query).")
failure()
Logger.error("\(TAG) Could not URL encode query: \(query).")
failure(nil)
return
}
let urlString = "/v1/gifs/search?api_key=\(kGiphyApiKey)&offset=\(kGiphyPageOffset)&limit=\(kGiphyPageSize)&q=\(queryEncoded)"
@ -267,14 +277,14 @@ enum GiphyFormat {
success: { _, value in
Logger.error("\(GiphyAPI.TAG) search request succeeded")
guard let imageInfos = self.parseGiphyImages(responseJson:value) else {
failure()
failure(nil)
return
}
success(imageInfos)
},
failure: { _, error in
Logger.error("\(GiphyAPI.TAG) search request failed: \(error)")
failure()
failure(error as NSError)
})
}
@ -282,45 +292,40 @@ enum GiphyFormat {
private func parseGiphyImages(responseJson:Any?) -> [GiphyImageInfo]? {
guard let responseJson = responseJson else {
Logger.error("\(GiphyAPI.TAG) Missing response.")
Logger.error("\(TAG) Missing response.")
return nil
}
guard let responseDict = responseJson as? [String:Any] else {
Logger.error("\(GiphyAPI.TAG) Invalid response.")
Logger.error("\(TAG) Invalid response.")
return nil
}
guard let imageDicts = responseDict["data"] as? [[String:Any]] else {
Logger.error("\(GiphyAPI.TAG) Invalid response data.")
Logger.error("\(TAG) Invalid response data.")
return nil
}
var result = [GiphyImageInfo]()
for imageDict in imageDicts {
guard let imageInfo = parseGiphyImage(imageDict:imageDict) else {
continue
}
result.append(imageInfo)
return imageDicts.flatMap { imageDict in
return parseGiphyImage(imageDict: imageDict)
}
return result
}
// Giphy API results are often incomplete or malformed, so we need to be defensive.
private func parseGiphyImage(imageDict: [String:Any]) -> GiphyImageInfo? {
guard let giphyId = imageDict["id"] as? String else {
Logger.warn("\(GiphyAPI.TAG) Image dict missing id.")
Logger.warn("\(TAG) Image dict missing id.")
return nil
}
guard giphyId.characters.count > 0 else {
Logger.warn("\(GiphyAPI.TAG) Image dict has invalid id.")
Logger.warn("\(TAG) Image dict has invalid id.")
return nil
}
guard let renditionDicts = imageDict["images"] as? [String:Any] else {
Logger.warn("\(GiphyAPI.TAG) Image dict missing renditions.")
Logger.warn("\(TAG) Image dict missing renditions.")
return nil
}
var renditions = [GiphyRendition]()
for (renditionName, renditionDict) in renditionDicts {
guard let renditionDict = renditionDict as? [String:Any] else {
Logger.warn("\(GiphyAPI.TAG) Invalid rendition dict.")
Logger.warn("\(TAG) Invalid rendition dict.")
continue
}
guard let rendition = parseGiphyRendition(renditionName:renditionName,
@ -330,12 +335,12 @@ enum GiphyFormat {
renditions.append(rendition)
}
guard renditions.count > 0 else {
Logger.warn("\(GiphyAPI.TAG) Image has no valid renditions.")
Logger.warn("\(TAG) Image has no valid renditions.")
return nil
}
guard let originalRendition = findOriginalRendition(renditions:renditions) else {
Logger.warn("\(GiphyAPI.TAG) Image has no original rendition.")
Logger.warn("\(TAG) Image has no original rendition.")
return nil
}
@ -368,28 +373,28 @@ enum GiphyFormat {
return nil
}
guard urlString.characters.count > 0 else {
Logger.warn("\(GiphyAPI.TAG) Rendition has invalid url.")
Logger.warn("\(TAG) Rendition has invalid url.")
return nil
}
guard let url = NSURL(string:urlString) else {
Logger.warn("\(GiphyAPI.TAG) Rendition url could not be parsed.")
Logger.warn("\(TAG) Rendition url could not be parsed.")
return nil
}
guard let fileExtension = url.pathExtension else {
Logger.warn("\(GiphyAPI.TAG) Rendition url missing file extension.")
guard let fileExtension = url.pathExtension?.lowercased() else {
Logger.warn("\(TAG) Rendition url missing file extension.")
return nil
}
var format = GiphyFormat.gif
if fileExtension.lowercased() == "gif" {
if fileExtension == "gif" {
format = .gif
} else if fileExtension.lowercased() == "jpg" {
} else if fileExtension == "jpg" {
format = .jpg
} else if fileExtension.lowercased() == "mp4" {
} else if fileExtension == "mp4" {
format = .mp4
} else if fileExtension.lowercased() == "webp" {
} else if fileExtension == "webp" {
return nil
} else {
Logger.warn("\(GiphyAPI.TAG) Invalid file extension: \(fileExtension).")
Logger.warn("\(TAG) Invalid file extension: \(fileExtension).")
return nil
}
@ -414,7 +419,7 @@ enum GiphyFormat {
return nil
}
guard parsedValue > 0 else {
Logger.verbose("\(GiphyAPI.TAG) \(typeName) has non-positive \(key): \(parsedValue).")
Logger.verbose("\(TAG) \(typeName) has non-positive \(key): \(parsedValue).")
return nil
}
return parsedValue

View File

@ -378,7 +378,7 @@ extension URLSessionTask {
// We write assets to the temporary directory so that iOS can clean them up.
// We try to eagerly clean up these assets when they are no longer in use.
let dirPath = NSTemporaryDirectory()
let fileExtension = assetRequest.rendition.fileExtension()
let fileExtension = assetRequest.rendition.fileExtension
let fileName = (NSUUID().uuidString as NSString).appendingPathExtension(fileExtension)!
let filePath = (dirPath as NSString).appendingPathComponent(fileName)

View File

@ -605,7 +605,7 @@
"GIF_PICKER_VIEW_MISSING_QUERY" = "Please enter your search.";
/* Title for the 'gif picker' dialog. */
"GIF_PICKER_VIEW_TITLE" = "Gif Search";
"GIF_PICKER_VIEW_TITLE" = "GIF Search";
/* Placeholder text for the search field in gif view */
"GIF_VIEW_SEARCH_PLACEHOLDER_TEXT" = "Enter your search";
@ -1305,7 +1305,7 @@
"SECURE_SESSION_RESET" = "Secure session was reset.";
/* Label for 'select gif to attach' action sheet button */
"SELECT_GIF_BUTTON" = "Gif";
"SELECT_GIF_BUTTON" = "GIF";
/* No comment provided by engineer. */
"SEND_AGAIN_BUTTON" = "Send Again";