Mhurd has submitted this change and it was merged. Change subject: Fix iOS 6 image gallery rotation ......................................................................
Fix iOS 6 image gallery rotation Also: - modify project settings to cause compiler errors when declared methods aren't defined - fixed instances of above warning Bug: T90752 Change-Id: I24357d7f1636bda191e183489717cfcb8078839e --- M Wikipedia.xcodeproj/project.pbxproj A wikipedia/Categories/UICollectionViewFlowLayout+AttributeUtils.h A wikipedia/Categories/UICollectionViewFlowLayout+AttributeUtils.m M wikipedia/EventLogging/EventLoggingFunnel.m M wikipedia/View Controllers/Image Gallery/WMFImageGalleryCollectionViewCell.m M wikipedia/View Controllers/Image Gallery/WMFImageGalleryViewController.m 6 files changed, 154 insertions(+), 59 deletions(-) Approvals: Fjalapeno: Looks good to me, but someone else must approve Mhurd: Verified; Looks good to me, approved diff --git a/Wikipedia.xcodeproj/project.pbxproj b/Wikipedia.xcodeproj/project.pbxproj index 46404cf..087b206 100644 --- a/Wikipedia.xcodeproj/project.pbxproj +++ b/Wikipedia.xcodeproj/project.pbxproj @@ -294,6 +294,8 @@ C42D94861A937DE000A4871A /* WMFBorderButton.m in Sources */ = {isa = PBXBuildFile; fileRef = C42D94831A937DE000A4871A /* WMFBorderButton.m */; }; C42D94871A937DE000A4871A /* WMFProgressLineView.m in Sources */ = {isa = PBXBuildFile; fileRef = C42D94851A937DE000A4871A /* WMFProgressLineView.m */; }; C42D94BC1A95405000A4871A /* NSArray+WMFExtensions.m in Sources */ = {isa = PBXBuildFile; fileRef = C42D94BB1A95405000A4871A /* NSArray+WMFExtensions.m */; }; + BCC185E81A9FA498005378F8 /* UICollectionViewFlowLayout+AttributeUtils.m in Sources */ = {isa = PBXBuildFile; fileRef = BCC185E71A9FA498005378F8 /* UICollectionViewFlowLayout+AttributeUtils.m */; }; + BCC185E91A9FA498005378F8 /* UICollectionViewFlowLayout+AttributeUtils.m in Sources */ = {isa = PBXBuildFile; fileRef = BCC185E71A9FA498005378F8 /* UICollectionViewFlowLayout+AttributeUtils.m */; }; C46FBA4B1A8530EE00C5730F /* Pods-acknowledgements.plist in Resources */ = {isa = PBXBuildFile; fileRef = C46FBA4A1A8530EE00C5730F /* Pods-acknowledgements.plist */; }; C90799BA1A8564C60044E13C /* WMFShareOptionsViewController.m in Sources */ = {isa = PBXBuildFile; fileRef = C90799B91A8564C60044E13C /* WMFShareOptionsViewController.m */; }; C913C89C1A94019A00BEEAF0 /* WMFSuggestedPagesFunnel.m in Sources */ = {isa = PBXBuildFile; fileRef = C913C89B1A94019A00BEEAF0 /* WMFSuggestedPagesFunnel.m */; }; @@ -813,6 +815,8 @@ C42D94851A937DE000A4871A /* WMFProgressLineView.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = WMFProgressLineView.m; sourceTree = "<group>"; }; C42D94BA1A95405000A4871A /* NSArray+WMFExtensions.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = "NSArray+WMFExtensions.h"; path = "Wikipedia/Categories/NSArray+WMFExtensions.h"; sourceTree = SOURCE_ROOT; }; C42D94BB1A95405000A4871A /* NSArray+WMFExtensions.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = "NSArray+WMFExtensions.m"; path = "Wikipedia/Categories/NSArray+WMFExtensions.m"; sourceTree = SOURCE_ROOT; }; + BCC185E61A9FA498005378F8 /* UICollectionViewFlowLayout+AttributeUtils.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "UICollectionViewFlowLayout+AttributeUtils.h"; sourceTree = "<group>"; }; + BCC185E71A9FA498005378F8 /* UICollectionViewFlowLayout+AttributeUtils.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = "UICollectionViewFlowLayout+AttributeUtils.m"; sourceTree = "<group>"; }; C46FBA4A1A8530EE00C5730F /* Pods-acknowledgements.plist */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.plist.xml; name = "Pods-acknowledgements.plist"; path = "../../../Pods/Target Support Files/Pods/Pods-acknowledgements.plist"; sourceTree = "<group>"; }; C90799B81A8564C60044E13C /* WMFShareOptionsViewController.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = WMFShareOptionsViewController.h; path = ShareCard/WMFShareOptionsViewController.h; sourceTree = "<group>"; }; C90799B91A8564C60044E13C /* WMFShareOptionsViewController.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = WMFShareOptionsViewController.m; path = ShareCard/WMFShareOptionsViewController.m; sourceTree = "<group>"; }; @@ -1772,6 +1776,8 @@ BC86B93C1A929CC500B4C039 /* UICollectionViewFlowLayout+NSCopying.m */, BC86B93E1A929D7900B4C039 /* UICollectionViewFlowLayout+WMFItemSizeThatFits.h */, BC86B93F1A929D7900B4C039 /* UICollectionViewFlowLayout+WMFItemSizeThatFits.m */, + BCC185E61A9FA498005378F8 /* UICollectionViewFlowLayout+AttributeUtils.h */, + BCC185E71A9FA498005378F8 /* UICollectionViewFlowLayout+AttributeUtils.m */, ); path = Categories; sourceTree = "<group>"; @@ -2772,6 +2778,7 @@ BCB66A0D1A85183000C7B1FE /* NSString+WMFHTMLParsing.m in Sources */, BCB669EE1A83F71C00C7B1FE /* MWKProtectionStatus.m in Sources */, BCB669F41A83F71C00C7B1FE /* MWKRecentSearchList.m in Sources */, + BCC185E91A9FA498005378F8 /* UICollectionViewFlowLayout+AttributeUtils.m in Sources */, 04BA48A21A80062F00CB5CAE /* UIFont+WMFStyle.m in Sources */, BCB669741A83F58600C7B1FE /* NSMutableDictionary+WMFMaybeSet.m in Sources */, 042BEAF01A92EE66002CF320 /* UIWebView+WMFTrackingView.m in Sources */, @@ -2921,6 +2928,7 @@ BC50C37F1A83C784006DC7AF /* WMFNetworkUtilities.m in Sources */, BCB58F441A890D9700465627 /* MWKImageInfo+MWKImageComparison.m in Sources */, BCB669AD1A83F6C400C7B1FE /* MWKSavedPageList.m in Sources */, + BCC185E81A9FA498005378F8 /* UICollectionViewFlowLayout+AttributeUtils.m in Sources */, 04B91AB718E4D5B200FFAA1C /* TabularScrollView.m in Sources */, 04C695D218ED213000D9F2DA /* UIScrollView+NoHorizontalScrolling.m in Sources */, 04E106381A3560A90046DC27 /* LeadImageTitleLabel.m in Sources */, @@ -3355,6 +3363,7 @@ "-Werror=switch", "-Werror=arc-retain-cycles", "-Werror=incompatible-pointer-types", + "-Werror=incomplete-implementation", ); WRAPPER_EXTENSION = app; }; @@ -3388,6 +3397,7 @@ "-Werror=switch", "-Werror=arc-retain-cycles", "-Werror=incompatible-pointer-types", + "-Werror=incomplete-implementation", ); WRAPPER_EXTENSION = app; }; diff --git a/wikipedia/Categories/UICollectionViewFlowLayout+AttributeUtils.h b/wikipedia/Categories/UICollectionViewFlowLayout+AttributeUtils.h new file mode 100644 index 0000000..b051322 --- /dev/null +++ b/wikipedia/Categories/UICollectionViewFlowLayout+AttributeUtils.h @@ -0,0 +1,43 @@ +// +// UICollectionViewFlowLayout+AttributeUtils.h +// Wikipedia +// +// Created by Brian Gerstle on 2/26/15. +// Copyright (c) 2015 Wikimedia Foundation. All rights reserved. +// + +#import <UIKit/UIKit.h> + +/** + * Utilities for getting layout attributes and index paths for items in a flow layout. + * @warning Most of these methods were naively implemented and only support layouts configured with a horizontal + * scrolling direction. + */ +@interface UICollectionViewFlowLayout (AttributeUtils) + +/// The @c NSIndexPath for the item closest to the current @c contentOffset of the receiver's @c collectionView. +- (NSIndexPath*)wmf_indexPathClosestToContentOffset; + +/** + * Sort an array of @c UICollectionViewLayoutAttribute objects by their distance to @c point in ascending order. + * @param attributes The @c UICollectionViewLayoutAttributes objects to sort. + * @param point A point within the @c contentSize of the receiver's @c collectionView. + * @return A sorted version of the given @c attributes array. + */ +- (NSArray*)wmf_sortAttributesByLeadingEdgeDistance:(NSArray*)attributes toPoint:(CGPoint)point; + +/** + * Get an array of the currently visible items' attributes, sorted by distance to @c point in ascending order. + * @param point A point within the @c contentSize of the receiver's @c collectionView. + * @return An array of @c UICollectionViewLayoutAttributes. + * @see -wmf_sortAttributesByLeadingEdgeDistance:toPoint + */ +- (NSArray*)wmf_layoutAttributesByDistanceToPoint:(CGPoint)point; + +/** + * Map the receiver's <code>collectionView.indexPathsForVisibleItems</code> into an array of their layout attributes. + * @return An array of @c UICollectionViewLayoutAttributes. + */ +- (NSArray*)wmf_layoutAttributesForVisibleIndexPaths; + +@end diff --git a/wikipedia/Categories/UICollectionViewFlowLayout+AttributeUtils.m b/wikipedia/Categories/UICollectionViewFlowLayout+AttributeUtils.m new file mode 100644 index 0000000..20d1d05 --- /dev/null +++ b/wikipedia/Categories/UICollectionViewFlowLayout+AttributeUtils.m @@ -0,0 +1,41 @@ +// +// UICollectionViewFlowLayout+AttributeUtils.m +// Wikipedia +// +// Created by Brian Gerstle on 2/26/15. +// Copyright (c) 2015 Wikimedia Foundation. All rights reserved. +// + +#import "UICollectionViewFlowLayout+AttributeUtils.h" +#import <BlocksKit/BlocksKit.h> + +@implementation UICollectionViewFlowLayout (AttributeUtils) + +- (NSIndexPath*)wmf_indexPathClosestToContentOffset +{ + return [[[self wmf_layoutAttributesByDistanceToPoint:self.collectionView.contentOffset] firstObject] indexPath]; +} + +- (NSArray*)wmf_sortAttributesByLeadingEdgeDistance:(NSArray*)attributes toPoint:(CGPoint)point +{ + return [attributes sortedArrayUsingComparator:^NSComparisonResult(UICollectionViewLayoutAttributes* attr1, + UICollectionViewLayoutAttributes* attr2) { + float leadingEdgeDistance1 = fabsf(point.x - attr1.frame.origin.x); + float leadingEdgeDistance2 = fabsf(point.x - attr2.frame.origin.x); + return leadingEdgeDistance1 - leadingEdgeDistance2; + }]; +} + +- (NSArray*)wmf_layoutAttributesByDistanceToPoint:(CGPoint)point +{ + return [self wmf_sortAttributesByLeadingEdgeDistance:[self wmf_layoutAttributesForVisibleIndexPaths] toPoint:point]; +} + +- (NSArray*)wmf_layoutAttributesForVisibleIndexPaths +{ + return [self.collectionView.indexPathsForVisibleItems bk_map:^(NSIndexPath* path) { + return [self layoutAttributesForItemAtIndexPath:path]; + }]; +} + +@end diff --git a/wikipedia/EventLogging/EventLoggingFunnel.m b/wikipedia/EventLogging/EventLoggingFunnel.m index c20e511..cb0325d 100644 --- a/wikipedia/EventLogging/EventLoggingFunnel.m +++ b/wikipedia/EventLogging/EventLoggingFunnel.m @@ -31,10 +31,10 @@ { SessionSingleton *session = [SessionSingleton sharedInstance]; NSString *wiki = [session.site.language stringByAppendingString:@"wiki"]; - [self log:eventData forWiki:wiki]; + [self log:eventData wiki:wiki]; } --(void)log:(NSDictionary *)eventData forWiki:(NSString *)wiki +-(void)log:(NSDictionary *)eventData wiki:(NSString *)wiki { if ([SessionSingleton sharedInstance].sendUsageReports) { (void)[[EventLogger alloc] initAndLogEvent:[self preprocessData:eventData] diff --git a/wikipedia/View Controllers/Image Gallery/WMFImageGalleryCollectionViewCell.m b/wikipedia/View Controllers/Image Gallery/WMFImageGalleryCollectionViewCell.m index 3eae7e7..b2a3227 100644 --- a/wikipedia/View Controllers/Image Gallery/WMFImageGalleryCollectionViewCell.m +++ b/wikipedia/View Controllers/Image Gallery/WMFImageGalleryCollectionViewCell.m @@ -21,12 +21,11 @@ { self = [super initWithFrame:frame]; if (self) { - self.contentView.backgroundColor = [UIColor blackColor]; - self.backgroundView.backgroundColor = [UIColor blackColor]; - self.backgroundColor = [UIColor blackColor]; + self.backgroundView.backgroundColor = [UIColor clearColor]; // image view's frame fills the scroll view's *bounds*, and zooming transforms it to the natural image size UIImageView *imageView = [[UIImageView alloc] initWithFrame:self.contentView.bounds]; + imageView.backgroundColor = [UIColor clearColor]; imageView.contentMode = UIViewContentModeScaleAspectFit; _imageView = imageView; diff --git a/wikipedia/View Controllers/Image Gallery/WMFImageGalleryViewController.m b/wikipedia/View Controllers/Image Gallery/WMFImageGalleryViewController.m index 2475517..e3b0930 100644 --- a/wikipedia/View Controllers/Image Gallery/WMFImageGalleryViewController.m +++ b/wikipedia/View Controllers/Image Gallery/WMFImageGalleryViewController.m @@ -22,6 +22,7 @@ #import "UICollectionViewFlowLayout+NSCopying.h" #import "UICollectionViewFlowLayout+WMFItemSizeThatFits.h" #import "UIViewController+Alert.h" +#import "UICollectionViewFlowLayout+AttributeUtils.h" // Model #import "MWKDataStore.h" @@ -42,7 +43,7 @@ #endif @interface WMFImageGalleryViewController () -<UIGestureRecognizerDelegate> +<UIGestureRecognizerDelegate, UICollectionViewDelegateFlowLayout> { MWKImageInfoFetcher *_imageInfoFetcher; AFHTTPRequestOperationManager *_imageFetcher; @@ -50,10 +51,10 @@ NSArray *_uniqueArticleImages; } -@property (nonatomic) BOOL didSetInitialItemSize; -@property (nonatomic) BOOL didPerformInitialLayout; @property (nonatomic) BOOL didApplyInitialVisibleImageIndex; +@property (nonatomic) NSUInteger preRotationVisibleImageIndex; +@property (nonatomic, weak, readonly) UICollectionViewFlowLayout *collectionViewFlowLayout; @property (nonatomic, weak, readonly) UIButton *closeButton; @property (nonatomic, getter=isChromeHidden) BOOL chromeHidden; @property (nonatomic, weak, readonly) UITapGestureRecognizer *chromeTapGestureRecognizer; @@ -184,6 +185,11 @@ return nil; } +- (NSUInteger)mostVisibleItemIndex +{ + return [self.collectionViewFlowLayout wmf_indexPathClosestToContentOffset].item; +} + #pragma mark - Networking & Persistence - (void)fetchImageInfo @@ -223,62 +229,53 @@ duration:(NSTimeInterval)duration { [super willRotateToInterfaceOrientation:toInterfaceOrientation duration:duration]; - if (UIInterfaceOrientationIsLandscape(self.interfaceOrientation) - == UIInterfaceOrientationIsLandscape(toInterfaceOrientation)) { - return; - } - - // need to capture visibleImageIndex before the animation, else collectionViewDelegate methods will throw it off - NSUInteger currentImageIndex = self.visibleImageIndex; - CGSize currentSize = self.view.bounds.size; - CGSize newBounds = CGSizeMake(currentSize.height, currentSize.width); + NSUInteger const currentImageIndex = [self mostVisibleItemIndex]; + ImgGalleryLog(@"Will scroll to %u after rotation animation finishes.", currentImageIndex); [UIView animateWithDuration:duration delay:0 - options:UIViewAnimationOptionAllowAnimatedContent | UIViewAnimationOptionBeginFromCurrentState + options:UIViewAnimationOptionBeginFromCurrentState | UIViewAnimationOptionAllowAnimatedContent animations:^{ - self.collectionViewFlowLayout.itemSize = [self.collectionViewFlowLayout wmf_itemSizeThatFits:newBounds]; - } - completion:^ (BOOL finished) { - self.visibleImageIndex = currentImageIndex; - }]; -} - -- (void)viewWillAppear:(BOOL)animated -{ - [super viewWillAppear:animated]; - // set this before, otherwise it won't be true inside other calls like viewDidLayoutSubviews - _didSetInitialItemSize = YES; - [self.collectionViewFlowLayout wmf_strictItemSizeToFit]; - [self applyChromeHidden:animated]; -} - -- (void)viewDidAppear:(BOOL)animated -{ - [super viewDidAppear:animated]; - // assert that all the expected state transitions happened - NSParameterAssert(_didSetInitialItemSize); - NSParameterAssert(_didPerformInitialLayout); - NSParameterAssert(_didApplyInitialVisibleImageIndex); + [self.collectionViewFlowLayout invalidateLayout]; + } + completion:^(BOOL finished) { + [self setVisibleImageIndex:currentImageIndex animated:NO forceViewUpdate:YES]; + }]; } - (void)viewDidLayoutSubviews { [super viewDidLayoutSubviews]; - // reset item size once the collection view has been laid out - if (_didSetInitialItemSize && !_didPerformInitialLayout) { - _didPerformInitialLayout = YES; + /* + only apply visible image index once the collection view has been populated with cells, otherwise calls to get + layout attributes of the item at `visibleImageIndex` will return `nil` (on iOS 6, at least) + */ + if (!self.didApplyInitialVisibleImageIndex && self.collectionView.visibleCells.count) { [self applyVisibleImageIndex:NO]; // only set the flag *after* the visible index has been updated, to make sure UICollectionViewDelegate // callbacks don't override it - _didApplyInitialVisibleImageIndex = YES; + self.didApplyInitialVisibleImageIndex = YES; } +} + +//- (void)viewWillAppear:(BOOL)animated +//{ +// [super viewWillAppear:animated]; +//} + +- (void)viewDidAppear:(BOOL)animated +{ + [super viewDidAppear:animated]; + // fetch after appearing so we don't do work while the animation is rendering + [self fetchImageInfo]; } - (void)viewDidLoad { [super viewDidLoad]; + // this prevents white lines from appearing during rotation animations on iOS 6 + self.view.backgroundColor = [UIColor blackColor]; // manually layout closeButton so we can programmatically increase it's hit size NSString* closeButtonTitle = WIKIGLYPH_X; @@ -318,8 +315,6 @@ forCellWithReuseIdentifier:WMFImageGalleryCollectionViewCellReuseId]; self.collectionView.pagingEnabled = YES; - - [self fetchImageInfo]; } #pragma mark - Chrome @@ -377,15 +372,25 @@ - (void)applyVisibleImageIndex:(BOOL)animated { if ([self isViewLoaded]) { - [self.collectionView scrollToItemAtIndexPath:[NSIndexPath indexPathForItem:self.visibleImageIndex inSection:0] - atScrollPosition:UICollectionViewScrollPositionLeft - animated:animated]; + // can't use scrollToItem because it doesn't handle post-rotation scrolling well on iOS 6 + UICollectionViewLayoutAttributes* visibleImageAttributes = + [self.collectionViewFlowLayout layoutAttributesForItemAtIndexPath: + [NSIndexPath indexPathForItem:self.visibleImageIndex inSection:0]]; + NSAssert(visibleImageAttributes, + @"Layout attributes for visible image were nil because %@ was called too early!", + NSStringFromSelector(_cmd)); + [self.collectionView setContentOffset:visibleImageAttributes.frame.origin animated:animated]; } } - (void)setVisibleImageIndex:(NSUInteger)visibleImageIndex animated:(BOOL)animated { - if (visibleImageIndex == _visibleImageIndex) { return; } + [self setVisibleImageIndex:visibleImageIndex animated:animated forceViewUpdate:NO]; +} + +- (void)setVisibleImageIndex:(NSUInteger)visibleImageIndex animated:(BOOL)animated forceViewUpdate:(BOOL)force +{ + if (!force && visibleImageIndex == _visibleImageIndex) { return; } NSParameterAssert(visibleImageIndex < self.uniqueArticleImages.count); _visibleImageIndex = visibleImageIndex; [self applyVisibleImageIndex:animated]; @@ -414,15 +419,11 @@ #pragma mark Delegate -- (void)collectionView:(UICollectionView *)collectionView - willDisplayCell:(UICollectionViewCell *)cell - forItemAtIndexPath:(NSIndexPath *)indexPath +- (CGSize)collectionView:(UICollectionView *)collectionView + layout:(UICollectionViewLayout *)collectionViewLayout + sizeForItemAtIndexPath:(NSIndexPath *)indexPath { - // only apply after initial visible index has been set - if (_didApplyInitialVisibleImageIndex) { - // silently update the visible image index (i.e. do *not* use the setter!) - _visibleImageIndex = indexPath.item; - } + return [self.collectionViewFlowLayout wmf_itemSizeThatFits:self.view.bounds.size]; } #pragma mark DataSource @@ -434,6 +435,7 @@ (WMFImageGalleryCollectionViewCell*) [collectionView dequeueReusableCellWithReuseIdentifier:WMFImageGalleryCollectionViewCellReuseId forIndexPath:indexPath]; + MWKImage *imageStub = self.uniqueArticleImages[indexPath.item]; MWKImageInfo *infoForImage = self.indexedImageInfo[imageStub.infoAssociationValue]; -- To view, visit https://gerrit.wikimedia.org/r/193292 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I24357d7f1636bda191e183489717cfcb8078839e Gerrit-PatchSet: 4 Gerrit-Project: apps/ios/wikipedia Gerrit-Branch: master Gerrit-Owner: Bgerstle <bgers...@wikimedia.org> Gerrit-Reviewer: Bgerstle <bgers...@wikimedia.org> Gerrit-Reviewer: Fjalapeno <cfl...@wikimedia.org> Gerrit-Reviewer: Mhurd <mh...@wikimedia.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits