Skip to content

Commit

Permalink
[iOS] Add flag to control feed header parameters
Browse files Browse the repository at this point in the history
Adds feature flag with 3 parameters to control the feed header.
1. Allows us to disable sticky feed header
2. Allows us to control feed header height
3. Moves the unread content dot feature within this flag

(cherry picked from commit f2b2244)

Bug: 1404737
Change-Id: I882c2ac71c90d727f59b165ae67b367b3f495ba4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4131175
Reviewed-by: Sergio Collazos <sczs@chromium.org>
Commit-Queue: Adam Arcaro <adamta@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1088310}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4134891
Auto-Submit: Adam Arcaro <adamta@google.com>
Commit-Queue: Sergio Collazos <sczs@chromium.org>
Cr-Commit-Position: refs/branch-heads/5481@{#129}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}
  • Loading branch information
adamta authored and Chromium LUCI CQ committed Jan 4, 2023
1 parent a1c66df commit ee309af
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 17 deletions.
4 changes: 1 addition & 3 deletions ios/chrome/browser/ui/ntp/feed_header_view_controller.mm
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@
const CGFloat kHeaderMenuButtonInsetTopAndBottom = 2;
const CGFloat kHeaderMenuButtonInsetSides = 2;
// The height of the header container. The content is unaffected.
// TODO(crbug.com/1277504): Only keep the WC header after launch.
const CGFloat kWebChannelsHeaderHeight = 52;
const CGFloat kDiscoverFeedHeaderHeight = 40;
const CGFloat kCustomSearchEngineLabelHeight = 18;
// * Values below are exclusive to Web Channels.
Expand Down Expand Up @@ -214,7 +212,7 @@ - (void)toggleBackgroundBlur:(BOOL)blurred animated:(BOOL)animated {

- (CGFloat)feedHeaderHeight {
return [self.feedControlDelegate isFollowingFeedAvailable]
? kWebChannelsHeaderHeight
? FollowingFeedHeaderHeight()
: kDiscoverFeedHeaderHeight;
}

Expand Down
17 changes: 5 additions & 12 deletions ios/chrome/browser/ui/ntp/new_tab_page_coordinator.mm
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,6 @@
#error "This file requires ARC support."
#endif

namespace {
// Flag to enable the checking of new content for the Follow Feed.
BASE_FEATURE(kEnableCheckForNewFollowContent,
"EnableCheckForNewFollowContent",
base::FEATURE_DISABLED_BY_DEFAULT);
} // namespace

@interface NewTabPageCoordinator () <AppStateObserver,
BooleanObserver,
ContentSuggestionsHeaderCommands,
Expand Down Expand Up @@ -423,7 +416,7 @@ - (void)constrainDiscoverHeaderMenuButtonNamedGuide {

- (void)updateFollowingFeedHasUnseenContent:(BOOL)hasUnseenContent {
if (![self isFollowingFeedAvailable] ||
!base::FeatureList::IsEnabled(kEnableCheckForNewFollowContent)) {
!IsDotEnabledForNewFollowedContent()) {
return;
}
if ([self doesFollowingFeedHaveContent]) {
Expand Down Expand Up @@ -885,7 +878,7 @@ - (void)handleFeedSelected:(FeedType)feedType {
// Saves scroll position before changing feed.
CGFloat scrollPosition = [self.ntpViewController scrollPosition];

if (feedType == FeedTypeFollowing) {
if (feedType == FeedTypeFollowing && IsDotEnabledForNewFollowedContent()) {
// Clears dot and notifies service that the Following feed content has
// been seen.
[self.feedHeaderViewController
Expand Down Expand Up @@ -1006,7 +999,8 @@ - (void)reloadContentSuggestions {
}

- (BOOL)isContentHeaderSticky {
return [self isFollowingFeedAvailable] && [self isFeedHeaderVisible];
return [self isFollowingFeedAvailable] && [self isFeedHeaderVisible] &&
!IsStickyHeaderDisabledForFollowingFeed();
}

- (void)signinPromoHasChangedVisibility:(BOOL)visible {
Expand Down Expand Up @@ -1495,8 +1489,7 @@ - (FeedHeaderViewController*)feedHeaderViewController {
DCHECK(!self.browser->GetBrowserState()->IsOffTheRecord());
if (!_feedHeaderViewController) {
BOOL followingSegmentDotVisible = NO;
if (base::FeatureList::IsEnabled(kEnableCheckForNewFollowContent) &&
IsWebChannelsEnabled()) {
if (IsDotEnabledForNewFollowedContent() && IsWebChannelsEnabled()) {
// Only show the dot if the user follows available publishers.
followingSegmentDotVisible =
[self doesFollowingFeedHaveContent] &&
Expand Down
15 changes: 15 additions & 0 deletions ios/chrome/browser/ui/ntp/new_tab_page_feature.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ BASE_DECLARE_FEATURE(kEnableCheckVisibilityOnAttentionLogStart);
// very short attention log.
BASE_DECLARE_FEATURE(kEnableRefineDataSourceReloadReporting);

// Flag to modify the feed header through the server. Enabling this feature on
// its own does nothing; relies on feature parameters.
BASE_DECLARE_FEATURE(kFeedHeaderSettings);

// Flag to override feed settings through the server. Enabling this feature on
// its own does nothing; relies on feature parameters.
BASE_DECLARE_FEATURE(kOverrideFeedSettings);
Expand Down Expand Up @@ -126,4 +130,15 @@ bool IsCheckVisibilityOnAttentionLogStartEnabled();
// attention log.
bool IsRefineDataSourceReloadReportingEnabled();

// YES if the Following feed header should not be sticky.
bool IsStickyHeaderDisabledForFollowingFeed();

// YES if a dot should appear to indicate that there is new content in the
// Following feed.
bool IsDotEnabledForNewFollowedContent();

// Returns a custom height for the Following feed header if it is overridden
// from the server, or returns the default value.
int FollowingFeedHeaderHeight();

#endif // IOS_CHROME_BROWSER_UI_NTP_NEW_TAB_PAGE_FEATURE_H_
30 changes: 28 additions & 2 deletions ios/chrome/browser/ui/ntp/new_tab_page_feature.mm
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,10 @@
"EnableRefineDataSourceReloadReporting",
base::FEATURE_DISABLED_BY_DEFAULT);

// Flag to override feed settings through the server. Enabling this feature on
// its own does nothing; relies on feature parameters.
BASE_FEATURE(kFeedHeaderSettings,
"FeedHeaderSettings",
base::FEATURE_DISABLED_BY_DEFAULT);

BASE_FEATURE(kOverrideFeedSettings,
"OverrideFeedSettings",
base::FEATURE_DISABLED_BY_DEFAULT);
Expand All @@ -80,6 +82,13 @@
const char kFollowingFeedDefaultSortTypeGroupedByPublisher[] =
"GroupedByPublisher";

// Feature parameters for `kFeedHeaderSettings`.
const char kEnableDotForNewFollowedContent[] =
"kEnableDotForNewFollowedContent";
const char kDisableStickyHeaderForFollowingFeed[] =
"DisableStickyHeaderForFollowingFeed";
const char kOverrideFeedHeaderHeight[] = "OverrideFeedHeaderHeight";

// Feature parameters for `kOverrideFeedSettings`.
const char kFeedSettingRefreshThresholdInSeconds[] =
"RefreshThresholdInSeconds";
Expand Down Expand Up @@ -147,3 +156,20 @@ bool IsCheckVisibilityOnAttentionLogStartEnabled() {
bool IsRefineDataSourceReloadReportingEnabled() {
return base::FeatureList::IsEnabled(kEnableRefineDataSourceReloadReporting);
}

bool IsStickyHeaderDisabledForFollowingFeed() {
return base::GetFieldTrialParamByFeatureAsBool(
kFeedHeaderSettings, kDisableStickyHeaderForFollowingFeed, false);
}

bool IsDotEnabledForNewFollowedContent() {
return base::GetFieldTrialParamByFeatureAsBool(
kFeedHeaderSettings, kEnableDotForNewFollowedContent, false);
}

int FollowingFeedHeaderHeight() {
int defaultWebChannelsHeaderHeight = 52;
return base::GetFieldTrialParamByFeatureAsInt(kFeedHeaderSettings,
kOverrideFeedHeaderHeight,
defaultWebChannelsHeaderHeight);
}

0 comments on commit ee309af

Please sign in to comment.