From b6a939115fead4a856b29b39e34f930b3e4e0b84 Mon Sep 17 00:00:00 2001 From: Michelle Linington Date: Tue, 15 Dec 2020 17:58:43 -0800 Subject: [PATCH] Fix peek and pop with CVC perf improvements - Permit a nil navigation controller if we're in a preview platter - Instead of directly pushing onto the navigation controller's stack, ask the split view controller to present the thread when we pop. - Instead of hiding the input toolbar when shown in a preview platter, set the bottomViewType to .none. Content insets are determined by the bottom bar size. Even if the input toolbar is hidden, we were still using its height in our content inset. This fixes that. --- ...ConversationViewController+BottomBar.swift | 38 +++++-------------- .../ConversationViewController+CVC.swift | 19 +++++----- .../ConversationViewController.h | 1 - .../ConversationViewController.m | 17 +++------ .../HomeView/ConversationListViewController.m | 4 +- 5 files changed, 25 insertions(+), 54 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController+BottomBar.swift b/Signal/src/ViewControllers/ConversationView/ConversationViewController+BottomBar.swift index 19bc4c9651..3121020951 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController+BottomBar.swift +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController+BottomBar.swift @@ -29,6 +29,10 @@ public extension ConversationViewController { owsAssertDebug(hasViewWillAppearEverBegun) if viewState.bottomViewType != newValue { + if viewState.bottomViewType == .inputToolbar { + // Dismiss the keyboard if we're swapping out the input toolbar + dismissKeyBoard() + } viewState.bottomViewType = newValue updateBottomBar() } @@ -62,7 +66,11 @@ public extension ConversationViewController { case .selection: return .selection case .normal: - return .inputToolbar + if viewState.isInPreviewPlatter || userLeftGroup { + return .none + } else { + return .inputToolbar + } } } }() @@ -127,7 +135,6 @@ public extension ConversationViewController { updateInputAccessoryPlaceholderHeight() updateBottomBarPosition() updateContentInsets(animated: hasAppearedAndHasAppliedFirstLoad) - updateInputVisibility() } // This is expensive. We only need to do it if conversationStyle has changed. @@ -224,33 +231,6 @@ public extension ConversationViewController { ensureBottomViewType() } - @objc - func updateInputVisibility() { - AssertIsOnMainThread() - - guard hasViewWillAppearEverBegun else { - return - } - guard let inputToolbar = inputToolbar else { - owsFailDebug("Missing inputToolbar.") - return - } - - if viewState.isInPreviewPlatter { - inputToolbar.isHidden = true - dismissKeyBoard() - return - } - - if self.userLeftGroup { - // user has requested they leave the group. further sends disallowed - inputToolbar.isHidden = true - dismissKeyBoard() - } else { - inputToolbar.isHidden = false - } - } - @objc func updateInputToolbarLayout() { AssertIsOnMainThread() diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController+CVC.swift b/Signal/src/ViewControllers/ConversationView/ConversationViewController+CVC.swift index 8bb9746f59..ed8608bfd9 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController+CVC.swift +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController+CVC.swift @@ -626,14 +626,19 @@ extension ConversationViewController: CVLoadCoordinatorDelegate { guard self.conversationStyle.type != .`default` else { // Once we built a normal style, never go back to // building an initial or placeholder style. - owsAssertDebug(navigationController != nil) + owsAssertDebug(navigationController != nil || viewState.isInPreviewPlatter) return buildDefaultConversationStyle(type: .`default`) } guard let navigationController = navigationController else { - // Treat all styles as "initial" (not to be trusted) until - // we have a navigationController. - return buildDefaultConversationStyle(type: .initial) + if viewState.isInPreviewPlatter { + // In a preview platter, we'll never have a navigation controller + return buildDefaultConversationStyle(type: .`default`) + } else { + // Treat all styles as "initial" (not to be trusted) until + // we have a navigationController. + return buildDefaultConversationStyle(type: .initial) + } } let collectionViewWidth = self.collectionView.width @@ -649,12 +654,6 @@ extension ConversationViewController: CVLoadCoordinatorDelegate { let hasValidStyle = !isMissingSafeAreaInsets && !hasInvalidWidth if hasValidStyle { // No need to rewrite style; style is already valid. - // - // Verify that navigationController view and CVC view - // state converge. - owsAssertDebug(navigationViewWidth == rootViewWidth) - owsAssertDebug(navigationViewWidth == collectionViewWidth) - return buildDefaultConversationStyle(type: .`default`) } else { let viewAge = abs(self.viewState.viewCreationDate.timeIntervalSinceNow) diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.h b/Signal/src/ViewControllers/ConversationView/ConversationViewController.h index 3507ebeda4..8f825c367e 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.h +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.h @@ -55,7 +55,6 @@ typedef NS_ENUM(NSUInteger, ConversationViewAction) { #pragma mark 3D Touch Methods - (void)peekSetup; -- (void)popped; #pragma mark - Keyboard Shortcuts diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index c9436aa88e..0c0996fc0c 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -357,6 +357,10 @@ typedef enum : NSUInteger { { if (self.viewState.isInPreviewPlatter != inPreviewPlatter) { self.viewState.isInPreviewPlatter = inPreviewPlatter; + + if (self.hasViewWillAppearEverBegun) { + [self ensureBottomViewType]; + } [self configureScrollDownButtons]; } } @@ -367,12 +371,6 @@ typedef enum : NSUInteger { self.actionOnOpen = ConversationViewActionNone; } -- (void)popped -{ - [self setInPreviewPlatter:NO]; - [self updateInputVisibility]; -} - - (void)updateV2GroupIfNecessary { if (!self.thread.isGroupV2Thread) { @@ -423,7 +421,8 @@ typedef enum : NSUInteger { - (void)viewDidLoad { - OWSAssertDebug(self.navigationController != nil); + // We won't have a navigation controller if we're presented in a preview + OWSAssertDebug(self.navigationController != nil || self.isInPreviewPlatter); #ifdef TESTABLE_BUILD [self.initialLoadBenchSteps step:@"viewDidLoad.1"]; @@ -632,10 +631,6 @@ typedef enum : NSUInteger { [self createGestureRecognizers]; } - // We need to recheck on every appearance, since the user may have left the group in the settings VC, - // or on another device. - [self updateInputVisibility]; - self.isViewVisible = YES; [self viewWillAppearForLoad]; diff --git a/Signal/src/ViewControllers/HomeView/ConversationListViewController.m b/Signal/src/ViewControllers/HomeView/ConversationListViewController.m index 286fa3faf6..d0b70be9c3 100644 --- a/Signal/src/ViewControllers/HomeView/ConversationListViewController.m +++ b/Signal/src/ViewControllers/HomeView/ConversationListViewController.m @@ -811,9 +811,7 @@ NSString *const kArchiveButtonPseudoGroup = @"kArchiveButtonPseudoGroup"; commitViewController:(UIViewController *)viewControllerToCommit { ConversationViewController *vc = (ConversationViewController *)viewControllerToCommit; - [vc popped]; - - [self.navigationController pushViewController:vc animated:NO]; + [self presentThread:vc.thread action:ConversationViewActionNone animated:NO]; } - (void)showNewConversationView