[Brave News]: Tweaks round 5 (#21516) (#21554)

* Migrate article channel names Hide sidebar on small screens PoC sidebar Lazy load sidebar Fix background Fix ad link Add articles with no images Create top bar Styling improvements Sidebar color tweaks Sidebar color tweaks Make buttons nicer * Same top level headings * Remove sidebar title * Monochrome news icon on peeking card * Fix peek button not showing on empty feeds * Heading cleanup
brave · Jan 14, 2024 · a1313ea · a1313ea
1 parent 1f704ec
commit a1313ea
Show file tree

Hide file tree

Showing 12 changed files with 219 additions and 93 deletions.
diff --git a/browser/ui/webui/brave_webui_source.cc b/browser/ui/webui/brave_webui_source.cc
@@ -232,10 +232,8 @@ void CustomizeWebUIHTMLSource(content::WebUI* web_ui,
         { "braveNewsNoContentActionLabel", IDS_BRAVE_NEWS_NO_CONTENT_ACTION_LABEL},  // NOLINT
         { "braveNewsPopularTitle", IDS_BRAVE_NEWS_POPULAR_TITLE},
         { "braveNewsNewsPeek", IDS_BRAVE_NEWS_NEWS_PEEK},
-        { "braveNewsMyFeedHeading", IDS_BRAVE_NEWS_MY_FEED_HEADING},
         { "braveNewsForYouFeed",IDS_BRAVE_NEWS_FOR_YOU_FEED},
         { "braveNewsFollowingFeed", IDS_BRAVE_NEWS_FOLLOWING_FEED},
-        { "braveNewsAddChannelsOrPublishers",IDS_BRAVE_NEWS_ADD_CHANNELS_OR_PUBLISHERS},
         { "braveNewsPublishersHeading", IDS_BRAVE_NEWS_PUBLISHERS_HEADING},
         { "braveNewsShowAll", IDS_BRAVE_NEWS_SHOW_ALL},
         { "braveNewsShowLess", IDS_BRAVE_NEWS_SHOW_LESS},

diff --git a/components/brave_new_tab_ui/components/default/braveNews/FeedV2.tsx b/components/brave_new_tab_ui/components/default/braveNews/FeedV2.tsx
@@ -4,19 +4,26 @@
 // You can obtain one at https://mozilla.org/MPL/2.0/.
 import Flex from '$web-common/Flex'
 import { getLocale } from '$web-common/locale'
-import Button from '@brave/leo/react/button'
 import Icon from '@brave/leo/react/icon'
 import { radius, spacing } from '@brave/leo/tokens/css'
 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 { CLASSNAME_PAGE_STUCK } from '../page'
+import SettingsButton from '../../../../brave_news/browser/resources/SettingsButton'
+import useMediaQuery from '$web-common/useMediaQuery'
+
+const SidebarMenu = React.lazy(() => import('./SidebarMenu'))
+const FeedNavigation = React.lazy(() => import('../../../../brave_news/browser/resources/FeedNavigation'))
+
+const isSmallQuery = '(max-width: 1024px)'
 
 const Root = styled(Variables)`
+  --bn-top-bar-height: 78px;
+
   padding-top: ${spacing.xl};
 
   display: grid;
@@ -50,17 +57,30 @@ const ButtonsContainer = styled.div`
     visibility: visible;
   }
 
-  display: flex;
-  gap: ${spacing.m};
   padding: ${spacing.m};
 
   background: var(--bn-glass-container);
+  backdrop-filter: blur(64px);
+
+  @media ${isSmallQuery} {
+    height: var(--bn-top-bar-height);
+
+    inset: 0;
+    bottom: unset;
+    padding: ${spacing['2Xl']} ${spacing.xl};
+    border-radius: 0;
+  }
 `
 
-const SettingsButton = styled(Button)`
-  --leo-button-color: var(--bn-glass-50);
-  --leo-button-radius: ${radius.s};
-  --leo-button-padding: ${spacing.s};
+const ButtonSpacer = styled.div`
+  max-width: min(540px, 100vw);
+
+  display: flex;
+  justify-content: flex-end;
+  gap: ${spacing.m};
+
+  margin-left: auto;
+  margin-right: auto;
 `
 
 const LoadNewContentButton = styled(NewsButton)`
@@ -69,9 +89,15 @@ const LoadNewContentButton = styled(NewsButton)`
   top: ${spacing['3Xl']};
 
   flex-grow: 0;
+
+  @media ${isSmallQuery} {
+    top: calc(var(--bn-top-bar-height) + var(--leo-spacing-m));
+  }
 `
 
 export default function FeedV2() {
+  const isSmall = useMediaQuery(isSmallQuery)
+
   const { feedV2, setCustomizePage, refreshFeedV2, feedV2UpdatesAvailable } = useBraveNews()
   const ref = React.useRef<HTMLDivElement>()
 
@@ -88,7 +114,7 @@ export default function FeedV2() {
 
   return <Root ref={ref as any} data-theme="dark">
     <SidebarContainer>
-      <FeedNavigation />
+      {!isSmall && <React.Suspense fallback={null}><FeedNavigation /></React.Suspense>}
     </SidebarContainer>
     <Flex align='center' direction='column' gap={spacing.l}>
       {feedV2UpdatesAvailable && <LoadNewContentButton onClick={refreshFeedV2}>
@@ -98,12 +124,15 @@ export default function FeedV2() {
     </Flex>
 
     <ButtonsContainer>
-      <SettingsButton fab kind='outline' onClick={() => setCustomizePage('news')} title={getLocale('braveNewsCustomizeFeed')}>
-        <Icon name="tune" />
-      </SettingsButton>
-      <SettingsButton fab isLoading={!feedV2} kind='outline' title={getLocale('braveNewsRefreshFeed')} onClick={() => {
-        refreshFeedV2()
-      }}><Icon name="refresh" /></SettingsButton>
+      <ButtonSpacer>
+        {isSmall && <React.Suspense fallback={null}><SidebarMenu /></React.Suspense>}
+        <SettingsButton onClick={() => setCustomizePage('news')} title={getLocale('braveNewsCustomizeFeed')}>
+          <Icon name="tune" />
+        </SettingsButton>
+        <SettingsButton isLoading={!feedV2} title={getLocale('braveNewsRefreshFeed')} onClick={() => {
+          refreshFeedV2()
+        }}><Icon name="refresh" /></SettingsButton>
+      </ButtonSpacer>
     </ButtonsContainer>
   </Root>
 }
diff --git a/components/brave_new_tab_ui/components/default/braveNews/SidebarMenu.tsx b/components/brave_new_tab_ui/components/default/braveNews/SidebarMenu.tsx
@@ -0,0 +1,60 @@
+// Copyright (c) 2024 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 * as React from 'react';
+import styled from 'styled-components';
+import FeedNavigation from '../../../../brave_news/browser/resources/FeedNavigation';
+import SettingsButton from '../../../../brave_news/browser/resources/SettingsButton';
+import Icon from '@brave/leo/react/icon';
+
+const Container = styled.dialog`
+  top: var(--bn-top-bar-height);
+  height: calc(100vh - var(--bn-top-bar-height));
+
+  overflow: hidden;
+
+  margin-left: 0;
+  margin-top: 0;
+  padding: 0;
+
+  position: fixed;
+  display: flex;
+
+  transition: transform 0.2s ease-in-out;
+  transform: translateX(-100%);
+
+  background: var(--bn-glass-card);
+  backdrop-filter: blur(64px);
+  border: none;
+  outline: none;
+
+  &[open] {
+    transform: translateX(0);
+  }
+
+  &> div {
+    background: unset;
+  }
+`
+
+const MenuButton = styled(SettingsButton)`
+  margin-right: auto;
+`
+
+export default function SidebarMenu() {
+  const dialogRef = React.useRef<HTMLDialogElement>()
+  return <MenuButton onClick={() => dialogRef.current?.showModal()}>
+    <Icon name="hamburger-menu" />
+    <Container ref={dialogRef as any} onClick={e => {
+      // Close the menu on click outside.
+      const bounds = e.currentTarget.getBoundingClientRect()
+      if (e.clientX < bounds.x || e.clientY < bounds.y || e.clientX > bounds.right || e.clientY > bounds.bottom) {
+        e.currentTarget.close()
+      }
+    }}>
+      <FeedNavigation />
+    </Container>
+  </MenuButton>
+}
diff --git a/components/brave_news/browser/combined_feed_parsing.cc b/components/brave_news/browser/combined_feed_parsing.cc
@@ -9,16 +9,18 @@
 #include <utility>
 #include <vector>
 
+#include "base/feature_list.h"
 #include "base/logging.h"
 #include "base/notreached.h"
 #include "base/strings/strcat.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "base/types/expected.h"
 #include "brave/components/brave_news/api/combined_feed.h"
+#include "brave/components/brave_news/browser/channel_migrator.h"
 #include "brave/components/brave_news/common/brave_news.mojom-forward.h"
-#include "brave/components/brave_news/common/brave_news.mojom-shared.h"
 #include "brave/components/brave_news/common/brave_news.mojom.h"
+#include "brave/components/brave_news/common/features.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
 #include "ui/base/l10n/time_format.h"
 #include "url/gurl.h"
@@ -49,9 +51,13 @@ base::expected<mojom::FeedItemPtr, std::string> ParseFeedItem(
         base::StrCat({"Item url was not HTTP or HTTPS: url=", url.spec()}));
   }
 
-  if (feed_item.padded_img.empty()) {
-    return base::unexpected(base::StrCat(
-        {"Found feed item with missing image. url=", feed_item.url}));
+  // FeedV2 supports articles with no images, such as the ones from Brave Blog.
+  if (!base::FeatureList::IsEnabled(
+          brave_news::features::kBraveNewsFeedUpdate)) {
+    if (feed_item.padded_img.empty()) {
+      return base::unexpected(base::StrCat(
+          {"Found feed item with missing image. url=", feed_item.url}));
+    }
   }
 
   if (feed_item.publisher_id.empty()) {
@@ -70,7 +76,7 @@ base::expected<mojom::FeedItemPtr, std::string> ParseFeedItem(
   }
 
   auto metadata = mojom::FeedItemMetadata::New();
-  metadata->category_name = feed_item.category;
+  metadata->category_name = GetMigratedChannel(feed_item.category);
   metadata->title = feed_item.title;
   metadata->description = feed_item.description;
   metadata->publisher_id = feed_item.publisher_id;

diff --git a/components/brave_news/browser/resources/FeedNavigation.tsx b/components/brave_news/browser/resources/FeedNavigation.tsx
@@ -12,6 +12,7 @@ import { useBraveNews } from './shared/Context';
 import { isPublisherEnabled } from './shared/api';
 import { FeedView } from './shared/useFeedV2';
 import { getLocale } from '$web-common/locale';
+import SettingsButton from './SettingsButton';
 
 const DEFAULT_SHOW_COUNT = 4;
 
@@ -32,15 +33,7 @@ const Container = styled(Card)`
   scrollbar-color: var(--bn-glass-10) var(--bn-glass-10);
 `
 
-const Heading = styled.h3`
-  font: ${font.default.semibold};
-  color: var(--bn-glass-25);
-  margin: 0;
-
-  padding-left: ${PAD_LEFT};
-`
-
-const CustomButton = styled.button <{ selected?: boolean, faint?: boolean, bold?: boolean }>`
+const CustomButton = styled.button <{ selected?: boolean, faint?: boolean, large?: boolean, bold?: boolean }>`
   padding: ${spacing.m};
   padding-left: ${PAD_LEFT};
 
@@ -52,8 +45,8 @@ const CustomButton = styled.button <{ selected?: boolean, faint?: boolean, bold?
   text-align: left;
   width: 100%;
 
-  color: ${p => p.faint ? `var(--bn-glass-25)` : `var(--bn-glass-70)`};
-  font: ${p => font.small[p.bold ? 'semibold' : 'regular']};
+  color: ${p => p.faint ? `var(--bn-glass-50)` : `var(--bn-glass-100)`};
+  font: ${p => font[p.large ? 'default' : 'small'][p.bold ? 'semibold' : 'regular']};
   cursor: pointer;
 
   &:hover {
@@ -79,20 +72,15 @@ const Section = styled.details`
     align-items: center;
     gap: ${spacing.m};
     list-style: none;
-    font: ${font.small.semibold};
+    font: ${font.default.semibold};
 
     cursor: pointer;
 
     :focus-visible {
       box-shadow: ${effect.focusState};
     }
 
-    ${CustomButton} {
-      padding: 0;
-      flex: 0;
-      display: flex;
-      gap: ${spacing.m};
-      align-items: center;
+    ${SettingsButton} {
       margin-left: auto;
     }
   }
@@ -118,11 +106,12 @@ function usePersistedState<T>(name: string, defaultValue: T) {
 }
 
 const Marker = <Icon name='arrow-small-right' className='marker' />
+const PlaceholderMarker = <Icon />
 
 export function Item(props: { id: FeedView, name: string }) {
   const { feedView, setFeedView } = useBraveNews()
-
-  return <CustomButton selected={props.id === feedView} onClick={() => setFeedView(props.id)} bold={props.id === 'all'}>
+  const topLevel = ['all', 'following'].includes(props.id)
+  return <CustomButton large={topLevel} selected={props.id === feedView} onClick={() => setFeedView(props.id)} bold={topLevel}>
     {props.name}
   </CustomButton>
 }
@@ -147,17 +136,18 @@ export default function Sidebar() {
     .slice(0, showingMoreChannels ? undefined : DEFAULT_SHOW_COUNT), [subscribedChannels, showingMoreChannels])
 
   return <Container>
-    <Heading>{getLocale('braveNewsMyFeedHeading')}</Heading>
     <Item id='all' name={getLocale('braveNewsForYouFeed')} />
     <Item id='following' name={getLocale('braveNewsFollowingFeed')} />
     {!!subscribedChannels.length && <Section open>
       <summary>
-        {Marker}
+        {subscribedChannels.length ? Marker : PlaceholderMarker}
         {getLocale('braveNewsChannelsHeader')}
-        <CustomButton faint onClick={() => setCustomizePage('news')}>
+        <SettingsButton size="tiny" onClick={e => {
+          setCustomizePage('news')
+          e.stopPropagation()
+        }}>
           <Icon name='plus-add' />
-          {getLocale('braveNewsAddChannelsOrPublishers')}
-        </CustomButton>
+        </SettingsButton>
       </summary>
       {slicedChannelIds.map(c => <Item key={c} id={`channels/${c}`} name={c} />)}
       {subscribedChannels.length > DEFAULT_SHOW_COUNT
@@ -169,12 +159,14 @@ export default function Sidebar() {
     </Section>}
     {!!subscribedPublisherIds.length && <Section open>
       <summary>
-        {Marker}
+        {subscribedPublisherIds.length ? Marker : PlaceholderMarker}
         {getLocale('braveNewsPublishersHeading')}
-        <CustomButton faint onClick={() => setCustomizePage('popular')}>
+        <SettingsButton size="tiny" onClick={e => {
+          setCustomizePage('popular')
+          e.stopPropagation()
+        }}>
           <Icon name='plus-add' />
-          {getLocale('braveNewsAddChannelsOrPublishers')}
-        </CustomButton>
+        </SettingsButton>
       </summary>
       {slicedPublisherIds.map(p => <Item key={p} id={`publishers/${p}`} name={publishers[p]?.publisherName} />)}
       {subscribedPublisherIds.length > DEFAULT_SHOW_COUNT

diff --git a/components/brave_news/browser/resources/Peek.tsx b/components/brave_news/browser/resources/Peek.tsx
@@ -3,17 +3,17 @@
 // 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 { getLocale } from '$web-common/locale';
 import Icon from '@brave/leo/react/icon';
-import { color, effect, gradient, radius, spacing } from '@brave/leo/tokens/css';
+import { color, effect, font, radius, spacing } from '@brave/leo/tokens/css';
 import * as React from 'react';
 import styled, { keyframes } from 'styled-components';
 import { NEWS_FEED_CLASS } from './Feed';
+import Variables from './Variables';
 import { MetaInfo } from './feed/ArticleMetaRow';
 import Card, { SmallImage, Title } from './feed/Card';
 import { useBraveNews } from './shared/Context';
 import { useUnpaddedImageUrl } from './shared/useUnpaddedImageUrl';
-import Variables from './Variables';
-import { getLocale } from '$web-common/locale'
 
 const NewsButton = styled.button`
   cursor: pointer;
@@ -27,10 +27,7 @@ const NewsButton = styled.button`
   backdrop-filter: blur(40px);
 
   color: ${color.white};
-
-  & > leo-icon[name="news-default"] {
-    --leo-icon-color: ${gradient.iconsActive};
-  }
+  font: ${font.default.semibold};
 
   & > leo-icon[name="carat-down"] {
     --leo-icon-color: rgba(255, 255, 255, 0.25);
@@ -114,7 +111,7 @@ export default function Peek() {
 
   return isShowOnNTPPrefEnabled
     ? <Container>
-      {(!isOptInPrefEnabled || data) && <NewsButton onClick={scrollToNews}>
+      {(!isOptInPrefEnabled || feedV2) && <NewsButton onClick={scrollToNews}>
         <Icon name='news-default' />
         {getLocale('braveNewsNewsPeek')}
         <Icon name='carat-down' />