diff --git a/browser/playlist/playlist_tab_helper.cc b/browser/playlist/playlist_tab_helper.cc index 01a4ec14fe6..971900c496b 100644 --- a/browser/playlist/playlist_tab_helper.cc +++ b/browser/playlist/playlist_tab_helper.cc @@ -259,6 +259,25 @@ void PlaylistTabHelper::OnFoundMediaFromContents( } } +std::vector PlaylistTabHelper::GetUnsavedItems() const { + if (found_items_.empty()) { + return {}; + } + + base::flat_set saved_items; + base::ranges::transform(saved_items_, + std::inserter(saved_items, saved_items.begin()), + [](const auto& item) { return item->id; }); + + std::vector unsaved_items; + for (const auto& found_item : found_items_) { + if (!base::Contains(saved_items, found_item->id)) { + unsaved_items.push_back(found_item->Clone()); + } + } + return unsaved_items; +} + void PlaylistTabHelper::OnAddedItems( std::vector items) { // mojo based observer tends to be notified later. i.e. OnItemCreated() will diff --git a/browser/playlist/playlist_tab_helper.h b/browser/playlist/playlist_tab_helper.h index d86634b15c9..e32939627f3 100644 --- a/browser/playlist/playlist_tab_helper.h +++ b/browser/playlist/playlist_tab_helper.h @@ -38,6 +38,8 @@ class PlaylistTabHelper return found_items_; } + std::vector GetUnsavedItems() const; + void AddObserver(PlaylistTabHelperObserver* observer); void RemoveObserver(PlaylistTabHelperObserver* observer); diff --git a/browser/ui/views/playlist/playlist_action_bubble_view.cc b/browser/ui/views/playlist/playlist_action_bubble_view.cc index 98b8bf2fa06..a9242ecdd3e 100644 --- a/browser/ui/views/playlist/playlist_action_bubble_view.cc +++ b/browser/ui/views/playlist/playlist_action_bubble_view.cc @@ -13,6 +13,7 @@ #include "brave/app/vector_icons/vector_icons.h" #include "brave/browser/playlist/playlist_tab_helper.h" #include "brave/browser/ui/color/brave_color_id.h" +#include "brave/browser/ui/views/playlist/playlist_action_icon_view.h" #include "brave/browser/ui/views/side_panel/playlist/playlist_side_panel_coordinator.h" #include "brave/grit/brave_theme_resources.h" #include "chrome/grit/generated_resources.h" @@ -32,6 +33,19 @@ namespace { PlaylistActionBubbleView* g_bubble = nullptr; +template >* = + nullptr> +void ShowBubble(std::unique_ptr bubble) { + DCHECK(!g_bubble); + + g_bubble = bubble.release(); + + auto* widget = views::BubbleDialogDelegateView::CreateBubble(g_bubble); + widget->Show(); +} + //////////////////////////////////////////////////////////////////////////////// // DefaultThumbnailBackground // @@ -65,7 +79,7 @@ class ConfirmBubble : public PlaylistActionBubbleView { METADATA_HEADER(ConfirmBubble); ConfirmBubble(Browser* browser, - views::View* anchor, + PlaylistActionIconView* anchor, playlist::PlaylistTabHelper* playlist_tab_helper); ~ConfirmBubble() override = default; @@ -87,6 +101,57 @@ class ConfirmBubble : public PlaylistActionBubbleView { void MoreMediaInContents(); }; +//////////////////////////////////////////////////////////////////////////////// +// AddBubble +// * Shows when users try adding items found from the current contents. +// * Shows a list of found items and users can select which one to add. +class AddBubble : public PlaylistActionBubbleView { + public: + METADATA_HEADER(AddBubble); + AddBubble(Browser* browser, + PlaylistActionIconView* anchor, + playlist::PlaylistTabHelper* playlist_tab_helper); + AddBubble(Browser* browser, + PlaylistActionIconView* anchor, + playlist::PlaylistTabHelper* playlist_tab_helper, + const std::vector& items); + + private: + class ItemRow : public views::Button { + public: + static constexpr int kWidth = 288; + + ItemRow(playlist::mojom::PlaylistItemPtr item, + base::RepeatingCallback callback); + + bool selected() const { return selected_; } + const playlist::mojom::PlaylistItemPtr& item() const { return item_; } + + // views::Button: + int GetHeightForWidth(int width) const override; + void OnThemeChanged() override; + + private: + ItemRow& OnPressed(const ui::Event& event); + + void SetSelected(bool selected); + void UpdateBackground(); + + playlist::mojom::PlaylistItemPtr item_; + bool selected_ = true; + + raw_ptr selected_icon_ = nullptr; + }; + + void AddSelected(); + void OnItemPressed(const ItemRow& row); + + base::flat_set selected_views_; +}; + +//////////////////////////////////////////////////////////////////////////////// +// ConfirmBubble Impl + ConfirmBubble::Row::Row(const std::u16string& text, const ui::ImageModel& icon, views::Button::PressedCallback callback) @@ -106,7 +171,7 @@ void ConfirmBubble::Row::Layout() { } ConfirmBubble::ConfirmBubble(Browser* browser, - views::View* anchor, + PlaylistActionIconView* anchor, playlist::PlaylistTabHelper* playlist_tab_helper) : PlaylistActionBubbleView(browser, anchor, playlist_tab_helper) { // What this look like @@ -144,13 +209,16 @@ ConfirmBubble::ConfirmBubble(Browser* browser, kIconSize), base::BindRepeating(&ConfirmBubble::RemoveFromPlaylist, base::Unretained(this)))); - AddChildView(std::make_unique()); - AddChildView(std::make_unique( - l10n_util::GetStringUTF16(IDS_PLAYLIST_MORE_MEDIA_IN_THIS_PAGE), - ui::ImageModel::FromVectorIcon(kLeoProductPlaylistIcon, - ui::kColorMenuIcon, kIconSize), - base::BindRepeating(&ConfirmBubble::MoreMediaInContents, - base::Unretained(this)))); + + if (playlist_tab_helper->GetUnsavedItems().size()) { + AddChildView(std::make_unique()); + AddChildView(std::make_unique( + l10n_util::GetStringUTF16(IDS_PLAYLIST_MORE_MEDIA_IN_THIS_PAGE), + ui::ImageModel::FromVectorIcon(kLeoProductPlaylistIcon, + ui::kColorMenuIcon, kIconSize), + base::BindRepeating(&ConfirmBubble::MoreMediaInContents, + base::Unretained(this)))); + } } void ConfirmBubble::OpenInPlaylist() { @@ -190,56 +258,41 @@ void ConfirmBubble::RemoveFromPlaylist() { } void ConfirmBubble::MoreMediaInContents() { - NOTIMPLEMENTED(); + auto show_add_bubble = base::BindOnce( + [](base::WeakPtr tab_helper, + Browser* browser, base::WeakPtr anchor) { + if (!tab_helper || !anchor) { + return; + } + + if (!tab_helper->found_items().size()) { + return; + } + + ::ShowBubble( + std::make_unique(browser, anchor.get(), tab_helper.get(), + tab_helper->GetUnsavedItems())); + }, + playlist_tab_helper_->GetWeakPtr(), + // |Browser| outlives TabHelper so it's okay to bind raw ptr here + browser_.get(), icon_view_->GetWeakPtr()); + + SetCloseCallback( + // WindowClosingImpl should be called first to clean up data before + // showing up new bubble. This callback is called by itself, it's okay to + // pass Unretained(). + base::BindOnce(&PlaylistActionBubbleView::WindowClosingImpl, + base::Unretained(this)) + .Then(std::move(show_add_bubble))); + + GetWidget()->Close(); } BEGIN_METADATA(ConfirmBubble, PlaylistActionBubbleView) END_METADATA //////////////////////////////////////////////////////////////////////////////// -// AddBubble -// * Shows when users try adding items found from the current contents. -// * Shows a list of found items and users can select which one to add. -class AddBubble : public PlaylistActionBubbleView { - public: - METADATA_HEADER(AddBubble); - AddBubble(Browser* browser, - views::View* anchor, - playlist::PlaylistTabHelper* playlist_tab_helper); - - private: - class ItemRow : public views::Button { - public: - static constexpr int kWidth = 288; - - ItemRow(playlist::mojom::PlaylistItemPtr item, - base::RepeatingCallback callback); - - bool selected() const { return selected_; } - const playlist::mojom::PlaylistItemPtr& item() const { return item_; } - - // views::Button: - int GetHeightForWidth(int width) const override; - void OnThemeChanged() override; - - private: - ItemRow& OnPressed(const ui::Event& event); - - void SetSelected(bool selected); - void UpdateBackground(); - - playlist::mojom::PlaylistItemPtr item_; - bool selected_ = true; - - raw_ptr selected_icon_ = nullptr; - }; - - void AddSelected(); - void OnItemPressed(const ItemRow& row); - - base::flat_set selected_views_; -}; - +// AddBubble Impl AddBubble::ItemRow::ItemRow( playlist::mojom::PlaylistItemPtr item, base::RepeatingCallback callback) @@ -316,8 +369,17 @@ void AddBubble::ItemRow::UpdateBackground() { } AddBubble::AddBubble(Browser* browser, - views::View* anchor, + PlaylistActionIconView* anchor, playlist::PlaylistTabHelper* playlist_tab_helper) + : AddBubble(browser, + anchor, + playlist_tab_helper, + playlist_tab_helper->found_items()) {} + +AddBubble::AddBubble(Browser* browser, + PlaylistActionIconView* anchor, + playlist::PlaylistTabHelper* playlist_tab_helper, + const std::vector& items) : PlaylistActionBubbleView(browser, anchor, playlist_tab_helper) { // What this look like // https://user-images.githubusercontent.com/5474642/243532255-f82fc740-eea0-4c52-b43a-378ab703d229.png @@ -344,7 +406,7 @@ AddBubble::AddBubble(Browser* browser, scroll_view->SetContents(std::make_unique()); contents->SetOrientation(views::BoxLayout::Orientation::kVertical); - for (const auto& item : playlist_tab_helper->found_items()) { + for (const auto& item : items) { selected_views_.insert(contents->AddChildView(std::make_unique( item.Clone(), base::BindRepeating(&AddBubble::OnItemPressed, base::Unretained(this))))); @@ -357,8 +419,12 @@ AddBubble::AddBubble(Browser* browser, SetButtonLabel(ui::DialogButton::DIALOG_BUTTON_OK, l10n_util::GetStringUTF16(IDS_PLAYLIST_ADD_SELECTED)); - SetAcceptCallback( - base::BindRepeating(&AddBubble::AddSelected, base::Unretained(this))); + + // This callback is called by itself, it's okay to pass Unretained(this). + SetAcceptCallback(base::BindOnce(&PlaylistActionBubbleView::WindowClosingImpl, + base::Unretained(this)) + .Then(base::BindOnce(&AddBubble::AddSelected, + base::Unretained(this)))); } void AddBubble::AddSelected() { @@ -403,20 +469,17 @@ END_METADATA // static void PlaylistActionBubbleView::ShowBubble( Browser* browser, - views::View* anchor, + PlaylistActionIconView* anchor, playlist::PlaylistTabHelper* playlist_tab_helper) { - DCHECK(!g_bubble); - DCHECK(playlist_tab_helper); - if (playlist_tab_helper->saved_items().size()) { - g_bubble = new ConfirmBubble(browser, anchor, playlist_tab_helper); + ::ShowBubble( + std::make_unique(browser, anchor, playlist_tab_helper)); } else if (playlist_tab_helper->found_items().size()) { - g_bubble = new AddBubble(browser, anchor, playlist_tab_helper); + ::ShowBubble( + std::make_unique(browser, anchor, playlist_tab_helper)); } else { NOTREACHED() << "Caller should filter this case"; } - auto* widget = views::BubbleDialogDelegateView::CreateBubble(g_bubble); - widget->Show(); } // static @@ -437,19 +500,28 @@ PlaylistActionBubbleView* PlaylistActionBubbleView::GetBubble() { PlaylistActionBubbleView::PlaylistActionBubbleView( Browser* browser, - views::View* anchor, + PlaylistActionIconView* anchor, playlist::PlaylistTabHelper* playlist_tab_helper) : BubbleDialogDelegateView(anchor, views::BubbleBorder::Arrow::TOP_RIGHT), browser_(browser), - playlist_tab_helper_(playlist_tab_helper) {} + playlist_tab_helper_(playlist_tab_helper), + icon_view_(anchor) {} PlaylistActionBubbleView::~PlaylistActionBubbleView() = default; void PlaylistActionBubbleView::WindowClosing() { BubbleDialogDelegateView::WindowClosing(); - DCHECK_EQ(g_bubble, this); - g_bubble = nullptr; + WindowClosingImpl(); +} + +void PlaylistActionBubbleView::WindowClosingImpl() { + // This method could be called multiple times during the closing process in + // order to show up a subsequent action bubble. So we should check if + // |g_bubble| is already filled up with a new bubble. + if (g_bubble == this) { + g_bubble = nullptr; + } } BEGIN_METADATA(PlaylistActionBubbleView, views::BubbleDialogDelegateView) diff --git a/browser/ui/views/playlist/playlist_action_bubble_view.h b/browser/ui/views/playlist/playlist_action_bubble_view.h index 9057c8aaa0b..b014985a3a7 100644 --- a/browser/ui/views/playlist/playlist_action_bubble_view.h +++ b/browser/ui/views/playlist/playlist_action_bubble_view.h @@ -7,6 +7,7 @@ #define BRAVE_BROWSER_UI_VIEWS_PLAYLIST_PLAYLIST_ACTION_BUBBLE_VIEW_H_ class Browser; +class PlaylistActionIconView; namespace playlist { class PlaylistTabHelper; @@ -19,7 +20,7 @@ class PlaylistActionBubbleView : public views::BubbleDialogDelegateView { METADATA_HEADER(PlaylistActionBubbleView); static void ShowBubble(Browser* browser, - views::View* anchor, + PlaylistActionIconView* anchor, playlist::PlaylistTabHelper* playlist_tab_helper); static bool IsShowingBubble(); static void CloseBubble(); @@ -27,16 +28,21 @@ class PlaylistActionBubbleView : public views::BubbleDialogDelegateView { ~PlaylistActionBubbleView() override; + void WindowClosingImpl(); + // views::BubbleDialogDelegateView: void WindowClosing() override; protected: PlaylistActionBubbleView(Browser* browser, - views::View* anchor, + PlaylistActionIconView* anchor, playlist::PlaylistTabHelper* playlist_tab_helper); - raw_ptr browser_; + raw_ptr browser_ = nullptr; raw_ptr playlist_tab_helper_ = nullptr; + + // Our anchor. + raw_ptr icon_view_ = nullptr; }; #endif // BRAVE_BROWSER_UI_VIEWS_PLAYLIST_PLAYLIST_ACTION_BUBBLE_VIEW_H_ diff --git a/browser/ui/views/playlist/playlist_action_icon_view.cc b/browser/ui/views/playlist/playlist_action_icon_view.cc index b9085cd48a6..012a8d286de 100644 --- a/browser/ui/views/playlist/playlist_action_icon_view.cc +++ b/browser/ui/views/playlist/playlist_action_icon_view.cc @@ -76,6 +76,10 @@ void PlaylistActionIconView::ShowPlaylistBubble() { PlaylistActionBubbleView::ShowBubble(browser_, this, playlist_tab_helper); } +base::WeakPtr PlaylistActionIconView::GetWeakPtr() { + return weak_ptr_factory_.GetWeakPtr(); +} + const gfx::VectorIcon& PlaylistActionIconView::GetVectorIcon() const { return state_ == State::kAdded ? kLeoProductPlaylistAddedIcon : kLeoProductPlaylistAddIcon; diff --git a/browser/ui/views/playlist/playlist_action_icon_view.h b/browser/ui/views/playlist/playlist_action_icon_view.h index b423c5096cb..5a60024b93a 100644 --- a/browser/ui/views/playlist/playlist_action_icon_view.h +++ b/browser/ui/views/playlist/playlist_action_icon_view.h @@ -31,6 +31,8 @@ class PlaylistActionIconView : public PageActionIconView, void ShowPlaylistBubble(); + base::WeakPtr GetWeakPtr(); + // PageActionIconView: void OnExecuting(ExecuteSource execute_source) override {} views::BubbleDialogDelegate* GetBubble() const override;