radunitescu's Objective-C Readability code for review
Change-Id: I9d787331ddfa93137eec1dec7f3fecaef5456f28
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6309086
Reviewed-by: Éric Noyau <noyau@chromium.org>
Commit-Queue: Radu Nitescu <radunitescu@google.com>
Cr-Commit-Position: refs/heads/main@{#1428860}
diff --git a/ios/chrome/browser/lens_overlay/model/lens_overlay_detents_manager.h b/ios/chrome/browser/lens_overlay/model/lens_overlay_detents_manager.h
index 92ff339..6bbeb954 100644
--- a/ios/chrome/browser/lens_overlay/model/lens_overlay_detents_manager.h
+++ b/ios/chrome/browser/lens_overlay/model/lens_overlay_detents_manager.h
@@ -9,10 +9,22 @@
#import "ios/chrome/browser/lens_overlay/model/lens_overlay_sheet_detent_state.h"
-@protocol LensOverlayDetentsChangeObserver;
+@protocol LensOverlayDetentsManagerDelegate;
// Manages the detents for a given bottom sheet, adapting to different detent
// sizes.
+//
+// The sheet detent state defines the set of detents the sheet can settle
+// into once the user completes a manual drag gesture and releases it.
+// The current presentation strategy dictates the possible height variations
+// between the available detents of each state.
+//
+// While the semantic meaning of each state is consistent, the way each state
+// presentation is dictated by the current presentation strategy. The employed
+// strategy dictates variation in height of detents.
+//
+// The number of detents can be subject to change and its consistency is not
+// guaranteed between presentation strategies.
@interface LensOverlayDetentsManager : NSObject
// The estimated detent medium detent height, with respect to the current
@@ -20,12 +32,15 @@
@property(nonatomic, readonly) CGFloat estimatedMediumDetentHeight;
// The object notified of bottom sheet detent changes.
-@property(nonatomic, weak) id<LensOverlayDetentsChangeObserver> observer;
+@property(nonatomic, weak) id<LensOverlayDetentsManagerDelegate> delegate;
// Current sheet dimension.
@property(nonatomic, readonly) SheetDimensionState sheetDimension;
-// The strategy to use when presenting in unrestricted mode.
+// The strategy to use when presenting.
+//
+// Changing the presentation strategy adjusts the detents for unrestricted
+// movement.
@property(nonatomic, assign)
SheetDetentPresentationStategy presentationStrategy;
@@ -55,15 +70,17 @@
@end
-// Observes changes in the detents and dimension states.
-@protocol LensOverlayDetentsChangeObserver <NSObject>
+// Reacts to changes in detents and dimension states.
+@protocol LensOverlayDetentsManagerDelegate <NSObject>
-// Called when the dimension state changes. Does not report the initial value,
-// only publishes changes recorded after the subscription.
-- (void)onBottomSheetDimensionStateChanged:(SheetDimensionState)state;
+// Called when the dimension state changes.
+- (void)lensOverlayDetentsManagerDidChangeDimensionState:
+ (LensOverlayDetentsManager*)detentsManager;
-// Called before dismissing the bottom sheet.
-- (BOOL)bottomSheetShouldDismissFromState:(SheetDimensionState)state;
+// Asks the delegate for permission to dismiss the presentation.
+- (BOOL)lensOverlayDetentsManagerShouldDismissBottomSheet:
+ (LensOverlayDetentsManager*)detentsManager;
+
@end
#endif // IOS_CHROME_BROWSER_LENS_OVERLAY_MODEL_LENS_OVERLAY_DETENTS_MANAGER_H_
diff --git a/ios/chrome/browser/lens_overlay/model/lens_overlay_detents_manager.mm b/ios/chrome/browser/lens_overlay/model/lens_overlay_detents_manager.mm
index c387696c..f415f77 100644
--- a/ios/chrome/browser/lens_overlay/model/lens_overlay_detents_manager.mm
+++ b/ios/chrome/browser/lens_overlay/model/lens_overlay_detents_manager.mm
@@ -6,32 +6,59 @@
namespace {
-NSString* const kCustomConsentSheetDetentIdentifier =
- @"kCustomConsentSheetDetentIdentifier";
+NSString* const kConsentSheetDetentIdentifier =
+ @"kConsentSheetDetentIdentifier";
+// The identifier for the medium detent when presenting in the translate filter
+// strategy.
NSString* const kTranslateModeMediumDetentIdentifier =
@"kTranslateModeMediumDetentIdentifier";
-NSString* const kCustomPeakSheetDetentIdentifier =
- @"kCustomPeakSheetDetentIdentifier";
+// The identifier for the peak detent.
+NSString* const kPeakSheetDetentIdentifier = @"kPeakSheetDetentIdentifier";
// The detent height in points for the 'peak' state of the bottom sheet.
-const CGFloat kPeakDetentHeight = 100;
+const CGFloat kPeakDetentHeight = 100.0;
// The percentage of the screen that will be covered by the bottom sheet in
// translate mode.
const CGFloat kTranslateSheetHeightRatio = 0.33;
+
} // namespace
-@interface LensOverlayDetentsManager (Private) <
- UISheetPresentationControllerDelegate>
+@interface LensOverlayDetentsManager () <UISheetPresentationControllerDelegate>
-@property(nonatomic, readonly) UISheetPresentationControllerDetent* largeDetent;
-@property(nonatomic, readonly)
- UISheetPresentationControllerDetent* mediumDetent;
-@property(nonatomic, readonly)
- UISheetPresentationControllerDetent* consentDetent;
-@property(nonatomic, readonly) UISheetPresentationControllerDetent* peakDetent;
+// Whether the bottom sheet being managed is in the medium detent dimension.
+- (BOOL)isInMediumDetent;
+
+// Whether the bottom sheet being managed is in the large detent dimension.
+- (BOOL)isInLargeDetent;
+
+// The height of The base window of the presentation
+- (CGFloat)windowHeight;
+
+// Changes the current set of available detents for a given a desired sheet
+// state. Also notifies the delegate of any change in detents.
+- (void)setDetentsForState:(SheetDetentState)state;
+
+// Informs the delegate when a change in the sheet dimension occurs.
+- (void)reportDimensionChangeIfNeeded;
+
+// A detent for the sheet that’s approximately the full height of the screen
+// (excluding the top safe area which is not covered).
+- (UISheetPresentationControllerDetent*)largeDetent;
+
+// A detent for the sheet that’s approximately half the height of the screen
+// when presenting for the selection filter and on third of the screen for
+// the translation filter.
+- (UISheetPresentationControllerDetent*)mediumDetent;
+
+// The detent in which to present the consent dialog.
+- (UISheetPresentationControllerDetent*)consentDetent;
+
+// The detent of the peak state that covers a small portion of the screen,
+// allowing most of the content behind the sheet to be visible.
+- (UISheetPresentationControllerDetent*)peakDetent;
@end
@@ -70,57 +97,56 @@
return self;
}
-#pragma mark - Public
+#pragma mark - Public properties
- (CGFloat)estimatedMediumDetentHeight {
switch (_presentationStrategy) {
case SheetDetentPresentationStategySelection:
- return self.windowHeight / 2;
+ return [self windowHeight] / 2;
case SheetDetentPresentationStategyTranslate:
- return self.windowHeight * kTranslateSheetHeightRatio;
+ return [self windowHeight] * kTranslateSheetHeightRatio;
default:
return 0;
}
}
- (SheetDimensionState)sheetDimension {
- NSString* identifier = _sheet.selectedDetentIdentifier;
- BOOL isInMediumDetent = self.isInMediumDetent;
- BOOL isInLargestDetent = self.isInLargeDetent;
- BOOL isPeaking =
- [identifier isEqualToString:kCustomPeakSheetDetentIdentifier];
- BOOL isConsent =
- [identifier isEqualToString:kCustomConsentSheetDetentIdentifier];
- if (isInLargestDetent) {
+ if ([self isInLargeDetent]) {
return SheetDimensionStateLarge;
- } else if (isInMediumDetent) {
- return SheetDimensionStateMedium;
- } else if (isPeaking) {
- return SheetDimensionStatePeaking;
- } else if (isConsent) {
- return SheetDimensionStateConsent;
- } else {
- return SheetDimensionStateHidden;
}
-}
+ if ([self isInMediumDetent]) {
+ return SheetDimensionStateMedium;
+ }
-- (void)adjustDetentsForState:(SheetDetentState)state {
- __weak __typeof(self) weakSelf = self;
- [_sheet animateChanges:^{
- [weakSelf setDetentsForState:state];
- }];
+ NSString* identifier = _sheet.selectedDetentIdentifier;
+ if ([identifier isEqualToString:kPeakSheetDetentIdentifier]) {
+ return SheetDimensionStatePeaking;
+ }
+
+ if ([identifier isEqualToString:kConsentSheetDetentIdentifier]) {
+ return SheetDimensionStateConsent;
+ }
+
+ return SheetDimensionStateHidden;
}
- (void)setPresentationStrategy:
(SheetDetentPresentationStategy)presentationStrategy {
_presentationStrategy = presentationStrategy;
- BOOL unrestrictedMovement = self.isInMediumDetent || self.isInLargeDetent;
- if (unrestrictedMovement) {
+ if ([self isInMediumDetent] || [self isInLargeDetent]) {
// Refresh the detents presentation for the unrestricted state.
[self adjustDetentsForState:SheetDetentStateUnrestrictedMovement];
}
}
+#pragma mark - Public methods
+
+- (void)adjustDetentsForState:(SheetDetentState)state {
+ [_sheet animateChanges:^{
+ [self setDetentsForState:state];
+ }];
+}
+
- (void)requestMaximizeBottomSheet {
[_sheet animateChanges:^{
_sheet.selectedDetentIdentifier =
@@ -146,16 +172,16 @@
- (BOOL)presentationControllerShouldDismiss:
(UIPresentationController*)presentationController {
- if (!_observer) {
+ if (!_delegate) {
return YES;
}
- return [_observer bottomSheetShouldDismissFromState:self.sheetDimension];
+ return [_delegate lensOverlayDetentsManagerShouldDismissBottomSheet:self];
}
- (void)presentationControllerDidDismiss:
(UIPresentationController*)presentationController {
_sheet.selectedDetentIdentifier = nil;
- [_observer onBottomSheetDimensionStateChanged:SheetDimensionStateHidden];
+ [_delegate lensOverlayDetentsManagerDidChangeDimensionState:self];
}
#pragma mark - Private
@@ -180,19 +206,19 @@
- (void)setDetentsForState:(SheetDetentState)state {
switch (state) {
case SheetDetentStateUnrestrictedMovement:
- _sheet.detents = @[ self.mediumDetent, self.largeDetent ];
- _sheet.largestUndimmedDetentIdentifier = self.largeDetent.identifier;
- _sheet.selectedDetentIdentifier = self.mediumDetent.identifier;
+ _sheet.detents = @[ [self mediumDetent], [self largeDetent] ];
+ _sheet.largestUndimmedDetentIdentifier = [self largeDetent].identifier;
+ _sheet.selectedDetentIdentifier = [self mediumDetent].identifier;
break;
case SheetDetentStatePeakEnabled:
- _sheet.detents = @[ self.peakDetent ];
- _sheet.largestUndimmedDetentIdentifier = self.peakDetent.identifier;
- _sheet.selectedDetentIdentifier = self.peakDetent.identifier;
+ _sheet.detents = @[ [self peakDetent] ];
+ _sheet.largestUndimmedDetentIdentifier = kPeakSheetDetentIdentifier;
+ _sheet.selectedDetentIdentifier = kPeakSheetDetentIdentifier;
break;
case SheetDetentStateConsentDialog:
- _sheet.detents = @[ self.consentDetent ];
- _sheet.largestUndimmedDetentIdentifier = self.consentDetent.identifier;
- _sheet.selectedDetentIdentifier = self.consentDetent.identifier;
+ _sheet.detents = @[ [self consentDetent] ];
+ _sheet.largestUndimmedDetentIdentifier = kConsentSheetDetentIdentifier;
+ _sheet.selectedDetentIdentifier = kConsentSheetDetentIdentifier;
break;
}
@@ -201,7 +227,7 @@
- (void)reportDimensionChangeIfNeeded {
if (self.sheetDimension != _latestReportedDimension) {
- [_observer onBottomSheetDimensionStateChanged:self.sheetDimension];
+ [_delegate lensOverlayDetentsManagerDidChangeDimensionState:self];
_latestReportedDimension = self.sheetDimension;
}
}
@@ -215,7 +241,7 @@
return [UISheetPresentationControllerDetent mediumDetent];
}
- CGFloat resolvedHeight = self.windowHeight * kTranslateSheetHeightRatio;
+ CGFloat resolvedHeight = [self windowHeight] * kTranslateSheetHeightRatio;
auto heightResolver = ^CGFloat(
id<UISheetPresentationControllerDetentResolutionContext> context) {
return resolvedHeight;
@@ -233,7 +259,7 @@
return presentedViewController.preferredContentSize.height;
};
return [UISheetPresentationControllerDetent
- customDetentWithIdentifier:kCustomConsentSheetDetentIdentifier
+ customDetentWithIdentifier:kConsentSheetDetentIdentifier
resolver:consentHeightResolver];
}
@@ -243,7 +269,7 @@
return kPeakDetentHeight;
};
return [UISheetPresentationControllerDetent
- customDetentWithIdentifier:kCustomPeakSheetDetentIdentifier
+ customDetentWithIdentifier:kPeakSheetDetentIdentifier
resolver:peakHeightResolver];
}
diff --git a/ios/chrome/browser/lens_overlay/model/lens_overlay_detents_manager_unittest.mm b/ios/chrome/browser/lens_overlay/model/lens_overlay_detents_manager_unittest.mm
index 55c5285..896cbe0 100644
--- a/ios/chrome/browser/lens_overlay/model/lens_overlay_detents_manager_unittest.mm
+++ b/ios/chrome/browser/lens_overlay/model/lens_overlay_detents_manager_unittest.mm
@@ -14,21 +14,25 @@
using base::test::ios::kWaitForUIElementTimeout;
using base::test::ios::WaitUntilConditionOrTimeout;
-// Fake observer that exposes the latest reported value by the detents manager.
-@interface FakeDetentsObserver : NSObject <LensOverlayDetentsChangeObserver>
+// Fake delegate that exposes the latest reported value by the detents manager.
+@interface FakeDetentsManagerDelegate
+ : NSObject <LensOverlayDetentsManagerDelegate>
// The latest reported dimension state.
@property(nonatomic, readonly) SheetDimensionState latestReportedDimensionState;
@end
-@implementation FakeDetentsObserver
-- (BOOL)bottomSheetShouldDismissFromState:(SheetDimensionState)state {
+@implementation FakeDetentsManagerDelegate
+
+- (BOOL)lensOverlayDetentsManagerShouldDismissBottomSheet:
+ (LensOverlayDetentsManager*)detentsManager {
return YES;
}
-- (void)onBottomSheetDimensionStateChanged:(SheetDimensionState)state {
- _latestReportedDimensionState = state;
+- (void)lensOverlayDetentsManagerDidChangeDimensionState:
+ (LensOverlayDetentsManager*)detentsManager {
+ _latestReportedDimensionState = detentsManager.sheetDimension;
}
@end
@@ -51,15 +55,9 @@
window:scoped_key_window_.Get()];
}
- ~LensOverlayDetentsManagerTest() override {
- detents_manager_ = nil;
- presented_view_controller_ = nil;
- presenting_view_controller_ = nil;
- }
-
LensOverlayDetentsManager* detents_manager_;
- UIViewController* presented_view_controller_;
- UIViewController* presenting_view_controller_;
+ __strong UIViewController* presented_view_controller_;
+ __strong UIViewController* presenting_view_controller_;
ScopedKeyWindow scoped_key_window_;
// Presents and blocks until the view controller is presented.
@@ -128,7 +126,7 @@
[detents_manager_ adjustDetentsForState:SheetDetentStateUnrestrictedMovement];
WaitForPresentation();
- // Then the default presentation dimension immediatelly after presenting
+ // Then the default presentation dimension immediately after presenting
// should be medium.
EXPECT_EQ(detents_manager_.sheetDimension, SheetDimensionStateMedium);
@@ -183,11 +181,12 @@
}
// Tests the ability of adjusting the detent after the initial presentation.
-TEST_F(LensOverlayDetentsManagerTest, TestAdjustingDetentsNotifiesObserver) {
- // Given a detents observer.
- FakeDetentsObserver* fakeObserver = [[FakeDetentsObserver alloc] init];
- detents_manager_.observer = fakeObserver;
- EXPECT_EQ(fakeObserver.latestReportedDimensionState,
+TEST_F(LensOverlayDetentsManagerTest, TestAdjustingDetentsNotifiesDelegate) {
+ // Given a detents delegate.
+ FakeDetentsManagerDelegate* fakeDelegate =
+ [[FakeDetentsManagerDelegate alloc] init];
+ detents_manager_.delegate = fakeDelegate;
+ EXPECT_EQ(fakeDelegate.latestReportedDimensionState,
SheetDimensionStateHidden);
// When presenting in the unrestricted movement state.
@@ -195,10 +194,10 @@
WaitForPresentation();
// Then the changes in the dimension state should be propagated to the detents
- // observer.
- EXPECT_EQ(fakeObserver.latestReportedDimensionState,
+ // delegate.
+ EXPECT_EQ(fakeDelegate.latestReportedDimensionState,
SheetDimensionStateMedium);
[detents_manager_ adjustDetentsForState:SheetDetentStatePeakEnabled];
- EXPECT_EQ(fakeObserver.latestReportedDimensionState,
+ EXPECT_EQ(fakeDelegate.latestReportedDimensionState,
SheetDimensionStatePeaking);
}
diff --git a/ios/chrome/browser/lens_overlay/ui/lens_overlay_consent_presenter.mm b/ios/chrome/browser/lens_overlay/ui/lens_overlay_consent_presenter.mm
index 0a574a4..e6463ab 100644
--- a/ios/chrome/browser/lens_overlay/ui/lens_overlay_consent_presenter.mm
+++ b/ios/chrome/browser/lens_overlay/ui/lens_overlay_consent_presenter.mm
@@ -9,7 +9,7 @@
#import "ios/chrome/browser/lens_overlay/model/lens_overlay_sheet_detent_state.h"
#import "ios/chrome/browser/lens_overlay/ui/lens_overlay_consent_view_controller.h"
-@interface LensOverlayConsentPresenter () <LensOverlayDetentsChangeObserver>
+@interface LensOverlayConsentPresenter () <LensOverlayDetentsManagerDelegate>
@end
@implementation LensOverlayConsentPresenter {
@@ -47,7 +47,7 @@
_detentsManager =
[[LensOverlayDetentsManager alloc] initWithBottomSheet:sheet
window:window];
- _detentsManager.observer = self;
+ _detentsManager.delegate = self;
[_detentsManager adjustDetentsForState:SheetDetentStateConsentDialog];
[_presentingViewController
presentViewController:_presentedConsentViewController
@@ -69,16 +69,18 @@
completion:completion];
}
-#pragma mark - LensOverlayDetentsChangeObserver
+#pragma mark - LensOverlayDetentsManagerDelegate
-- (void)onBottomSheetDimensionStateChanged:(SheetDimensionState)state {
- if (state == SheetDimensionStateHidden) {
+- (void)lensOverlayDetentsManagerDidChangeDimensionState:
+ (LensOverlayDetentsManager*)detentsManager {
+ if (detentsManager.sheetDimension == SheetDimensionStateHidden) {
[self.delegate requestDismissalOfConsentDialog:self];
}
}
-- (BOOL)bottomSheetShouldDismissFromState:(SheetDimensionState)state {
- DCHECK(state == SheetDimensionStateConsent);
+- (BOOL)lensOverlayDetentsManagerShouldDismissBottomSheet:
+ (LensOverlayDetentsManager*)detentsManager {
+ DCHECK(detentsManager.sheetDimension == SheetDimensionStateConsent);
return YES;
}
diff --git a/ios/chrome/browser/lens_overlay/ui/lens_overlay_results_page_presenter.mm b/ios/chrome/browser/lens_overlay/ui/lens_overlay_results_page_presenter.mm
index f85efa4..06f793b 100644
--- a/ios/chrome/browser/lens_overlay/ui/lens_overlay_results_page_presenter.mm
+++ b/ios/chrome/browser/lens_overlay/ui/lens_overlay_results_page_presenter.mm
@@ -31,8 +31,9 @@
} // namespace
-@interface LensOverlayResultsPagePresenter () <LensOverlayPanTrackerDelegate,
- LensOverlayDetentsChangeObserver>
+@interface LensOverlayResultsPagePresenter () <
+ LensOverlayPanTrackerDelegate,
+ LensOverlayDetentsManagerDelegate>
@end
@implementation LensOverlayResultsPagePresenter {
@@ -160,7 +161,7 @@
initWithBottomSheet:sheet
window:self.presentationWindow
presentationStrategy:strategy];
- _detentsManager.observer = self;
+ _detentsManager.delegate = self;
[_detentsManager adjustDetentsForState:SheetDetentStateUnrestrictedMovement];
if (maximizeSheet) {
@@ -389,14 +390,17 @@
}
}
-#pragma mark - LensOverlayDetentsChangeObserver
+#pragma mark - LensOverlayDetentsManagerDelegate
-- (void)onBottomSheetDimensionStateChanged:(SheetDimensionState)state {
- [self.delegate onResultsPageDimensionStateChanged:state];
+- (void)lensOverlayDetentsManagerDidChangeDimensionState:
+ (LensOverlayDetentsManager*)detentsManager {
+ [self.delegate
+ onResultsPageDimensionStateChanged:detentsManager.sheetDimension];
}
-- (BOOL)bottomSheetShouldDismissFromState:(SheetDimensionState)state {
- switch (state) {
+- (BOOL)lensOverlayDetentsManagerShouldDismissBottomSheet:
+ (LensOverlayDetentsManager*)detentsManager {
+ switch (self.sheetDimension) {
case SheetDimensionStateConsent:
case SheetDimensionStateHidden:
return YES;
