Commit 3fdbbad5 authored by adamta's avatar adamta Committed by Commit Bot

[iOS] Manually trigger Discover feed IPH bubble

Reverts cl/2414613 and adds the ability to trigger the Discover feed
IPH bubble manually, rather than at the same time as the other bubbles.

Stores a reference to the Discover feed header menu button in the
ContentSuggestionsCoordinator. This allows for the menu button to be
accessed whenever it is needed, in order to create a named guide for
the IPH to work. Also, this allows for the feed alert menu to access
the button more easily.

Change-Id: I7cd50c922af8e2ace6914e680cce7f494e325bb6
Bug: 1139320, 1131571
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2507386
Commit-Queue: Adam Trudeau-Arcaro <adamta@google.com>
Reviewed-by: default avatarGauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822785}
parent 42f14ace
...@@ -2421,6 +2421,9 @@ NSString* const kBrowserViewControllerSnackbarCategory = ...@@ -2421,6 +2421,9 @@ NSString* const kBrowserViewControllerSnackbarCategory =
self.browserContainerViewController.contentView = nil; self.browserContainerViewController.contentView = nil;
self.browserContainerViewController.contentViewController = self.browserContainerViewController.contentViewController =
viewController; viewController;
[_ntpCoordinatorsForWebStates[webState]
constrainDiscoverHeaderMenuButtonNamedGuide];
[self.bubblePresenter presentDiscoverFeedHeaderTipBubble];
} else { } else {
self.browserContainerViewController.contentView = self.browserContainerViewController.contentView =
[self viewForWebState:webState]; [self viewForWebState:webState];
...@@ -5010,14 +5013,6 @@ NSString* const kBrowserViewControllerSnackbarCategory = ...@@ -5010,14 +5013,6 @@ NSString* const kBrowserViewControllerSnackbarCategory =
newTabPageCoordinator.toolbarDelegate = self.toolbarInterface; newTabPageCoordinator.toolbarDelegate = self.toolbarInterface;
newTabPageCoordinator.webState = webState; newTabPageCoordinator.webState = webState;
_ntpCoordinatorsForWebStates[webState] = newTabPageCoordinator; _ntpCoordinatorsForWebStates[webState] = newTabPageCoordinator;
[newTabPageCoordinator.viewController
willMoveToParentViewController:self.browserContainerViewController];
[self.browserContainerViewController
addChildViewController:newTabPageCoordinator.viewController];
[self.browserContainerViewController.view
addSubview:newTabPageCoordinator.viewController.view];
[newTabPageCoordinator.viewController
didMoveToParentViewController:self.browserContainerViewController];
} else { } else {
NewTabPageCoordinator* newTabPageCoordinator = NewTabPageCoordinator* newTabPageCoordinator =
_ntpCoordinatorsForWebStates[webState]; _ntpCoordinatorsForWebStates[webState];
......
...@@ -38,6 +38,9 @@ class ChromeBrowserState; ...@@ -38,6 +38,9 @@ class ChromeBrowserState;
// Notifies the presenter that the tools menu has been displayed. // Notifies the presenter that the tools menu has been displayed.
- (void)toolsMenuDisplayed; - (void)toolsMenuDisplayed;
// Presents a bubble associated with the Discover feed header's menu button.
- (void)presentDiscoverFeedHeaderTipBubble;
@end @end
#endif // IOS_CHROME_BROWSER_UI_BUBBLE_BUBBLE_PRESENTER_H_ #endif // IOS_CHROME_BROWSER_UI_BUBBLE_BUBBLE_PRESENTER_H_
...@@ -148,6 +148,41 @@ const CGFloat kBubblePresentationDelay = 1; ...@@ -148,6 +148,41 @@ const CGFloat kBubblePresentationDelay = 1;
} }
} }
- (void)presentDiscoverFeedHeaderTipBubble {
BubbleArrowDirection arrowDirection = BubbleArrowDirectionDown;
NSString* text =
l10n_util::GetNSStringWithFixup(IDS_IOS_DISCOVER_FEED_HEADER_IPH);
NamedGuide* guide = [NamedGuide guideWithName:kDiscoverFeedHeaderMenuGuide
view:self.rootViewController.view];
DCHECK(guide);
UIView* menuButton = guide.constrainedView;
// Checks "canPresentBubble" after checking that the NTP with feed is visible.
// This ensures that the feature tracker doesn't trigger the IPH event if the
// bubble isn't shown, which would prevent it from ever being shown again.
if (!menuButton || ![self canPresentBubble]) {
return;
}
CGPoint discoverFeedHeaderAnchor =
[menuButton.superview convertPoint:menuButton.frame.origin toView:nil];
discoverFeedHeaderAnchor.x += menuButton.frame.size.width / 2;
// If the feature engagement tracker does not consider it valid to display
// the new tab tip, then end early to prevent the potential reassignment
// of the existing |tabTipBubblePresenter| to nil.
BubbleViewControllerPresenter* presenter = [self
presentBubbleForFeature:feature_engagement::kIPHDiscoverFeedHeaderFeature
direction:arrowDirection
alignment:BubbleAlignmentTrailing
text:text
voiceOverAnnouncement:nil
anchorPoint:discoverFeedHeaderAnchor];
if (!presenter)
return;
self.discoverFeedHeaderMenuTipBubblePresenter = presenter;
}
#pragma mark - Private #pragma mark - Private
- (void)presentBubbles { - (void)presentBubbles {
...@@ -163,7 +198,6 @@ const CGFloat kBubblePresentationDelay = 1; ...@@ -163,7 +198,6 @@ const CGFloat kBubblePresentationDelay = 1;
// The bottom toolbar and Discover feed header menu don't use the // The bottom toolbar and Discover feed header menu don't use the
// isUserEngaged, so don't check if the user is engaged here. // isUserEngaged, so don't check if the user is engaged here.
[self presentBottomToolbarTipBubble]; [self presentBottomToolbarTipBubble];
[self presentDiscoverFeedHeaderTipBubble];
} }
- (void)presentLongPressBubble { - (void)presentLongPressBubble {
...@@ -258,42 +292,6 @@ presentBubbleForFeature:(const base::Feature&)feature ...@@ -258,42 +292,6 @@ presentBubbleForFeature:(const base::Feature&)feature
->NotifyEvent(feature_engagement::events::kBottomToolbarOpened); ->NotifyEvent(feature_engagement::events::kBottomToolbarOpened);
} }
// Presents a bubble associated with the Discover feed header's menu button.
- (void)presentDiscoverFeedHeaderTipBubble {
BubbleArrowDirection arrowDirection = BubbleArrowDirectionDown;
NSString* text =
l10n_util::GetNSStringWithFixup(IDS_IOS_DISCOVER_FEED_HEADER_IPH);
NamedGuide* guide = [NamedGuide guideWithName:kDiscoverFeedHeaderMenuGuide
view:self.rootViewController.view];
DCHECK(guide);
UIView* menuButton = guide.constrainedView;
// Checks "canPresentBubble" after checking that the NTP with feed is visible.
// This ensures that the feature tracker doesn't trigger the IPH event if the
// bubble isn't shown, which would prevent it from ever being shown again.
if (!menuButton || ![self canPresentBubble]) {
return;
}
CGPoint discoverFeedHeaderAnchor =
[menuButton.superview convertPoint:menuButton.frame.origin toView:nil];
discoverFeedHeaderAnchor.x += menuButton.frame.size.width / 2;
// If the feature engagement tracker does not consider it valid to display
// the new tab tip, then end early to prevent the potential reassignment
// of the existing |tabTipBubblePresenter| to nil.
BubbleViewControllerPresenter* presenter = [self
presentBubbleForFeature:feature_engagement::kIPHDiscoverFeedHeaderFeature
direction:arrowDirection
alignment:BubbleAlignmentTrailing
text:text
voiceOverAnnouncement:nil
anchorPoint:discoverFeedHeaderAnchor];
if (!presenter)
return;
self.discoverFeedHeaderMenuTipBubblePresenter = presenter;
}
// Optionally presents a bubble associated with the new tab tip in-product help // Optionally presents a bubble associated with the new tab tip in-product help
// promotion. If the feature engagement tracker determines it is valid to show // promotion. If the feature engagement tracker determines it is valid to show
// the new tab tip, then it initializes |tabTipBubblePresenter| and presents // the new tab tip, then it initializes |tabTipBubblePresenter| and presents
......
...@@ -5,7 +5,6 @@ ...@@ -5,7 +5,6 @@
#import "ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_discover_header_item.h" #import "ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_discover_header_item.h"
#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_constants.h" #import "ios/chrome/browser/ui/content_suggestions/content_suggestions_constants.h"
#import "ios/chrome/browser/ui/util/named_guide.h"
#import "ios/chrome/browser/ui/util/uikit_ui_util.h" #import "ios/chrome/browser/ui/util/uikit_ui_util.h"
#import "ios/chrome/common/ui/colors/UIColor+cr_semantic_colors.h" #import "ios/chrome/common/ui/colors/UIColor+cr_semantic_colors.h"
#import "ios/chrome/common/ui/colors/semantic_color_names.h" #import "ios/chrome/common/ui/colors/semantic_color_names.h"
...@@ -175,12 +174,9 @@ const CGFloat kFeedCardIPhoneWidth = 375; ...@@ -175,12 +174,9 @@ const CGFloat kFeedCardIPhoneWidth = 375;
} else { } else {
[self setHeaderForFeedVisible:visible animate:NO]; [self setHeaderForFeedVisible:visible animate:NO];
} }
// TODO(crbug.com/1131571)(adamta@): Check if still necessary to layout
// content view.
[self.contentView layoutIfNeeded]; [self.contentView layoutIfNeeded];
NamedGuide* menuButtonGuide =
[NamedGuide guideWithName:kDiscoverFeedHeaderMenuGuide
view:self.menuButton];
menuButtonGuide.constrainedView = self.menuButton;
self.discoverFeedVisible = [NSNumber numberWithBool:visible]; self.discoverFeedVisible = [NSNumber numberWithBool:visible];
} }
......
...@@ -54,6 +54,9 @@ class WebState; ...@@ -54,6 +54,9 @@ class WebState;
// Tell location bar has taken focus. // Tell location bar has taken focus.
- (void)locationBarDidBecomeFirstResponder; - (void)locationBarDidBecomeFirstResponder;
// Constrains the named layout guide for the Discover header menu button.
- (void)constrainDiscoverHeaderMenuButtonNamedGuide;
@end @end
#endif // IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_COORDINATOR_H_ #endif // IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_COORDINATOR_H_
...@@ -69,6 +69,7 @@ ...@@ -69,6 +69,7 @@
#import "ios/chrome/browser/ui/settings/utils/pref_backed_boolean.h" #import "ios/chrome/browser/ui/settings/utils/pref_backed_boolean.h"
#import "ios/chrome/browser/ui/sharing/sharing_coordinator.h" #import "ios/chrome/browser/ui/sharing/sharing_coordinator.h"
#import "ios/chrome/browser/ui/util/multi_window_support.h" #import "ios/chrome/browser/ui/util/multi_window_support.h"
#import "ios/chrome/browser/ui/util/named_guide.h"
#import "ios/chrome/browser/ui/util/uikit_ui_util.h" #import "ios/chrome/browser/ui/util/uikit_ui_util.h"
#import "ios/chrome/browser/url_loading/url_loading_browser_agent.h" #import "ios/chrome/browser/url_loading/url_loading_browser_agent.h"
#import "ios/chrome/browser/url_loading/url_loading_params.h" #import "ios/chrome/browser/url_loading/url_loading_params.h"
...@@ -109,6 +110,7 @@ ...@@ -109,6 +110,7 @@
DiscoverFeedMetricsRecorder* discoverFeedMetricsRecorder; DiscoverFeedMetricsRecorder* discoverFeedMetricsRecorder;
@property(nonatomic, strong) NTPHomeMediator* NTPMediator; @property(nonatomic, strong) NTPHomeMediator* NTPMediator;
@property(nonatomic, strong) UIViewController* discoverFeedViewController; @property(nonatomic, strong) UIViewController* discoverFeedViewController;
@property(nonatomic, strong) UIView* discoverFeedHeaderMenuButton;
@property(nonatomic, strong) URLDragDropHandler* dragDropHandler; @property(nonatomic, strong) URLDragDropHandler* dragDropHandler;
@property(nonatomic, strong) ActionSheetCoordinator* alertCoordinator; @property(nonatomic, strong) ActionSheetCoordinator* alertCoordinator;
// Redefined as readwrite. // Redefined as readwrite.
...@@ -337,6 +339,14 @@ ...@@ -337,6 +339,14 @@
return self.suggestionsViewController; return self.suggestionsViewController;
} }
- (void)constrainDiscoverHeaderMenuButtonNamedGuide {
NamedGuide* menuButtonGuide =
[NamedGuide guideWithName:kDiscoverFeedHeaderMenuGuide
view:self.discoverFeedHeaderMenuButton];
menuButtonGuide.constrainedView = self.discoverFeedHeaderMenuButton;
}
#pragma mark - ContentSuggestionsViewControllerAudience #pragma mark - ContentSuggestionsViewControllerAudience
- (void)promoShown { - (void)promoShown {
...@@ -346,6 +356,10 @@ ...@@ -346,6 +356,10 @@
[self.headerController setPromoCanShow:notificationPromo->CanShow()]; [self.headerController setPromoCanShow:notificationPromo->CanShow()];
} }
- (void)discoverHeaderMenuButtonShown:(UIView*)menuButton {
_discoverFeedHeaderMenuButton = menuButton;
}
#pragma mark - OverscrollActionsControllerDelegate #pragma mark - OverscrollActionsControllerDelegate
- (void)overscrollActionsController:(OverscrollActionsController*)controller - (void)overscrollActionsController:(OverscrollActionsController*)controller
...@@ -428,7 +442,7 @@ ...@@ -428,7 +442,7 @@
#pragma mark - DiscoverFeedMenuCommands #pragma mark - DiscoverFeedMenuCommands
- (void)openDiscoverFeedMenu:(UIView*)menuButton { - (void)openDiscoverFeedMenu {
[self.alertCoordinator stop]; [self.alertCoordinator stop];
self.alertCoordinator = nil; self.alertCoordinator = nil;
...@@ -437,8 +451,8 @@ ...@@ -437,8 +451,8 @@
browser:self.browser browser:self.browser
title:nil title:nil
message:nil message:nil
rect:menuButton.frame rect:self.discoverFeedHeaderMenuButton.frame
view:menuButton.superview]; view:self.discoverFeedHeaderMenuButton.superview];
__weak ContentSuggestionsCoordinator* weakSelf = self; __weak ContentSuggestionsCoordinator* weakSelf = self;
if ([self.contentSuggestionsVisible value]) { if ([self.contentSuggestionsVisible value]) {
......
...@@ -533,8 +533,9 @@ NSString* const kContentSuggestionsMostVisitedAccessibilityIdentifierPrefix = ...@@ -533,8 +533,9 @@ NSString* const kContentSuggestionsMostVisitedAccessibilityIdentifierPrefix =
ContentSuggestionsDiscoverHeaderCell* discoverFeedHeader = ContentSuggestionsDiscoverHeaderCell* discoverFeedHeader =
base::mac::ObjCCastStrict<ContentSuggestionsDiscoverHeaderCell>(cell); base::mac::ObjCCastStrict<ContentSuggestionsDiscoverHeaderCell>(cell);
[discoverFeedHeader.menuButton addTarget:self [discoverFeedHeader.menuButton addTarget:self
action:@selector(openDiscoverFeedMenu:) action:@selector(openDiscoverFeedMenu)
forControlEvents:UIControlEventTouchUpInside]; forControlEvents:UIControlEventTouchUpInside];
[self.audience discoverHeaderMenuButtonShown:discoverFeedHeader.menuButton];
} }
return cell; return cell;
} }
...@@ -945,8 +946,8 @@ NSString* const kContentSuggestionsMostVisitedAccessibilityIdentifierPrefix = ...@@ -945,8 +946,8 @@ NSString* const kContentSuggestionsMostVisitedAccessibilityIdentifierPrefix =
} }
// Opens top-level feed menu when pressing |menuButton|. // Opens top-level feed menu when pressing |menuButton|.
- (void)openDiscoverFeedMenu:(id)menuButton { - (void)openDiscoverFeedMenu {
[self.discoverFeedMenuHandler openDiscoverFeedMenu:menuButton]; [self.discoverFeedMenuHandler openDiscoverFeedMenu];
} }
// Evaluates whether or not another set of Discover feed articles should be // Evaluates whether or not another set of Discover feed articles should be
......
...@@ -11,6 +11,10 @@ ...@@ -11,6 +11,10 @@
// Notifies the audience that the promo has been shown. // Notifies the audience that the promo has been shown.
- (void)promoShown; - (void)promoShown;
// Notifies the audience that the Discover feed header menu has been shown, and
// provides a reference to the button.
- (void)discoverHeaderMenuButtonShown:(UIView*)menuButton;
@end @end
#endif // IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_VIEW_CONTROLLER_AUDIENCE_H_ #endif // IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_VIEW_CONTROLLER_AUDIENCE_H_
...@@ -9,7 +9,7 @@ ...@@ -9,7 +9,7 @@
@protocol DiscoverFeedMenuCommands @protocol DiscoverFeedMenuCommands
// Opens Discover feed control menu. // Opens Discover feed control menu.
- (void)openDiscoverFeedMenu:(UIView*)menuButton; - (void)openDiscoverFeedMenu;
// Sends a notification to indicate that the Discover feed has loaded, so the // Sends a notification to indicate that the Discover feed has loaded, so the
// IPH can be shown. // IPH can be shown.
......
...@@ -62,6 +62,10 @@ class WebState; ...@@ -62,6 +62,10 @@ class WebState;
// Tell location bar has taken focus. // Tell location bar has taken focus.
- (void)locationBarDidBecomeFirstResponder; - (void)locationBarDidBecomeFirstResponder;
// Constrains the named layout guide for the Discover header menu button.
- (void)constrainDiscoverHeaderMenuButtonNamedGuide;
@end @end
#endif // IOS_CHROME_BROWSER_UI_NTP_NEW_TAB_PAGE_COORDINATOR_H_ #endif // IOS_CHROME_BROWSER_UI_NTP_NEW_TAB_PAGE_COORDINATOR_H_
...@@ -72,9 +72,6 @@ ...@@ -72,9 +72,6 @@
- (void)stop { - (void)stop {
if (!self.started) if (!self.started)
return; return;
[self.viewController willMoveToParentViewController:nil];
[self.viewController.view removeFromSuperview];
[self.viewController removeFromParentViewController];
[self.contentSuggestionsCoordinator stop]; [self.contentSuggestionsCoordinator stop];
self.contentSuggestionsCoordinator = nil; self.contentSuggestionsCoordinator = nil;
self.incognitoViewController = nil; self.incognitoViewController = nil;
...@@ -121,10 +118,16 @@ ...@@ -121,10 +118,16 @@
- (void)locationBarDidBecomeFirstResponder { - (void)locationBarDidBecomeFirstResponder {
[self.contentSuggestionsCoordinator locationBarDidBecomeFirstResponder]; [self.contentSuggestionsCoordinator locationBarDidBecomeFirstResponder];
} }
- (void)locationBarDidResignFirstResponder { - (void)locationBarDidResignFirstResponder {
[self.contentSuggestionsCoordinator locationBarDidResignFirstResponder]; [self.contentSuggestionsCoordinator locationBarDidResignFirstResponder];
} }
- (void)constrainDiscoverHeaderMenuButtonNamedGuide {
[self.contentSuggestionsCoordinator
constrainDiscoverHeaderMenuButtonNamedGuide];
}
#pragma mark - LogoAnimationControllerOwnerOwner #pragma mark - LogoAnimationControllerOwnerOwner
- (id<LogoAnimationControllerOwner>)logoAnimationControllerOwner { - (id<LogoAnimationControllerOwner>)logoAnimationControllerOwner {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment