Uplift of #21439 (squashed) to beta

brave · Dec 21, 2023 · 65f50d3 · 65f50d3
1 parent 632ccd5
commit 65f50d3
Show file tree

Hide file tree

Showing 13 changed files with 168 additions and 67 deletions.
diff --git a/components/brave_new_tab_ui/components/default/braveNews/FeedV2.tsx b/components/brave_new_tab_ui/components/default/braveNews/FeedV2.tsx
@@ -11,8 +11,10 @@ import * as React from 'react'
 import styled from 'styled-components'
 import Feed from '../../../../brave_news/browser/resources/Feed'
 import FeedNavigation from '../../../../brave_news/browser/resources/FeedNavigation'
+import NewsButton from '../../../../brave_news/browser/resources/NewsButton'
 import Variables from '../../../../brave_news/browser/resources/Variables'
 import { useBraveNews } from '../../../../brave_news/browser/resources/shared/Context'
+import { isPublisherEnabled } from '../../../../brave_news/browser/resources/shared/api'
 import { CLASSNAME_PAGE_STUCK } from '../page'
 
 const Root = styled(Variables)`
@@ -56,19 +58,13 @@ const ButtonsContainer = styled.div`
   background: var(--bn-glass-container);
 `
 
-const NewsButton = styled(Button)`
+const SettingsButton = styled(Button)`
   --leo-button-color: var(--bn-glass-50);
   --leo-button-radius: ${radius.s};
   --leo-button-padding: ${spacing.m};
 `
 
-const LoadNewContentButton = styled(Button)`
-  --leo-button-color: var(--bn-glass-10);
-
-  border-radius: 20px;
-  overflow: hidden;
-  backdrop-filter: brightness(0.8) blur(32px);
-
+const LoadNewContentButton = styled(NewsButton)`
   position: fixed;
   z-index: 1;
   top: ${spacing['3Xl']};
@@ -77,7 +73,22 @@ const LoadNewContentButton = styled(Button)`
 `
 
 export default function FeedV2() {
-  const { feedV2, setCustomizePage, refreshFeedV2, feedV2UpdatesAvailable } = useBraveNews()
+  const { feedV2, setCustomizePage, refreshFeedV2, feedV2UpdatesAvailable, publishers, channels } = useBraveNews()
+
+  // We don't want to decide whether we have subscriptions until the publishers
+  // and channels have loaded.
+  const loaded = React.useMemo(() => !!Object.values(publishers).length && !!Object.values(channels).length, [publishers, channels])
+
+  // This is a bit of an interesting |useMemo| - we only want it to be updated
+  // when the feed changes so as to not break the case where:
+  // 1. The user has no feeds (we show the NoFeeds card)
+  // 2. The user subscribes to a feed (we should still show the NoFeeds card,
+  //    not the "Empty Feed")
+  // To achieve this, |hasSubscriptions| is only updated when the feed changes,
+  // or the opt-in status is changed.
+  const hasSubscriptions = React.useMemo(() => !loaded
+    || Object.values(publishers).some(isPublisherEnabled)
+    || Object.values(channels).some(c => c.subscribedLocales.length), [feedV2, loaded])
 
   const ref = React.useRef<HTMLDivElement>()
 
@@ -92,15 +103,6 @@ export default function FeedV2() {
     ref.current?.scrollIntoView()
   }, [feedV2?.items])
 
-  // For some reason |createGlobalStyle| doesn't seem to work in Brave Core
-  // To get the background blur effect looking nice, we need to set the body
-  // background to black - unfortunately we can't do this in root HTML file
-  // because we want to avoid the background flash effect.
-  React.useEffect(() => {
-    // Note: This is always black because this doesn't support light mode.
-    document.body.style.backgroundColor = 'black';
-  }, [])
-
   return <Root ref={ref as any} data-theme="dark">
     <SidebarContainer>
       <FeedNavigation />
@@ -109,16 +111,16 @@ export default function FeedV2() {
       {feedV2UpdatesAvailable && <LoadNewContentButton onClick={refreshFeedV2}>
         {getLocale('braveNewsNewContentAvailable')}
       </LoadNewContentButton>}
-      <Feed feed={feedV2} />
+      <Feed feed={feedV2} hasSubscriptions={hasSubscriptions} />
     </Flex>
 
     <ButtonsContainer>
-      <NewsButton fab kind='outline' onClick={() => setCustomizePage('news')} title={getLocale('braveNewsCustomizeFeed')}>
+      <SettingsButton fab kind='outline' onClick={() => setCustomizePage('news')} title={getLocale('braveNewsCustomizeFeed')}>
         <Icon name="settings" />
-      </NewsButton>
-      <NewsButton fab isLoading={!feedV2} kind='outline' title={getLocale('braveNewsRefreshFeed')} onClick={() => {
+      </SettingsButton>
+      <SettingsButton fab isLoading={!feedV2} kind='outline' title={getLocale('braveNewsRefreshFeed')} onClick={() => {
         refreshFeedV2()
-      }}><Icon name="refresh" /></NewsButton>
+      }}><Icon name="refresh" /></SettingsButton>
     </ButtonsContainer>
   </Root>
 }
diff --git a/components/brave_new_tab_ui/components/default/braveNews/cards/cardOptIn.tsx b/components/brave_new_tab_ui/components/default/braveNews/cards/cardOptIn.tsx
@@ -8,6 +8,7 @@ import { getLocale, getLocaleWithTag } from '../../../../../common/locale'
 import * as Card from '../cardIntro'
 import BraveNewsLogoUrl from '../braveNewsLogo.svg'
 import { CardButton, TertiaryButton } from '../default'
+import { NEWS_FEED_CLASS } from '../../../../../brave_news/browser/resources/Feed'
 
 type Props = {
   onOptIn: () => unknown
@@ -19,7 +20,7 @@ const descriptionTwoTextParts = getLocaleWithTag('braveNewsIntroDescriptionTwo')
 export default function IntroCard (props: Props) {
   const introElementRef = React.useRef(null)
   return (
-    <Card.Intro ref={introElementRef}>
+    <Card.Intro ref={introElementRef} className={NEWS_FEED_CLASS}>
       <Card.Image src={BraveNewsLogoUrl} />
       <Card.Title>{getLocale('braveNewsIntroTitle')}</Card.Title>
       <div>

diff --git a/components/brave_news/browser/brave_news_controller.cc b/components/brave_news/browser/brave_news_controller.cc
@@ -307,6 +307,12 @@ void BraveNewsController::SetChannelSubscribed(
     SetChannelSubscribedCallback callback) {
   auto result =
       channels_controller_.SetChannelSubscribed(locale, channel_id, subscribed);
+
+  // When channels are changed, see if it affects the feed.
+  if (MaybeInitFeedV2()) {
+    feed_v2_builder_->RecheckFeedHash();
+  }
+
   std::move(callback).Run(std::move(result));
 }
 

diff --git a/components/brave_news/browser/channels_controller.cc b/components/brave_news/browser/channels_controller.cc
@@ -172,6 +172,11 @@ bool ChannelsController::GetChannelSubscribed(const std::string& locale,
 }
 
 void ChannelsController::OnPublishersUpdated(PublishersController* controller) {
+  // Don't notify updates until the pref is initialized.
+  if (!prefs_->HasPrefPath(prefs::kBraveNewsChannels)) {
+    return;
+  }
+
   GetAllChannels(base::BindOnce(
       [](ChannelsController* controller, Channels channels) {
         auto event = mojom::ChannelsEvent::New();

diff --git a/components/brave_news/browser/feed_v2_builder.cc b/components/brave_news/browser/feed_v2_builder.cc
@@ -85,9 +85,16 @@ std::string GetFeedHash(const Channels& channels,
   }
 
   for (const auto& [id, publisher] : publishers) {
-    if (publisher->user_enabled_status == mojom::UserEnabled::ENABLED) {
+    if (publisher->user_enabled_status == mojom::UserEnabled::ENABLED ||
+        publisher->type == mojom::PublisherType::DIRECT_SOURCE) {
       hash_items.push_back(id);
     }
+
+    // Disabling a publisher should also change the hash, as it will affect what
+    // articles can be shown.
+    if (publisher->user_enabled_status == mojom::UserEnabled::DISABLED) {
+      hash_items.push_back(id + "_disabled");
+    }
   }
 
   for (const auto& [region, etag] : etags) {
@@ -918,8 +925,7 @@ void FeedV2Builder::GetSignals(GetSignalsCallback callback) {
                  weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
-void FeedV2Builder::OnPublishersUpdated(
-    PublishersController* publishers_controller) {
+void FeedV2Builder::RecheckFeedHash() {
   const auto& publishers = publishers_controller_->GetLastPublishers();
   auto channels =
       channels_controller_->GetChannelsFromPublishers(publishers, &*prefs_);
@@ -929,6 +935,11 @@ void FeedV2Builder::OnPublishersUpdated(
   }
 }
 
+void FeedV2Builder::OnPublishersUpdated(
+    PublishersController* publishers_controller) {
+  RecheckFeedHash();
+}
+
 void FeedV2Builder::UpdateData(UpdateSettings settings,
                                base::OnceCallback<void()> callback) {
   if (current_update_) {

diff --git a/components/brave_news/browser/feed_v2_builder.h b/components/brave_news/browser/feed_v2_builder.h
@@ -59,6 +59,8 @@ class FeedV2Builder : public PublishersController::Observer {
 
   void GetSignals(GetSignalsCallback callback);
 
+  void RecheckFeedHash();
+
   // PublishersController::Observer:
   void OnPublishersUpdated(PublishersController* controller) override;
 

diff --git a/components/brave_news/browser/resources/Feed.tsx b/components/brave_news/browser/resources/Feed.tsx
@@ -3,19 +3,20 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this file,
 // You can obtain one at https://mozilla.org/MPL/2.0/.
 
+import { spacing } from "@brave/leo/tokens/css";
 import { FeedItemV2, FeedV2 } from "gen/brave/components/brave_news/common/brave_news.mojom.m";
 import * as React from 'react';
 import styled from "styled-components";
 import Advert from "./feed/Ad";
 import Article from "./feed/Article";
+import CaughtUp from "./feed/CaughtUp";
 import Cluster from "./feed/Cluster";
 import Discover from "./feed/Discover";
 import HeroArticle from "./feed/Hero";
-import { getHistoryValue, setHistoryState } from "./shared/history";
-import NoArticles from "./feed/NoArticles";
 import LoadingCard from "./feed/LoadingCard";
-import { spacing } from "@brave/leo/tokens/css";
-import CaughtUp from "./feed/CaughtUp";
+import NoArticles from "./feed/NoArticles";
+import NoFeeds from "./feed/NoFeeds";
+import { getHistoryValue, setHistoryState } from "./shared/history";
 
 // Restoring scroll position is complicated - we have two available strategies:
 // 1. Scroll to the same position - as long as the window hasn't been resized,
@@ -44,6 +45,7 @@ const FeedContainer = styled.div`
 
 interface Props {
   feed: FeedV2 | undefined;
+  hasSubscriptions: boolean;
 }
 
 const getKey = (feedItem: FeedItemV2, index: number): React.Key => {
@@ -74,7 +76,7 @@ const saveScrollPos = (itemId: React.Key) => () => {
   })
 }
 
-export default function Component({ feed }: Props) {
+export default function Component({ feed, hasSubscriptions }: Props) {
   const [cardCount, setCardCount] = React.useState(getHistoryValue(HISTORY_CARD_COUNT, PAGE_SIZE));
 
   // Store the number of cards we've loaded in history - otherwise when we
@@ -144,12 +146,15 @@ export default function Component({ feed }: Props) {
   }, [cardCount, feed?.items])
 
   return <FeedContainer className={NEWS_FEED_CLASS}>
-    {feed
-      ? !feed.items.length ? <NoArticles />
-      : <>
-          {cards}
-          <CaughtUp />
-      </>
-      : <LoadingCard />}
+    {!hasSubscriptions
+      ? <NoFeeds />
+      : feed
+        ? !feed.items.length
+          ? <NoArticles />
+          : <>
+            {cards}
+            <CaughtUp />
+          </>
+        : <LoadingCard />}
   </FeedContainer>
 }
diff --git a/components/brave_news/browser/resources/FeedPage.tsx b/components/brave_news/browser/resources/FeedPage.tsx
@@ -23,6 +23,6 @@ export default function FeedPage() {
   const { feedV2 } = useBraveNews()
   return <Container>
     <h2>The Feed ({feedV2?.items.length} items. Truncated at {truncate})</h2>
-    <Feed feed={feedV2} />
+    <Feed feed={feedV2} hasSubscriptions />
   </Container>
 }
diff --git a/components/brave_news/browser/resources/NewsButton.tsx b/components/brave_news/browser/resources/NewsButton.tsx
@@ -0,0 +1,14 @@
+// Copyright (c) 2023 The Brave Authors. All rights reserved.
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this file,
+// You can obtain one at https://mozilla.org/MPL/2.0/.
+import Button from '@brave/leo/react/button'
+import styled from 'styled-components'
+
+export default styled(Button)`
+  --leo-button-color: var(--bn-glass-10);
+
+  border-radius: 20px;
+  overflow: hidden;
+  backdrop-filter: brightness(0.8) blur(32px);
+`
diff --git a/components/brave_news/browser/resources/Peek.tsx b/components/brave_news/browser/resources/Peek.tsx
@@ -98,26 +98,35 @@ const scrollToNews = () => {
 }
 
 export default function Peek() {
-  const { feedV2 } = useBraveNews()
+  const { feedV2, isShowOnNTPPrefEnabled, isOptInPrefEnabled } = useBraveNews()
   const top = feedV2?.items?.find(a => a.article || a.hero)
   const data = (top?.hero ?? top?.article)?.data
   const imageUrl = useUnpaddedImageUrl(data?.image.paddedImageUrl?.url ?? data?.image.imageUrl?.url, undefined, true)
 
-  if (!data) return null
-
-  return <Container>
-    <NewsButton onClick={scrollToNews}>
-      <Icon name='news-default' />
-      {getLocale('braveNewsNewsPeek')}
-      <Icon name='carat-down' />
-    </NewsButton>
-    <PeekingCard onClick={scrollToNews}>
-      <div>
-        <MetaInfo article={data} />
-        <Title>{data.title}</Title>
-      </div>
-      <SmallImage src={imageUrl} />
-    </PeekingCard>
-  </Container>
+  // For some reason |createGlobalStyle| doesn't seem to work in Brave Core
+  // To get the background blur effect looking nice, we need to set the body
+  // background to black - unfortunately we can't do this in root HTML file
+  // because we want to avoid the background flash effect.
+  React.useEffect(() => {
+    // Note: This is always black because this doesn't support light mode.
+    document.body.style.backgroundColor = 'black';
+  }, [])
+
+  return isShowOnNTPPrefEnabled
+    ? <Container>
+      {(!isOptInPrefEnabled || data) && <NewsButton onClick={scrollToNews}>
+        <Icon name='news-default' />
+        {getLocale('braveNewsNewsPeek')}
+        <Icon name='carat-down' />
+      </NewsButton>}
+      {data && <PeekingCard onClick={scrollToNews}>
+        <div>
+          <MetaInfo article={data} />
+          <Title>{data.title}</Title>
+        </div>
+        <SmallImage src={imageUrl} />
+      </PeekingCard>}
+    </Container>
+    : null
 }
 
diff --git a/components/brave_news/browser/resources/feed/NoArticles.tsx b/components/brave_news/browser/resources/feed/NoArticles.tsx
@@ -36,24 +36,24 @@ export default function NoArticles() {
       <rect x="9.5" y="75" width="64" height="4" rx="2" fill="url(#paint4_linear_5051_10730)" />
       <defs>
         <linearGradient id="paint0_linear_5051_10730" x1="49.5" y1="1" x2="49.5" y2="78.5" gradientUnits="userSpaceOnUse">
-          <stop stop-color="white" stop-opacity="0.15" />
-          <stop offset="1" stop-color="white" stop-opacity="0" />
+          <stop stopColor="white" stopOpacity="0.15" />
+          <stop offset="1" stopColor="white" stopOpacity="0" />
         </linearGradient>
         <linearGradient id="paint1_linear_5051_10730" x1="9.5" y1="33" x2="89.5" y2="33" gradientUnits="userSpaceOnUse">
-          <stop stop-color="white" stop-opacity="0.03" />
-          <stop offset="1" stop-color="white" stop-opacity="0.2" />
+          <stop stopColor="white" stopOpacity="0.03" />
+          <stop offset="1" stopColor="white" stopOpacity="0.2" />
         </linearGradient>
         <linearGradient id="paint2_linear_5051_10730" x1="9.5" y1="71.0426" x2="89.5" y2="71.0425" gradientUnits="userSpaceOnUse">
-          <stop stop-color="white" stop-opacity="0.03" />
-          <stop offset="1" stop-color="white" stop-opacity="0.2" />
+          <stop stopColor="white" stopOpacity="0.03" />
+          <stop offset="1" stopColor="white" stopOpacity="0.2" />
         </linearGradient>
         <linearGradient id="paint3_linear_5051_10730" x1="9.5" y1="60.0213" x2="57.5" y2="60.0213" gradientUnits="userSpaceOnUse">
-          <stop stop-color="white" stop-opacity="0.03" />
-          <stop offset="1" stop-color="white" stop-opacity="0.2" />
+          <stop stopColor="white" stopOpacity="0.03" />
+          <stop offset="1" stopColor="white" stopOpacity="0.2" />
         </linearGradient>
         <linearGradient id="paint4_linear_5051_10730" x1="9.5" y1="77.0426" x2="73.5" y2="77.0425" gradientUnits="userSpaceOnUse">
-          <stop stop-color="white" stop-opacity="0.03" />
-          <stop offset="1" stop-color="white" stop-opacity="0.2" />
+          <stop stopColor="white" stopOpacity="0.03" />
+          <stop offset="1" stopColor="white" stopOpacity="0.2" />
         </linearGradient>
       </defs>
     </svg>