From 63a0d0f106045785357f2cf7fc7a04bb00960b7b Mon Sep 17 00:00:00 2001 From: VladiMihaylenko Date: Thu, 17 Nov 2016 14:25:16 +0300 Subject: [PATCH] Review fixes --- iphone/Maps/Classes/MWMActionBarButton.mm | 4 +-- iphone/Maps/Classes/MWMBookmarkCell.mm | 4 +-- .../Maps/Classes/MWMEditBookmarkController.mm | 7 +++-- .../Classes/MWMOpeningHoursLayoutHelper.h | 1 - .../Classes/MWMOpeningHoursLayoutHelper.mm | 8 +++--- .../Maps/Classes/MWMPPPreviewLayoutHelper.h | 1 - .../Maps/Classes/MWMPPPreviewLayoutHelper.mm | 15 ++++++----- iphone/Maps/Classes/MWMPlacePageActionBar.mm | 7 +++-- .../Classes/MWMPlacePageCellUpdateProtocol.h | 2 +- iphone/Maps/Classes/MWMPlacePageData.mm | 2 ++ iphone/Maps/Classes/MWMPlacePageLayout.mm | 19 ++++++------- iphone/Maps/Classes/MWMPlacePageLayoutImpl.h | 2 +- iphone/Maps/Classes/MWMPlacePageManager.mm | 2 +- .../Classes/MWMiPadPlacePageLayoutImpl.mm | 21 +++++++-------- .../Classes/MWMiPhonePlacePageLayoutImpl.mm | 27 ++++++++----------- 15 files changed, 57 insertions(+), 65 deletions(-) diff --git a/iphone/Maps/Classes/MWMActionBarButton.mm b/iphone/Maps/Classes/MWMActionBarButton.mm index a474dc1253..f926d30b0e 100644 --- a/iphone/Maps/Classes/MWMActionBarButton.mm +++ b/iphone/Maps/Classes/MWMActionBarButton.mm @@ -1,7 +1,7 @@ #import "MWMActionBarButton.h" #import "Common.h" -#import "MWMCircularProgress.h" #import "MWMButton.h" +#import "MWMCircularProgress.h" #import "UIColor+MapsMeColor.h" NSString * titleForButton(EButton type, BOOL isSelected) @@ -173,7 +173,7 @@ NSString * titleForButton(EButton type, BOOL isSelected) { [super layoutSubviews]; self.frame = self.superview.bounds; - auto constexpr designOffset = 4; + CGFloat constexpr designOffset = 4; self.progressWrapper.size = {self.button.height - designOffset, self.button.height - designOffset}; self.progressWrapper.center = self.button.center; } diff --git a/iphone/Maps/Classes/MWMBookmarkCell.mm b/iphone/Maps/Classes/MWMBookmarkCell.mm index d7790ad309..eab86eb4d4 100644 --- a/iphone/Maps/Classes/MWMBookmarkCell.mm +++ b/iphone/Maps/Classes/MWMBookmarkCell.mm @@ -72,7 +72,7 @@ NSString * const kTextViewContentSizeKeyPath = @"contentSize"; [self stateOpen:NO]; [self setNeedsLayout]; - [self.updateCellDelegate updateCell]; + [self.updateCellDelegate cellUpdated]; return; } @@ -201,6 +201,6 @@ NSString * const kTextViewContentSizeKeyPath = @"contentSize"; [self setNeedsLayout]; } -- (IBAction)editTap { [self.editBookmarkDelegate editBookmark];} +- (IBAction)editTap { [self.editBookmarkDelegate editBookmark]; } @end diff --git a/iphone/Maps/Classes/MWMEditBookmarkController.mm b/iphone/Maps/Classes/MWMEditBookmarkController.mm index f5ec046b5e..85d24e99c1 100644 --- a/iphone/Maps/Classes/MWMEditBookmarkController.mm +++ b/iphone/Maps/Classes/MWMEditBookmarkController.mm @@ -51,7 +51,7 @@ enum RowInMetaInfo { [super viewDidLoad]; auto data = self.data; - NSAssert(data, @"Entity and data can't be nil both!"); + NSAssert(data, @"Data can't be nil!"); self.cachedDescription = data.bookmarkDescription; self.cachedTitle = data.externalTitle ? data.externalTitle : data.title; self.cachedCategory = data.bookmarkCategory; @@ -94,15 +94,14 @@ enum RowInMetaInfo - (void)onSave { [self.view endEditing:YES]; - Framework & f = GetFramework(); + auto & f = GetFramework(); BookmarkCategory * category = f.GetBmCategory(m_cachedBac.m_categoryIndex); if (!category) return; { BookmarkCategory::Guard guard(*category); - Bookmark * bookmark = - static_cast(guard.m_controller.GetUserMarkForEdit(m_cachedBac.m_bookmarkIndex)); + auto bookmark = static_cast(guard.m_controller.GetUserMarkForEdit(m_cachedBac.m_bookmarkIndex)); if (!bookmark) return; diff --git a/iphone/Maps/Classes/MWMOpeningHoursLayoutHelper.h b/iphone/Maps/Classes/MWMOpeningHoursLayoutHelper.h index 43ee2809c2..7da07515a6 100644 --- a/iphone/Maps/Classes/MWMOpeningHoursLayoutHelper.h +++ b/iphone/Maps/Classes/MWMOpeningHoursLayoutHelper.h @@ -3,7 +3,6 @@ @interface MWMOpeningHoursLayoutHelper : NSObject - (instancetype)initWithTableView:(UITableView *)tableView; -- (void)registerCells; - (void)configWithData:(MWMPlacePageData *)data; - (UITableViewCell *)cellForRowAtIndexPath:(NSIndexPath *)indexPath; diff --git a/iphone/Maps/Classes/MWMOpeningHoursLayoutHelper.mm b/iphone/Maps/Classes/MWMOpeningHoursLayoutHelper.mm index 400650b24d..814f6881ea 100644 --- a/iphone/Maps/Classes/MWMOpeningHoursLayoutHelper.mm +++ b/iphone/Maps/Classes/MWMOpeningHoursLayoutHelper.mm @@ -10,7 +10,7 @@ namespace { -array kCells = {{@"_MWMOHHeaderCell", @"_MWMOHSubCell"}}; +array const kCells = {{@"_MWMOHHeaderCell", @"_MWMOHSubCell"}}; NSAttributedString * richStringFromDay(osmoh::Day const & day, BOOL isClosedNow) { @@ -120,7 +120,10 @@ NSAttributedString * richStringFromDay(osmoh::Day const & day, BOOL isClosedNow) { self = [super init]; if (self) + { _tableView = tableView; + [self registerCells]; + } return self; } @@ -145,8 +148,7 @@ NSAttributedString * richStringFromDay(osmoh::Day const & day, BOOL isClosedNow) if (self.data.metainfoRows[indexPath.row] == place_page::MetainfoRows::OpeningHours) { - _MWMOHHeaderCell * cell = - [tableView dequeueReusableCellWithIdentifier:[_MWMOHHeaderCell className]]; + _MWMOHHeaderCell * cell = [tableView dequeueReusableCellWithIdentifier:[_MWMOHHeaderCell className]]; if (m_days.size() > 1) { diff --git a/iphone/Maps/Classes/MWMPPPreviewLayoutHelper.h b/iphone/Maps/Classes/MWMPPPreviewLayoutHelper.h index ca0a582a30..580237abf0 100644 --- a/iphone/Maps/Classes/MWMPPPreviewLayoutHelper.h +++ b/iphone/Maps/Classes/MWMPPPreviewLayoutHelper.h @@ -3,7 +3,6 @@ @interface MWMPPPreviewLayoutHelper : NSObject - (instancetype)initWithTableView:(UITableView *)tableView; -- (void)registerCells; - (UITableViewCell *)cellForRowAtIndexPath:(NSIndexPath *)indexPath withData:(MWMPlacePageData *)data; diff --git a/iphone/Maps/Classes/MWMPPPreviewLayoutHelper.mm b/iphone/Maps/Classes/MWMPPPreviewLayoutHelper.mm index 6fcc397cde..e2b821a724 100644 --- a/iphone/Maps/Classes/MWMPPPreviewLayoutHelper.mm +++ b/iphone/Maps/Classes/MWMPPPreviewLayoutHelper.mm @@ -9,7 +9,7 @@ namespace { -array kPreviewCells = {{@"_MWMPPPTitle", @"_MWMPPPExternalTitle", @"_MWMPPPSubtitle", +array const kPreviewCells = {{@"_MWMPPPTitle", @"_MWMPPPExternalTitle", @"_MWMPPPSubtitle", @"_MWMPPPSchedule", @"_MWMPPPBooking", @"_MWMPPPAddress", @"_MWMPPPSpace"}}; } // namespace @@ -30,7 +30,8 @@ array kPreviewCells = {{@"_MWMPPPTitle", @"_MWMPPPExternalTitle", - (void)layoutSubviews { [super layoutSubviews]; - self.separatorInset = {0, self.width / 2, 0, self.width / 2}; + auto const inset = self.width / 2; + self.separatorInset = {0, inset, 0, inset}; } - (IBAction)tap @@ -137,7 +138,10 @@ array kPreviewCells = {{@"_MWMPPPTitle", @"_MWMPPPExternalTitle", { self = [super init]; if (self) + { _tableView = tableView; + [self registerCells]; + } return self; } @@ -158,7 +162,7 @@ array kPreviewCells = {{@"_MWMPPPTitle", @"_MWMPPPExternalTitle", // -2 because last cell is always the spacer cell. BOOL const isNeedToShowDistance = (indexPath.row == data.previewRows.size() - 2); - UITableViewCell * c = [tableView dequeueReusableCellWithIdentifier:cellName]; + auto * c = [tableView dequeueReusableCellWithIdentifier:cellName]; switch(row) { case PreviewRows::Title: @@ -252,10 +256,7 @@ array kPreviewCells = {{@"_MWMPPPTitle", @"_MWMPPPExternalTitle", - (void)setDistanceToObject:(NSString *)distance { - if (!distance.length) - return; - - if ([self.distance isEqualToString:distance]) + if (!distance.length || [self.distance isEqualToString:distance]) return; self.distance = distance; diff --git a/iphone/Maps/Classes/MWMPlacePageActionBar.mm b/iphone/Maps/Classes/MWMPlacePageActionBar.mm index eb028e4dd5..5047972117 100644 --- a/iphone/Maps/Classes/MWMPlacePageActionBar.mm +++ b/iphone/Maps/Classes/MWMPlacePageActionBar.mm @@ -161,14 +161,12 @@ extern NSString * const kAlohalyticsTapEventKey; - (void)setDownloadingState:(MWMCircularProgressState)state { - MWMCircularProgress * p = self.progressFromActiveButton; - p.state = state; + self.progressFromActiveButton.state = state; } - (void)setDownloadingProgress:(CGFloat)progress { - MWMCircularProgress * p = self.progressFromActiveButton; - p.progress = progress; + self.progressFromActiveButton.progress = progress; } - (MWMCircularProgress *)progressFromActiveButton @@ -178,6 +176,7 @@ extern NSString * const kAlohalyticsTapEventKey; for (UIView * view in self.buttons) { + NSAssert(view.subviews.count, @"Subviews can't be empty!"); MWMActionBarButton * button = view.subviews[0]; if (button.type != EButton::Download) continue; diff --git a/iphone/Maps/Classes/MWMPlacePageCellUpdateProtocol.h b/iphone/Maps/Classes/MWMPlacePageCellUpdateProtocol.h index 716aff9d83..b051d67856 100644 --- a/iphone/Maps/Classes/MWMPlacePageCellUpdateProtocol.h +++ b/iphone/Maps/Classes/MWMPlacePageCellUpdateProtocol.h @@ -1,5 +1,5 @@ @protocol MWMPlacePageCellUpdateProtocol -- (void)updateCell; +- (void)cellUpdated; @end diff --git a/iphone/Maps/Classes/MWMPlacePageData.mm b/iphone/Maps/Classes/MWMPlacePageData.mm index cd1d0093a2..0b3942765b 100644 --- a/iphone/Maps/Classes/MWMPlacePageData.mm +++ b/iphone/Maps/Classes/MWMPlacePageData.mm @@ -71,6 +71,8 @@ using namespace place_page; if (self.schedule != OpeningHours::Unknown) m_previewRows.push_back(PreviewRows::Schedule); if (self.isBooking) m_previewRows.push_back(PreviewRows::Booking); if (self.address.length) m_previewRows.push_back(PreviewRows::Address); + + NSAssert(!m_previewRows.empty(), @"Preview row's can't be empty!"); m_previewRows.push_back(PreviewRows::Space); } diff --git a/iphone/Maps/Classes/MWMPlacePageLayout.mm b/iphone/Maps/Classes/MWMPlacePageLayout.mm index 684285015e..1369b11295 100644 --- a/iphone/Maps/Classes/MWMPlacePageLayout.mm +++ b/iphone/Maps/Classes/MWMPlacePageLayout.mm @@ -1,15 +1,15 @@ #import "MWMPlacePageLayout.h" +#import "MWMBookmarkCell.h" +#import "MWMCircularProgress.h" #import "MWMiPadPlacePageLayoutImpl.h" #import "MWMiPhonePlacePageLayoutImpl.h" -#import "MWMCircularProgress.h" +#import "MWMPlacePageButtonCell.h" #import "MWMPlacePageCellUpdateProtocol.h" #import "MWMPlacePageData.h" -#import "MWMPPPreviewLayoutHelper.h" -#import "MWMBookmarkCell.h" -#import "MWMOpeningHoursLayoutHelper.h" -#import "MWMPlacePageButtonCell.h" #import "MWMPlacePageInfoCell.h" #import "MWMPlacePageLayoutImpl.h" +#import "MWMOpeningHoursLayoutHelper.h" +#import "MWMPPPreviewLayoutHelper.h" #import "UIColor+MapsMeColor.h" #include "storage/storage.hpp" @@ -19,7 +19,7 @@ namespace { -array kBookmarkCells = {{@"MWMBookmarkCell"}}; +array const kBookmarkCells = {{@"MWMBookmarkCell"}}; using place_page::MetainfoRows; @@ -33,7 +33,7 @@ map const kMetaInfoCells = { {MetainfoRows::Coordinate, @"PlacePageInfoCell"}, {MetainfoRows::Internet, @"PlacePageInfoCell"}}; -array kButtonsCells = {{@"MWMPlacePageButtonCell"}}; +array const kButtonsCells = {{@"MWMPlacePageButtonCell"}}; } // namespace @@ -102,9 +102,6 @@ array kButtonsCells = {{@"MWMPlacePageButtonCell"}}; [tv registerNib:[UINib nibWithNibName:kBookmarkCells[0] bundle:nil] forCellReuseIdentifier:kBookmarkCells[0]]; - [self.previewLayoutHelper registerCells]; - [self.openingHoursLayoutHelper registerCells]; - // Register all meta info cells. for (auto const & pair : kMetaInfoCells) { @@ -362,7 +359,7 @@ array kButtonsCells = {{@"MWMPlacePageButtonCell"}}; #pragma mark - MWMPlacePageCellUpdateProtocol -- (void)updateCell +- (void)cellUpdated { auto const update = @selector(update); [NSObject cancelPreviousPerformRequestsWithTarget:self selector:update object:nil]; diff --git a/iphone/Maps/Classes/MWMPlacePageLayoutImpl.h b/iphone/Maps/Classes/MWMPlacePageLayoutImpl.h index db8cd57670..1d6e014215 100644 --- a/iphone/Maps/Classes/MWMPlacePageLayoutImpl.h +++ b/iphone/Maps/Classes/MWMPlacePageLayoutImpl.h @@ -18,7 +18,7 @@ inline void animate(TMWMVoidBlock animate, TMWMVoidBlock completion = nil) completion(); }]; } -} // namepsace place_page_layout +} // namepsace place_page_layout @protocol MWMPlacePageLayoutDelegate; diff --git a/iphone/Maps/Classes/MWMPlacePageManager.mm b/iphone/Maps/Classes/MWMPlacePageManager.mm index 9ae7f5ff0d..9d047a7361 100644 --- a/iphone/Maps/Classes/MWMPlacePageManager.mm +++ b/iphone/Maps/Classes/MWMPlacePageManager.mm @@ -317,7 +317,7 @@ - (void)setTopBound:(CGFloat)topBound { - if (topBound == _topBound) + if (_topBound == topBound) return; _topBound = topBound; diff --git a/iphone/Maps/Classes/MWMiPadPlacePageLayoutImpl.mm b/iphone/Maps/Classes/MWMiPadPlacePageLayoutImpl.mm index 19da643504..f7dda429de 100644 --- a/iphone/Maps/Classes/MWMiPadPlacePageLayoutImpl.mm +++ b/iphone/Maps/Classes/MWMiPadPlacePageLayoutImpl.mm @@ -1,12 +1,12 @@ -#import "MWMPlacePageLayout.h" #import "MWMiPadPlacePageLayoutImpl.h" +#import "MWMPlacePageLayout.h" namespace { CGFloat const kPlacePageWidth = 360; -CGFloat const kLeftOffset = 12.; -CGFloat const kTopOffset = 36.; -CGFloat const kBottomOffset = 60.; +CGFloat const kLeftOffset = 12; +CGFloat const kTopOffset = 36; +CGFloat const kBottomOffset = 60; } // namespace @interface MWMPPView (ActionBarLayout) @@ -17,11 +17,9 @@ CGFloat const kBottomOffset = 60.; - (void)layoutSubviews { + [super layoutSubviews]; if (!IPAD) - { - [super layoutSubviews]; return; - } for (UIView * sv in self.subviews) { @@ -86,7 +84,7 @@ CGFloat const kBottomOffset = 60.; ppView.tableView.scrollEnabled = NO; actionBar.alpha = 0; ppView.alpha = 0; - ppView.origin = {-self.leftBound - kPlacePageWidth, self.topBound}; + ppView.origin = {- kPlacePageWidth, self.topBound}; [self.ownerView addSubview:ppView]; place_page_layout::animate(^{ @@ -100,7 +98,7 @@ CGFloat const kBottomOffset = 60.; { auto ppView = self.placePageView; place_page_layout::animate(^{ - ppView.maxX = -self.leftBound; + ppView.maxX = 0; ppView.alpha = 0; },^{ [self.placePageView removeFromSuperview]; @@ -187,7 +185,7 @@ CGFloat const kBottomOffset = 60.; - (void)didPan:(UIPanGestureRecognizer *)pan { MWMPPView * view = self.placePageView; - UIView * superview = view.superview; + auto superview = view.superview; CGFloat const leftOffset = self.leftBound; view.minX += [pan translationInView:superview].x; @@ -199,7 +197,8 @@ CGFloat const kBottomOffset = 60.; UIGestureRecognizerState const state = pan.state; if (state == UIGestureRecognizerStateEnded || state == UIGestureRecognizerStateCancelled) { - if (alpha < 0.8) + CGFloat constexpr designAlpha = 0.8; + if (alpha < designAlpha) { [self onClose]; } diff --git a/iphone/Maps/Classes/MWMiPhonePlacePageLayoutImpl.mm b/iphone/Maps/Classes/MWMiPhonePlacePageLayoutImpl.mm index 4d0dd2143b..cd9f37845b 100644 --- a/iphone/Maps/Classes/MWMiPhonePlacePageLayoutImpl.mm +++ b/iphone/Maps/Classes/MWMiPhonePlacePageLayoutImpl.mm @@ -67,13 +67,13 @@ CGFloat const kLuftDraggingOffset = 30; - (void)onShow { self.state = State::Bottom; - [self colapse]; + [self collapse]; } - (void)onClose { place_page_layout::animate(^{ - self.actionBar.origin = {0., self.ownerView.height}; + self.actionBar.minY = self.ownerView.height; [self.scrollView setContentOffset:{} animated:YES]; },^{ [self.actionBar removeFromSuperview]; @@ -85,7 +85,7 @@ CGFloat const kLuftDraggingOffset = 30; - (void)onScreenResize:(CGSize const &)size { self.scrollView.frame = {{}, size}; - self.placePageView.origin = {0., size.height}; + self.placePageView.minY = size.height; auto actionBar = self.actionBar; actionBar.frame = {{0., size.height - actionBar.height}, {size.width, actionBar.height}}; @@ -100,9 +100,9 @@ CGFloat const kLuftDraggingOffset = 30; - (void)onActionBarInit:(MWMPlacePageActionBar *)actionBar { - UIView * superview = self.ownerView; + auto superview = self.ownerView; self.actionBar = actionBar; - actionBar.origin = {0., superview.height}; + actionBar.minY = superview.height; [superview addSubview:_actionBar]; } @@ -118,16 +118,15 @@ CGFloat const kLuftDraggingOffset = 30; actionBar.minY = actionBar.superview.height - actionBar.height; // We decrease expanded offset for 2 pixels because it looks more clear. - auto constexpr designOffset = 2; + CGFloat constexpr designOffset = 2; self.expandedContentOffset = height + actionBar.height - designOffset; - auto const targetOffset = - self.state == State::Bottom ? self.expandedContentOffset : self.topContentOffset; + auto const targetOffset = self.state == State::Bottom ? self.expandedContentOffset : self.topContentOffset; [self.scrollView setContentOffset:{ 0, targetOffset } animated:YES]; }); } -- (void)colapse +- (void)collapse { self.scrollView.scrollEnabled = NO; [self.placePageView hideTableView:YES]; @@ -154,10 +153,7 @@ CGFloat const kLuftDraggingOffset = 30; { auto const target = self.openContentOffset; auto const ppView = self.placePageView; - if (target > ppView.height) - return ppView.height; - - return target; + return MIN(target, ppView.height); } - (void)scrollViewDidScroll:(MWMPPScrollView *)scrollView @@ -203,8 +199,7 @@ CGFloat const kLuftDraggingOffset = 30; { auto const isDirectionUp = self.direction == ScrollDirection::Up; self.state = isDirectionUp ? State::Top : State::Bottom; - (*targetContentOffset).y = - isDirectionUp ? openOffset : self.expandedContentOffset; + (*targetContentOffset).y = isDirectionUp ? openOffset : self.expandedContentOffset; } else if (actualOffset > openOffset && targetOffset < openOffset) { @@ -215,7 +210,7 @@ CGFloat const kLuftDraggingOffset = 30; { (*targetContentOffset).y = 0; place_page_layout::animate(^{ - self.actionBar.origin = {0., self.ownerView.height}; + self.actionBar.minY = self.ownerView.height; }); } else