Commit b3a62810 authored by adamta's avatar adamta Committed by Commit Bot

[iOS] Show IPH bubble for Discover feed in right position

Corrects the location for the Discover feed header menu IPH bubble by
using the view instead of the button's frame. Adds an event for when
the Discover feed is loaded in order to add a server-side config for
the IPH.

Bug: 1124448, 1085419
Change-Id: I4df2919e428a5cdc429b6b157a34b82f80e3421b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2401971Reviewed-by: default avatarTommy Nyquist <nyquist@chromium.org>
Reviewed-by: default avatarSergio Collazos <sczs@chromium.org>
Commit-Queue: Adam Trudeau-Arcaro <adamta@google.com>
Cr-Commit-Position: refs/heads/master@{#805914}
parent 4f128089
...@@ -42,6 +42,7 @@ const char kClearedBrowsingData[] = "cleared_browsing_data"; ...@@ -42,6 +42,7 @@ const char kClearedBrowsingData[] = "cleared_browsing_data";
const char kViewedReadingList[] = "viewed_reading_list"; const char kViewedReadingList[] = "viewed_reading_list";
const char kTriggeredTranslateInfobar[] = "triggered_translate_infobar"; const char kTriggeredTranslateInfobar[] = "triggered_translate_infobar";
const char kBottomToolbarOpened[] = "bottom_toolbar_opened"; const char kBottomToolbarOpened[] = "bottom_toolbar_opened";
const char kDiscoverFeedLoaded[] = "discover_feed_loaded";
#endif // defined(OS_IOS) #endif // defined(OS_IOS)
} // namespace events } // namespace events
......
...@@ -75,6 +75,9 @@ extern const char kTriggeredTranslateInfobar[]; ...@@ -75,6 +75,9 @@ extern const char kTriggeredTranslateInfobar[];
// The user has viewed the the BottomToolbar tip. // The user has viewed the the BottomToolbar tip.
extern const char kBottomToolbarOpened[]; extern const char kBottomToolbarOpened[];
// The Discover feed has loaded content in the NTP.
extern const char kDiscoverFeedLoaded[];
#endif // defined(OS_IOS) #endif // defined(OS_IOS)
} // namespace events } // namespace events
......
...@@ -271,9 +271,14 @@ presentBubbleForFeature:(const base::Feature&)feature ...@@ -271,9 +271,14 @@ presentBubbleForFeature:(const base::Feature&)feature
BubbleArrowDirection arrowDirection = BubbleArrowDirectionDown; BubbleArrowDirection arrowDirection = BubbleArrowDirectionDown;
NSString* text = NSString* text =
l10n_util::GetNSStringWithFixup(IDS_IOS_DISCOVER_FEED_HEADER_IPH); l10n_util::GetNSStringWithFixup(IDS_IOS_DISCOVER_FEED_HEADER_IPH);
NamedGuide* guide = [NamedGuide guideWithName:kDiscoverFeedHeaderMenuGuide
view:self.rootViewController.view];
DCHECK(guide);
UIView* menuButton = guide.constrainedView;
CGPoint discoverFeedHeaderAnchor = CGPoint discoverFeedHeaderAnchor =
[self anchorPointToGuide:kDiscoverFeedHeaderMenuGuide [menuButton.superview convertPoint:menuButton.frame.origin toView:nil];
direction:arrowDirection]; discoverFeedHeaderAnchor.x += menuButton.frame.size.width / 2;
// If the feature engagement tracker does not consider it valid to display // If the feature engagement tracker does not consider it valid to display
// the new tab tip, then end early to prevent the potential reassignment // the new tab tip, then end early to prevent the potential reassignment
......
...@@ -37,6 +37,7 @@ source_set("content_suggestions") { ...@@ -37,6 +37,7 @@ source_set("content_suggestions") {
":feature_flags", ":feature_flags",
"//base", "//base",
"//components/favicon/core", "//components/favicon/core",
"//components/feature_engagement/public",
"//components/feed/core/shared_prefs:feed_shared_prefs", "//components/feed/core/shared_prefs:feed_shared_prefs",
"//components/ntp_snippets", "//components/ntp_snippets",
"//components/ntp_tiles", "//components/ntp_tiles",
...@@ -54,6 +55,7 @@ source_set("content_suggestions") { ...@@ -54,6 +55,7 @@ source_set("content_suggestions") {
"//ios/chrome/browser/discover_feed", "//ios/chrome/browser/discover_feed",
"//ios/chrome/browser/drag_and_drop", "//ios/chrome/browser/drag_and_drop",
"//ios/chrome/browser/favicon", "//ios/chrome/browser/favicon",
"//ios/chrome/browser/feature_engagement",
"//ios/chrome/browser/main:public", "//ios/chrome/browser/main:public",
"//ios/chrome/browser/metrics:metrics_internal", "//ios/chrome/browser/metrics:metrics_internal",
"//ios/chrome/browser/ntp", "//ios/chrome/browser/ntp",
......
...@@ -176,8 +176,7 @@ const CGFloat kFeedCardIPhoneWidth = 375; ...@@ -176,8 +176,7 @@ const CGFloat kFeedCardIPhoneWidth = 375;
[NamedGuide guideWithName:kDiscoverFeedHeaderMenuGuide [NamedGuide guideWithName:kDiscoverFeedHeaderMenuGuide
view:self.menuButton]; view:self.menuButton];
menuButtonGuide.constrainedFrame = menuButtonGuide.constrainedView = self.menuButton;
[self.contentView convertRect:self.menuButton.frame toView:nil];
self.discoverFeedVisible = [NSNumber numberWithBool:visible]; self.discoverFeedVisible = [NSNumber numberWithBool:visible];
} }
......
...@@ -8,6 +8,8 @@ ...@@ -8,6 +8,8 @@
#include "base/metrics/user_metrics.h" #include "base/metrics/user_metrics.h"
#include "base/metrics/user_metrics_action.h" #include "base/metrics/user_metrics_action.h"
#include "base/scoped_observer.h" #include "base/scoped_observer.h"
#import "components/feature_engagement/public/event_constants.h"
#import "components/feature_engagement/public/tracker.h"
#include "components/feed/core/shared_prefs/pref_names.h" #include "components/feed/core/shared_prefs/pref_names.h"
#include "components/ntp_snippets/content_suggestions_service.h" #include "components/ntp_snippets/content_suggestions_service.h"
#include "components/ntp_snippets/pref_names.h" #include "components/ntp_snippets/pref_names.h"
...@@ -23,6 +25,7 @@ ...@@ -23,6 +25,7 @@
#include "ios/chrome/browser/favicon/ios_chrome_large_icon_cache_factory.h" #include "ios/chrome/browser/favicon/ios_chrome_large_icon_cache_factory.h"
#include "ios/chrome/browser/favicon/ios_chrome_large_icon_service_factory.h" #include "ios/chrome/browser/favicon/ios_chrome_large_icon_service_factory.h"
#include "ios/chrome/browser/favicon/large_icon_cache.h" #include "ios/chrome/browser/favicon/large_icon_cache.h"
#import "ios/chrome/browser/feature_engagement/tracker_factory.h"
#import "ios/chrome/browser/main/browser.h" #import "ios/chrome/browser/main/browser.h"
#include "ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.h" #include "ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.h"
#include "ios/chrome/browser/ntp_tiles/ios_most_visited_sites_factory.h" #include "ios/chrome/browser/ntp_tiles/ios_most_visited_sites_factory.h"
...@@ -479,6 +482,12 @@ ...@@ -479,6 +482,12 @@
[self.alertCoordinator start]; [self.alertCoordinator start];
} }
- (void)notifyFeedLoadedForHeaderMenu {
feature_engagement::TrackerFactory::GetForBrowserState(
self.browser->GetBrowserState())
->NotifyEvent(feature_engagement::events::kDiscoverFeedLoaded);
}
#pragma mark - DiscoverFeedDelegate #pragma mark - DiscoverFeedDelegate
- (void)recreateDiscoverFeedViewController { - (void)recreateDiscoverFeedViewController {
......
...@@ -55,6 +55,9 @@ const CGFloat kDiscoverFeedContentWith = 430; ...@@ -55,6 +55,9 @@ const CGFloat kDiscoverFeedContentWith = 430;
const CGFloat kPaginationOffset = 400; const CGFloat kPaginationOffset = 400;
// Height for the Discover Feed section header. // Height for the Discover Feed section header.
const CGFloat kDiscoverFeedFeaderHeight = 30; const CGFloat kDiscoverFeedFeaderHeight = 30;
// Minimum height of the Discover feed content to indicate that the articles
// have loaded.
const CGFloat kDiscoverFeedLoadedHeight = 1000;
} }
NSString* const kContentSuggestionsMostVisitedAccessibilityIdentifierPrefix = NSString* const kContentSuggestionsMostVisitedAccessibilityIdentifierPrefix =
...@@ -816,6 +819,12 @@ NSString* const kContentSuggestionsMostVisitedAccessibilityIdentifierPrefix = ...@@ -816,6 +819,12 @@ NSString* const kContentSuggestionsMostVisitedAccessibilityIdentifierPrefix =
if (object == self.feedView && [keyPath isEqualToString:@"contentSize"]) { if (object == self.feedView && [keyPath isEqualToString:@"contentSize"]) {
// Reload the CollectionView data to adjust to the new Feed height. // Reload the CollectionView data to adjust to the new Feed height.
[self.collectionView reloadData]; [self.collectionView reloadData];
// Indicates that the feed articles have been loaded by checking its height.
// TODO(crbug.com/1126940): Use a callback from Mulder to determine this
// more reliably.
if (self.feedView.contentSize.height > kDiscoverFeedLoadedHeight) {
[self.discoverFeedMenuHandler notifyFeedLoadedForHeaderMenu];
}
} }
} }
......
...@@ -11,6 +11,10 @@ ...@@ -11,6 +11,10 @@
// Opens Discover feed control menu. // Opens Discover feed control menu.
- (void)openDiscoverFeedMenu:(UIView*)menuButton; - (void)openDiscoverFeedMenu:(UIView*)menuButton;
// Sends a notification to indicate that the Discover feed has loaded, so the
// IPH can be shown.
- (void)notifyFeedLoadedForHeaderMenu;
@end @end
#endif // IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_DISCOVER_FEED_MENU_COMMANDS_H_ #endif // IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_DISCOVER_FEED_MENU_COMMANDS_H_
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