From 00b0ed708bc15f3e454f07e94f750595c649260e Mon Sep 17 00:00:00 2001 From: Jagadesh P Date: Sun, 16 Jun 2024 02:24:00 +0530 Subject: [PATCH 1/2] [BraveNews] Refactor brave news's button and dialog and fix dialog not getting closed while clicking that button. Basically this issue is getting fixed when we use |OnWidgetDestroyed| method of |BraveNewsBubbleView| to reset the current bubble(slower) instead of depending on |SetCloseCallback|(faster). Refer: https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/ui/views/location_bar/icon_label_bubble_view.h;l=291;drc=f2263729318bf6737e153bc27e70ab3379a789b4 --- browser/ui/BUILD.gn | 6 +- .../brave_news_bubble_controller.cc | 60 +++++++++++++++++++ .../brave_news/brave_news_bubble_controller.h | 51 ++++++++++++++++ .../brave_news/brave_news_bubble_view.cc | 14 +++++ .../views/brave_news/brave_news_bubble_view.h | 8 ++- .../brave_news_location_view.cc | 47 ++++++++------- .../brave_news_location_view.h | 19 ++++-- .../location_bar/brave_location_bar_view.cc | 2 +- .../location_bar/brave_location_bar_view.h | 2 +- 9 files changed, 176 insertions(+), 33 deletions(-) create mode 100644 browser/ui/views/brave_news/brave_news_bubble_controller.cc create mode 100644 browser/ui/views/brave_news/brave_news_bubble_controller.h rename browser/ui/views/{location_bar => brave_news}/brave_news_location_view.cc (87%) rename browser/ui/views/{location_bar => brave_news}/brave_news_location_view.h (82%) diff --git a/browser/ui/BUILD.gn b/browser/ui/BUILD.gn index c8e8198a7c2a..25476512970a 100644 --- a/browser/ui/BUILD.gn +++ b/browser/ui/BUILD.gn @@ -1050,6 +1050,10 @@ source_set("ui") { "views/brave_help_bubble/brave_help_bubble_delegate_view.h", "views/brave_help_bubble/brave_help_bubble_host_view.cc", "views/brave_help_bubble/brave_help_bubble_host_view.h", + "views/brave_news/brave_news_bubble_controller.cc", + "views/brave_news/brave_news_bubble_controller.h", + "views/brave_news/brave_news_location_view.cc", + "views/brave_news/brave_news_location_view.h", "views/frame/vertical_tab_strip_region_view.cc", "views/frame/vertical_tab_strip_region_view.h", "views/frame/vertical_tab_strip_root_view.cc", @@ -1058,8 +1062,6 @@ source_set("ui") { "views/frame/vertical_tab_strip_widget_delegate_view.h", "views/location_bar/brave_location_bar_view.cc", "views/location_bar/brave_location_bar_view.h", - "views/location_bar/brave_news_location_view.cc", - "views/location_bar/brave_news_location_view.h", "views/location_bar/brave_star_view.cc", "views/location_bar/brave_star_view.h", "views/profiles/brave_avatar_toolbar_button.cc", diff --git a/browser/ui/views/brave_news/brave_news_bubble_controller.cc b/browser/ui/views/brave_news/brave_news_bubble_controller.cc new file mode 100644 index 000000000000..1410a07bd8fa --- /dev/null +++ b/browser/ui/views/brave_news/brave_news_bubble_controller.cc @@ -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/. */ + +#include "brave/browser/ui/views/brave_news/brave_news_bubble_controller.h" + +#include + +#include "base/memory/ptr_util.h" +#include "brave/browser/ui/views/brave_news/brave_news_bubble_view.h" +#include "brave/browser/ui/views/brave_news/brave_news_location_view.h" +#include "ui/views/bubble/bubble_dialog_delegate_view.h" + +namespace brave_news { +// static +BraveNewsBubbleController* +BraveNewsBubbleController::CreateOrGetFromWebContents( + content::WebContents* web_contents) { + CHECK(web_contents); + BraveNewsBubbleController::CreateForWebContents(web_contents); + return BraveNewsBubbleController::FromWebContents(web_contents); +} + +BraveNewsBubbleController::~BraveNewsBubbleController() = default; + +void BraveNewsBubbleController::ShowBubble( + base::WeakPtr anchor_view) { + if (!anchor_view) { + return; + } + + bubble_ = new BraveNewsBubbleView(anchor_view.get(), web_contents_); + views::BubbleDialogDelegateView::CreateBubble( + base::WrapUnique( + static_cast(bubble_.get()))) + ->Show(); +} + +BraveNewsBubbleView* BraveNewsBubbleController::GetBubble() { + return bubble_; +} + +void BraveNewsBubbleController::OnBubbleClosed() { + bubble_ = nullptr; +} + +base::WeakPtr +BraveNewsBubbleController::AsWeakPtr() { + return weak_ptr_factory_.GetWeakPtr(); +} + +BraveNewsBubbleController::BraveNewsBubbleController( + content::WebContents* web_contents) + : content::WebContentsUserData(*web_contents), + web_contents_(web_contents) {} + +WEB_CONTENTS_USER_DATA_KEY_IMPL(BraveNewsBubbleController); + +} // namespace brave_news diff --git a/browser/ui/views/brave_news/brave_news_bubble_controller.h b/browser/ui/views/brave_news/brave_news_bubble_controller.h new file mode 100644 index 000000000000..5952f2561495 --- /dev/null +++ b/browser/ui/views/brave_news/brave_news_bubble_controller.h @@ -0,0 +1,51 @@ +/* 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/. */ + +#ifndef BRAVE_BROWSER_UI_VIEWS_BRAVE_NEWS_BRAVE_NEWS_BUBBLE_CONTROLLER_H_ +#define BRAVE_BROWSER_UI_VIEWS_BRAVE_NEWS_BRAVE_NEWS_BUBBLE_CONTROLLER_H_ + +#include "base/memory/raw_ptr.h" +#include "base/memory/weak_ptr.h" +#include "content/public/browser/web_contents_user_data.h" + +class PlaylistActionIconView; + +namespace content { +class WebContents; +} + +class BraveNewsBubbleView; +class BraveNewsLocationView; + +namespace brave_news { + +class BraveNewsBubbleController + : public content::WebContentsUserData { + public: + static BraveNewsBubbleController* CreateOrGetFromWebContents( + content::WebContents* web_contents); + + ~BraveNewsBubbleController() override; + + void ShowBubble(base::WeakPtr anchor_view); + BraveNewsBubbleView* GetBubble(); + void OnBubbleClosed(); + base::WeakPtr AsWeakPtr(); + + private: + friend class content::WebContentsUserData; + WEB_CONTENTS_USER_DATA_KEY_DECL(); + + explicit BraveNewsBubbleController(content::WebContents* web_contents); + + raw_ptr bubble_ = nullptr; + raw_ptr web_contents_ = nullptr; + + base::WeakPtrFactory weak_ptr_factory_{this}; +}; + +} // namespace brave_news + +#endif // BRAVE_BROWSER_UI_VIEWS_BRAVE_NEWS_BRAVE_NEWS_BUBBLE_CONTROLLER_H_ diff --git a/browser/ui/views/brave_news/brave_news_bubble_view.cc b/browser/ui/views/brave_news/brave_news_bubble_view.cc index 643ca0e58e20..f24d3f9fd0db 100644 --- a/browser/ui/views/brave_news/brave_news_bubble_view.cc +++ b/browser/ui/views/brave_news/brave_news_bubble_view.cc @@ -13,6 +13,7 @@ #include "base/strings/utf_string_conversions.h" #include "brave/browser/brave_news/brave_news_tab_helper.h" #include "brave/browser/themes/brave_dark_mode_utils.h" +#include "brave/browser/ui/views/brave_news/brave_news_bubble_controller.h" #include "brave/browser/ui/views/brave_news/brave_news_feed_item_view.h" #include "brave/browser/ui/views/brave_news/brave_news_feeds_container_view.h" #include "brave/components/brave_news/common/pref_names.h" @@ -77,6 +78,11 @@ BraveNewsBubbleView::BraveNewsBubbleView(views::View* action_view, contents_(contents) { DCHECK(contents); + auto* controller = + brave_news::BraveNewsBubbleController::FromWebContents(contents); + CHECK(controller); + controller_ = controller->AsWeakPtr(); + SetButtons(ui::DIALOG_BUTTON_NONE); SetAccessibleWindowRole(ax::mojom::Role::kDialog); set_adjust_if_offscreen(true); @@ -146,6 +152,14 @@ void BraveNewsBubbleView::OpenManageFeeds() { /*navigation_handle_callback=*/{}); } +void BraveNewsBubbleView::OnWidgetDestroyed(views::Widget*) { + if (!controller_) { + return; + } + + controller_->OnBubbleClosed(); +} + void BraveNewsBubbleView::OnThemeChanged() { views::BubbleDialogDelegateView::OnThemeChanged(); diff --git a/browser/ui/views/brave_news/brave_news_bubble_view.h b/browser/ui/views/brave_news/brave_news_bubble_view.h index 7a29a32a0c9d..a21e7bf8d8e5 100644 --- a/browser/ui/views/brave_news/brave_news_bubble_view.h +++ b/browser/ui/views/brave_news/brave_news_bubble_view.h @@ -16,6 +16,9 @@ #include "ui/views/widget/widget.h" class BraveNewsFeedsContainerView; +namespace brave_news { +class BraveNewsBubbleController; +} class BraveNewsBubbleView : public views::BubbleDialogDelegateView { METADATA_HEADER(BraveNewsBubbleView, views::BubbleDialogDelegateView) @@ -32,13 +35,16 @@ class BraveNewsBubbleView : public views::BubbleDialogDelegateView { void OpenManageFeeds(); // views::BubbleDialogDelegateView: + void OnWidgetDestroyed(views::Widget*) override; void OnThemeChanged() override; private: - raw_ptr contents_; + raw_ptr contents_ = nullptr; raw_ptr title_label_ = nullptr; raw_ptr subtitle_label_ = nullptr; raw_ptr feeds_container_ = nullptr; + + base::WeakPtr controller_; }; #endif // BRAVE_BROWSER_UI_VIEWS_BRAVE_NEWS_BRAVE_NEWS_BUBBLE_VIEW_H_ diff --git a/browser/ui/views/location_bar/brave_news_location_view.cc b/browser/ui/views/brave_news/brave_news_location_view.cc similarity index 87% rename from browser/ui/views/location_bar/brave_news_location_view.cc rename to browser/ui/views/brave_news/brave_news_location_view.cc index 0e8b390177e3..c7e1a865fe5b 100644 --- a/browser/ui/views/location_bar/brave_news_location_view.cc +++ b/browser/ui/views/brave_news/brave_news_location_view.cc @@ -3,7 +3,7 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this file, // you can obtain one at http://mozilla.org/MPL/2.0/. -#include "brave/browser/ui/views/location_bar/brave_news_location_view.h" +#include "brave/browser/ui/views/brave_news/brave_news_location_view.h" #include #include @@ -12,6 +12,7 @@ #include "base/functional/bind.h" #include "base/functional/callback_forward.h" #include "brave/browser/brave_news/brave_news_tab_helper.h" +#include "brave/browser/ui/views/brave_news/brave_news_bubble_controller.h" #include "brave/browser/ui/views/brave_news/brave_news_bubble_view.h" #include "brave/components/brave_news/common/pref_names.h" #include "brave/components/vector_icons/vector_icons.h" @@ -61,10 +62,6 @@ BraveNewsLocationView::BraveNewsLocationView( BraveNewsLocationView::~BraveNewsLocationView() = default; -views::BubbleDialogDelegate* BraveNewsLocationView::GetBubble() const { - return bubble_view_; -} - void BraveNewsLocationView::UpdateImpl() { auto* contents = GetWebContents(); BraveNewsTabHelper* tab_helper = @@ -148,22 +145,7 @@ void BraveNewsLocationView::OnThemeChanged() { void BraveNewsLocationView::OnExecuting( PageActionIconView::ExecuteSource execute_source) { - // If the bubble is already open, do nothing. - if (IsBubbleShowing()) { - return; - } - - auto* contents = GetWebContents(); - if (!contents) { - return; - } - - bubble_view_ = new BraveNewsBubbleView(this, contents); - bubble_view_->SetCloseCallback(base::BindOnce( - &BraveNewsLocationView::OnBubbleClosed, base::Unretained(this))); - auto* bubble_widget = - views::BubbleDialogDelegateView::CreateBubble(bubble_view_); - bubble_widget->Show(); + ShowBraveNewsBubble(); } void BraveNewsLocationView::UpdateIconColor(bool subscribed) { @@ -178,8 +160,27 @@ void BraveNewsLocationView::UpdateIconColor(bool subscribed) { SetIconColor(icon_color); } -void BraveNewsLocationView::OnBubbleClosed() { - bubble_view_ = nullptr; +brave_news::BraveNewsBubbleController* BraveNewsLocationView::GetController() + const { + auto* web_contents = GetWebContents(); + return web_contents ? brave_news::BraveNewsBubbleController:: + CreateOrGetFromWebContents(web_contents) + : nullptr; +} + +views::BubbleDialogDelegate* BraveNewsLocationView::GetBubble() const { + auto* controller = GetController(); + return controller ? controller->GetBubble() : nullptr; +} + +void BraveNewsLocationView::ShowBraveNewsBubble() { + if (auto* controller = GetController()) { + controller->ShowBubble(AsWeakPtr()); + } +} + +base::WeakPtr BraveNewsLocationView::AsWeakPtr() { + return weak_ptr_factory_.GetWeakPtr(); } BEGIN_METADATA(BraveNewsLocationView) diff --git a/browser/ui/views/location_bar/brave_news_location_view.h b/browser/ui/views/brave_news/brave_news_location_view.h similarity index 82% rename from browser/ui/views/location_bar/brave_news_location_view.h rename to browser/ui/views/brave_news/brave_news_location_view.h index 23214e4d1463..227f7e41e02b 100644 --- a/browser/ui/views/location_bar/brave_news_location_view.h +++ b/browser/ui/views/brave_news/brave_news_location_view.h @@ -3,8 +3,8 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this file, // you can obtain one at http://mozilla.org/MPL/2.0/. -#ifndef BRAVE_BROWSER_UI_VIEWS_LOCATION_BAR_BRAVE_NEWS_LOCATION_VIEW_H_ -#define BRAVE_BROWSER_UI_VIEWS_LOCATION_BAR_BRAVE_NEWS_LOCATION_VIEW_H_ +#ifndef BRAVE_BROWSER_UI_VIEWS_BRAVE_NEWS_BRAVE_NEWS_LOCATION_VIEW_H_ +#define BRAVE_BROWSER_UI_VIEWS_BRAVE_NEWS_BRAVE_NEWS_LOCATION_VIEW_H_ #include #include @@ -20,6 +20,10 @@ class Profile; class BraveNewsBubbleView; +namespace brave_news { +class BraveNewsBubbleController; +} + // LocationBar action for Brave News which shows a bubble allowing the user to // manage feed subscriptions for the current Tab class BraveNewsLocationView : public PageActionIconView, @@ -35,6 +39,8 @@ class BraveNewsLocationView : public PageActionIconView, BraveNewsLocationView& operator=(const BraveNewsLocationView&) = delete; ~BraveNewsLocationView() override; + base::WeakPtr AsWeakPtr(); + // PageActionIconView: views::BubbleDialogDelegate* GetBubble() const override; void UpdateImpl() override; @@ -51,13 +57,15 @@ class BraveNewsLocationView : public PageActionIconView, void WebContentsDestroyed() override; protected: + brave_news::BraveNewsBubbleController* GetController() const; + // PageActionIconView: void OnExecuting(PageActionIconView::ExecuteSource execute_source) override; const gfx::VectorIcon& GetVectorIcon() const override; private: void UpdateIconColor(bool subscribed); - void OnBubbleClosed(); + void ShowBraveNewsBubble(); base::ScopedObservation @@ -65,7 +73,8 @@ class BraveNewsLocationView : public PageActionIconView, BooleanPrefMember should_show_; BooleanPrefMember opted_in_; BooleanPrefMember news_enabled_; - raw_ptr bubble_view_ = nullptr; + + base::WeakPtrFactory weak_ptr_factory_{this}; }; -#endif // BRAVE_BROWSER_UI_VIEWS_LOCATION_BAR_BRAVE_NEWS_LOCATION_VIEW_H_ +#endif // BRAVE_BROWSER_UI_VIEWS_BRAVE_NEWS_BRAVE_NEWS_LOCATION_VIEW_H_ diff --git a/browser/ui/views/location_bar/brave_location_bar_view.cc b/browser/ui/views/location_bar/brave_location_bar_view.cc index fc52508ef969..607d0306b85e 100644 --- a/browser/ui/views/location_bar/brave_location_bar_view.cc +++ b/browser/ui/views/location_bar/brave_location_bar_view.cc @@ -17,7 +17,7 @@ #include "brave/browser/ui/tabs/brave_tab_prefs.h" #include "brave/browser/ui/tabs/features.h" #include "brave/browser/ui/views/brave_actions/brave_actions_container.h" -#include "brave/browser/ui/views/location_bar/brave_news_location_view.h" +#include "brave/browser/ui/views/brave_news/brave_news_location_view.h" #include "brave/browser/ui/views/playlist/playlist_action_icon_view.h" #include "brave/browser/ui/views/toolbar/brave_toolbar_view.h" #include "brave/components/commander/common/buildflags/buildflags.h" diff --git a/browser/ui/views/location_bar/brave_location_bar_view.h b/browser/ui/views/location_bar/brave_location_bar_view.h index 9d0a9317cf3e..4e7f37d2c286 100644 --- a/browser/ui/views/location_bar/brave_location_bar_view.h +++ b/browser/ui/views/location_bar/brave_location_bar_view.h @@ -10,7 +10,7 @@ #include #include "base/gtest_prod_util.h" -#include "brave/browser/ui/views/location_bar/brave_news_location_view.h" +#include "brave/browser/ui/views/brave_news/brave_news_location_view.h" #include "brave/browser/ui/views/view_shadow.h" #include "brave/components/ipfs/buildflags/buildflags.h" #include "brave/components/tor/buildflags/buildflags.h" From 82317f8bb47aab3892da66082a31a494aac94429 Mon Sep 17 00:00:00 2001 From: Jagadesh P Date: Wed, 19 Jun 2024 04:32:32 +0530 Subject: [PATCH 2/2] [BraveNews] Rename brave_news_location_view.cc[h] -> brave_news_action_icon_view.cc[h] --- browser/ui/BUILD.gn | 4 +- ...view.cc => brave_news_action_icon_view.cc} | 44 +++++++++---------- ...n_view.h => brave_news_action_icon_view.h} | 31 +++++++------ .../brave_news_bubble_controller.cc | 4 +- .../brave_news/brave_news_bubble_controller.h | 6 +-- .../location_bar/brave_location_bar_view.cc | 30 +++++++------ .../location_bar/brave_location_bar_view.h | 4 +- 7 files changed, 61 insertions(+), 62 deletions(-) rename browser/ui/views/brave_news/{brave_news_location_view.cc => brave_news_action_icon_view.cc} (78%) rename browser/ui/views/brave_news/{brave_news_location_view.h => brave_news_action_icon_view.h} (67%) diff --git a/browser/ui/BUILD.gn b/browser/ui/BUILD.gn index 25476512970a..3899da45e0a4 100644 --- a/browser/ui/BUILD.gn +++ b/browser/ui/BUILD.gn @@ -1050,10 +1050,10 @@ source_set("ui") { "views/brave_help_bubble/brave_help_bubble_delegate_view.h", "views/brave_help_bubble/brave_help_bubble_host_view.cc", "views/brave_help_bubble/brave_help_bubble_host_view.h", + "views/brave_news/brave_news_action_icon_view.cc", + "views/brave_news/brave_news_action_icon_view.h", "views/brave_news/brave_news_bubble_controller.cc", "views/brave_news/brave_news_bubble_controller.h", - "views/brave_news/brave_news_location_view.cc", - "views/brave_news/brave_news_location_view.h", "views/frame/vertical_tab_strip_region_view.cc", "views/frame/vertical_tab_strip_region_view.h", "views/frame/vertical_tab_strip_root_view.cc", diff --git a/browser/ui/views/brave_news/brave_news_location_view.cc b/browser/ui/views/brave_news/brave_news_action_icon_view.cc similarity index 78% rename from browser/ui/views/brave_news/brave_news_location_view.cc rename to browser/ui/views/brave_news/brave_news_action_icon_view.cc index c7e1a865fe5b..8c9a24f87e13 100644 --- a/browser/ui/views/brave_news/brave_news_location_view.cc +++ b/browser/ui/views/brave_news/brave_news_action_icon_view.cc @@ -1,9 +1,9 @@ -// Copyright (c) 2022 The Brave Authors. All rights reserved. +// 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 http://mozilla.org/MPL/2.0/. +// You can obtain one at https://mozilla.org/MPL/2.0/. -#include "brave/browser/ui/views/brave_news/brave_news_location_view.h" +#include "brave/browser/ui/views/brave_news/brave_news_action_icon_view.h" #include #include @@ -34,7 +34,7 @@ constexpr SkColor kSubscribedDarkColor = SkColorSetRGB(115, 122, 222); } // namespace -BraveNewsLocationView::BraveNewsLocationView( +BraveNewsActionIconView::BraveNewsActionIconView( Profile* profile, IconLabelBubbleView::Delegate* icon_label_bubble_delegate, PageActionIconView::Delegate* page_action_icon_delegate) @@ -47,22 +47,22 @@ BraveNewsLocationView::BraveNewsLocationView( should_show_.Init(brave_news::prefs::kShouldShowToolbarButton, profile->GetPrefs(), - base::BindRepeating(&BraveNewsLocationView::UpdateImpl, + base::BindRepeating(&BraveNewsActionIconView::UpdateImpl, base::Unretained(this))); opted_in_.Init(brave_news::prefs::kBraveNewsOptedIn, profile->GetPrefs(), - base::BindRepeating(&BraveNewsLocationView::UpdateImpl, + base::BindRepeating(&BraveNewsActionIconView::UpdateImpl, base::Unretained(this))); news_enabled_.Init(brave_news::prefs::kNewTabPageShowToday, profile->GetPrefs(), - base::BindRepeating(&BraveNewsLocationView::UpdateImpl, + base::BindRepeating(&BraveNewsActionIconView::UpdateImpl, base::Unretained(this))); Update(); } -BraveNewsLocationView::~BraveNewsLocationView() = default; +BraveNewsActionIconView::~BraveNewsActionIconView() = default; -void BraveNewsLocationView::UpdateImpl() { +void BraveNewsActionIconView::UpdateImpl() { auto* contents = GetWebContents(); BraveNewsTabHelper* tab_helper = contents ? BraveNewsTabHelper::FromWebContents(contents) : nullptr; @@ -111,30 +111,30 @@ void BraveNewsLocationView::UpdateImpl() { SetVisible(is_visible); } -void BraveNewsLocationView::WebContentsDestroyed() { +void BraveNewsActionIconView::WebContentsDestroyed() { page_feeds_observer_.Reset(); Observe(nullptr); } -const gfx::VectorIcon& BraveNewsLocationView::GetVectorIcon() const { +const gfx::VectorIcon& BraveNewsActionIconView::GetVectorIcon() const { return kLeoRssIcon; } -std::u16string BraveNewsLocationView::GetTextForTooltipAndAccessibleName() +std::u16string BraveNewsActionIconView::GetTextForTooltipAndAccessibleName() const { return l10n_util::GetStringUTF16(IDS_BRAVE_NEWS_ACTION_VIEW_TOOLTIP); } -bool BraveNewsLocationView::ShouldShowLabel() const { +bool BraveNewsActionIconView::ShouldShowLabel() const { return false; } -void BraveNewsLocationView::OnAvailableFeedsChanged( +void BraveNewsActionIconView::OnAvailableFeedsChanged( const std::vector& feeds) { Update(); } -void BraveNewsLocationView::OnThemeChanged() { +void BraveNewsActionIconView::OnThemeChanged() { bool subscribed = false; if (auto* contents = GetWebContents()) { subscribed = BraveNewsTabHelper::FromWebContents(contents)->IsSubscribed(); @@ -143,12 +143,12 @@ void BraveNewsLocationView::OnThemeChanged() { PageActionIconView::OnThemeChanged(); } -void BraveNewsLocationView::OnExecuting( +void BraveNewsActionIconView::OnExecuting( PageActionIconView::ExecuteSource execute_source) { ShowBraveNewsBubble(); } -void BraveNewsLocationView::UpdateIconColor(bool subscribed) { +void BraveNewsActionIconView::UpdateIconColor(bool subscribed) { SkColor icon_color; if (subscribed) { auto is_dark = GetNativeTheme()->GetPreferredColorScheme() == @@ -160,7 +160,7 @@ void BraveNewsLocationView::UpdateIconColor(bool subscribed) { SetIconColor(icon_color); } -brave_news::BraveNewsBubbleController* BraveNewsLocationView::GetController() +brave_news::BraveNewsBubbleController* BraveNewsActionIconView::GetController() const { auto* web_contents = GetWebContents(); return web_contents ? brave_news::BraveNewsBubbleController:: @@ -168,20 +168,20 @@ brave_news::BraveNewsBubbleController* BraveNewsLocationView::GetController() : nullptr; } -views::BubbleDialogDelegate* BraveNewsLocationView::GetBubble() const { +views::BubbleDialogDelegate* BraveNewsActionIconView::GetBubble() const { auto* controller = GetController(); return controller ? controller->GetBubble() : nullptr; } -void BraveNewsLocationView::ShowBraveNewsBubble() { +void BraveNewsActionIconView::ShowBraveNewsBubble() { if (auto* controller = GetController()) { controller->ShowBubble(AsWeakPtr()); } } -base::WeakPtr BraveNewsLocationView::AsWeakPtr() { +base::WeakPtr BraveNewsActionIconView::AsWeakPtr() { return weak_ptr_factory_.GetWeakPtr(); } -BEGIN_METADATA(BraveNewsLocationView) +BEGIN_METADATA(BraveNewsActionIconView) END_METADATA diff --git a/browser/ui/views/brave_news/brave_news_location_view.h b/browser/ui/views/brave_news/brave_news_action_icon_view.h similarity index 67% rename from browser/ui/views/brave_news/brave_news_location_view.h rename to browser/ui/views/brave_news/brave_news_action_icon_view.h index 227f7e41e02b..32303f125b3d 100644 --- a/browser/ui/views/brave_news/brave_news_location_view.h +++ b/browser/ui/views/brave_news/brave_news_action_icon_view.h @@ -1,10 +1,10 @@ -// Copyright (c) 2022 The Brave Authors. All rights reserved. +// 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 http://mozilla.org/MPL/2.0/. +// You can obtain one at https://mozilla.org/MPL/2.0/. -#ifndef BRAVE_BROWSER_UI_VIEWS_BRAVE_NEWS_BRAVE_NEWS_LOCATION_VIEW_H_ -#define BRAVE_BROWSER_UI_VIEWS_BRAVE_NEWS_BRAVE_NEWS_LOCATION_VIEW_H_ +#ifndef BRAVE_BROWSER_UI_VIEWS_BRAVE_NEWS_BRAVE_NEWS_ACTION_ICON_VIEW_H_ +#define BRAVE_BROWSER_UI_VIEWS_BRAVE_NEWS_BRAVE_NEWS_ACTION_ICON_VIEW_H_ #include #include @@ -18,7 +18,6 @@ #include "ui/views/view.h" class Profile; -class BraveNewsBubbleView; namespace brave_news { class BraveNewsBubbleController; @@ -26,20 +25,20 @@ class BraveNewsBubbleController; // LocationBar action for Brave News which shows a bubble allowing the user to // manage feed subscriptions for the current Tab -class BraveNewsLocationView : public PageActionIconView, - public BraveNewsTabHelper::PageFeedsObserver, - public content::WebContentsObserver { - METADATA_HEADER(BraveNewsLocationView, PageActionIconView) +class BraveNewsActionIconView : public PageActionIconView, + public BraveNewsTabHelper::PageFeedsObserver, + public content::WebContentsObserver { + METADATA_HEADER(BraveNewsActionIconView, PageActionIconView) public: - BraveNewsLocationView( + BraveNewsActionIconView( Profile* profile, IconLabelBubbleView::Delegate* icon_label_bubble_delegate, PageActionIconView::Delegate* page_action_icon_delegate); - BraveNewsLocationView(const BraveNewsLocationView&) = delete; - BraveNewsLocationView& operator=(const BraveNewsLocationView&) = delete; - ~BraveNewsLocationView() override; + BraveNewsActionIconView(const BraveNewsActionIconView&) = delete; + BraveNewsActionIconView& operator=(const BraveNewsActionIconView&) = delete; + ~BraveNewsActionIconView() override; - base::WeakPtr AsWeakPtr(); + base::WeakPtr AsWeakPtr(); // PageActionIconView: views::BubbleDialogDelegate* GetBubble() const override; @@ -74,7 +73,7 @@ class BraveNewsLocationView : public PageActionIconView, BooleanPrefMember opted_in_; BooleanPrefMember news_enabled_; - base::WeakPtrFactory weak_ptr_factory_{this}; + base::WeakPtrFactory weak_ptr_factory_{this}; }; -#endif // BRAVE_BROWSER_UI_VIEWS_BRAVE_NEWS_BRAVE_NEWS_LOCATION_VIEW_H_ +#endif // BRAVE_BROWSER_UI_VIEWS_BRAVE_NEWS_BRAVE_NEWS_ACTION_ICON_VIEW_H_ diff --git a/browser/ui/views/brave_news/brave_news_bubble_controller.cc b/browser/ui/views/brave_news/brave_news_bubble_controller.cc index 1410a07bd8fa..75abe2700c83 100644 --- a/browser/ui/views/brave_news/brave_news_bubble_controller.cc +++ b/browser/ui/views/brave_news/brave_news_bubble_controller.cc @@ -8,8 +8,8 @@ #include #include "base/memory/ptr_util.h" +#include "brave/browser/ui/views/brave_news/brave_news_action_icon_view.h" #include "brave/browser/ui/views/brave_news/brave_news_bubble_view.h" -#include "brave/browser/ui/views/brave_news/brave_news_location_view.h" #include "ui/views/bubble/bubble_dialog_delegate_view.h" namespace brave_news { @@ -25,7 +25,7 @@ BraveNewsBubbleController::CreateOrGetFromWebContents( BraveNewsBubbleController::~BraveNewsBubbleController() = default; void BraveNewsBubbleController::ShowBubble( - base::WeakPtr anchor_view) { + base::WeakPtr anchor_view) { if (!anchor_view) { return; } diff --git a/browser/ui/views/brave_news/brave_news_bubble_controller.h b/browser/ui/views/brave_news/brave_news_bubble_controller.h index 5952f2561495..0bb6c9301066 100644 --- a/browser/ui/views/brave_news/brave_news_bubble_controller.h +++ b/browser/ui/views/brave_news/brave_news_bubble_controller.h @@ -10,14 +10,12 @@ #include "base/memory/weak_ptr.h" #include "content/public/browser/web_contents_user_data.h" -class PlaylistActionIconView; - namespace content { class WebContents; } class BraveNewsBubbleView; -class BraveNewsLocationView; +class BraveNewsActionIconView; namespace brave_news { @@ -29,7 +27,7 @@ class BraveNewsBubbleController ~BraveNewsBubbleController() override; - void ShowBubble(base::WeakPtr anchor_view); + void ShowBubble(base::WeakPtr anchor_view); BraveNewsBubbleView* GetBubble(); void OnBubbleClosed(); base::WeakPtr AsWeakPtr(); diff --git a/browser/ui/views/location_bar/brave_location_bar_view.cc b/browser/ui/views/location_bar/brave_location_bar_view.cc index 607d0306b85e..aebba9fb603b 100644 --- a/browser/ui/views/location_bar/brave_location_bar_view.cc +++ b/browser/ui/views/location_bar/brave_location_bar_view.cc @@ -17,7 +17,7 @@ #include "brave/browser/ui/tabs/brave_tab_prefs.h" #include "brave/browser/ui/tabs/features.h" #include "brave/browser/ui/views/brave_actions/brave_actions_container.h" -#include "brave/browser/ui/views/brave_news/brave_news_location_view.h" +#include "brave/browser/ui/views/brave_news/brave_news_action_icon_view.h" #include "brave/browser/ui/views/playlist/playlist_action_icon_view.h" #include "brave/browser/ui/views/toolbar/brave_toolbar_view.h" #include "brave/components/commander/common/buildflags/buildflags.h" @@ -120,11 +120,11 @@ void BraveLocationBarView::Init() { } if (!browser_->profile()->IsOffTheRecord()) { - brave_news_location_view_ = - AddChildView(std::make_unique( + brave_news_action_icon_view_ = + AddChildView(std::make_unique( browser_->profile(), this, this)); - brave_news_location_view_->SetVisible(false); - views::InkDrop::Get(brave_news_location_view_) + brave_news_action_icon_view_->SetVisible(false); + views::InkDrop::Get(brave_news_action_icon_view_) ->SetVisibleOpacity(GetPageActionInkDropVisibleOpacity()); } #if BUILDFLAG(ENABLE_TOR) @@ -199,8 +199,8 @@ void BraveLocationBarView::Update(content::WebContents* contents) { } #endif - if (brave_news_location_view_) { - brave_news_location_view_->Update(); + if (brave_news_action_icon_view_) { + brave_news_action_icon_view_->Update(); } LocationBarView::Update(contents); @@ -263,8 +263,8 @@ void BraveLocationBarView::OnChanged() { } #endif - if (brave_news_location_view_) { - brave_news_location_view_->Update(); + if (brave_news_action_icon_view_) { + brave_news_action_icon_view_->Update(); } // OnChanged calls Layout @@ -273,8 +273,8 @@ void BraveLocationBarView::OnChanged() { std::vector BraveLocationBarView::GetTrailingViews() { std::vector views; - if (brave_news_location_view_) { - views.push_back(brave_news_location_view_); + if (brave_news_action_icon_view_) { + views.push_back(brave_news_action_icon_view_); } #if BUILDFLAG(ENABLE_TOR) if (onion_location_view_) { @@ -314,9 +314,11 @@ gfx::Size BraveLocationBarView::CalculatePreferredSize( brave_actions_min + GetLayoutConstant(LOCATION_BAR_ELEMENT_PADDING); min_size.Enlarge(extra_width, 0); } - if (brave_news_location_view_ && brave_news_location_view_->GetVisible()) { - const int extra_width = GetLayoutConstant(LOCATION_BAR_ELEMENT_PADDING) + - brave_news_location_view_->GetMinimumSize().width(); + if (brave_news_action_icon_view_ && + brave_news_action_icon_view_->GetVisible()) { + const int extra_width = + GetLayoutConstant(LOCATION_BAR_ELEMENT_PADDING) + + brave_news_action_icon_view_->GetMinimumSize().width(); min_size.Enlarge(extra_width, 0); } #if BUILDFLAG(ENABLE_TOR) diff --git a/browser/ui/views/location_bar/brave_location_bar_view.h b/browser/ui/views/location_bar/brave_location_bar_view.h index 4e7f37d2c286..b71069dae513 100644 --- a/browser/ui/views/location_bar/brave_location_bar_view.h +++ b/browser/ui/views/location_bar/brave_location_bar_view.h @@ -10,7 +10,7 @@ #include #include "base/gtest_prod_util.h" -#include "brave/browser/ui/views/brave_news/brave_news_location_view.h" +#include "brave/browser/ui/views/brave_news/brave_news_action_icon_view.h" #include "brave/browser/ui/views/view_shadow.h" #include "brave/components/ipfs/buildflags/buildflags.h" #include "brave/components/tor/buildflags/buildflags.h" @@ -117,7 +117,7 @@ class BraveLocationBarView : public LocationBarView { std::unique_ptr shadow_; raw_ptr brave_actions_ = nullptr; - raw_ptr brave_news_location_view_ = nullptr; + raw_ptr brave_news_action_icon_view_ = nullptr; #if BUILDFLAG(ENABLE_TOR) raw_ptr onion_location_view_ = nullptr; #endif