[Containers] Activate PageActionIconView for Partitioned Storage on right click (#35194)

Activate PageActionIconView for Partitioned Storage on right click

As requested by the design team, we activate the PageActionIconView for the
Partitioned Storage on right click.
This commit is contained in:
Sangwoo Ko
2026-04-03 03:13:52 +02:00
committed by GitHub
parent b0bc0b0128
commit 7d3fc7cf97
15 changed files with 154 additions and 15 deletions
+15 -4
View File
@@ -7,6 +7,7 @@
#include "base/functional/callback_helpers.h"
#include "base/types/to_address.h"
#include "brave/browser/ui/browser_commands.h"
#include "brave/components/ai_chat/core/common/buildflags/buildflags.h"
#include "brave/components/containers/buildflags/buildflags.h"
#include "brave/components/playlist/core/common/features.h"
@@ -81,10 +82,20 @@ void BraveBrowserActions::InitializeBrowserActions() {
#if BUILDFLAG(ENABLE_CONTAINERS)
if (base::FeatureList::IsEnabled(containers::features::kContainers)) {
root_action_item_->AddChild(actions::ActionItem::Builder(base::DoNothing())
.SetActionId(kActionShowPartitionedStorage)
.SetEnabled(true)
.Build());
root_action_item_->AddChild(
actions::ActionItem::Builder(
// Safe to bind bwi to the callback because root_action_item_ is
// going to be destroyed on the base class's destructor while the
// browser window interface is also member of the base class.
base::BindRepeating(
[](BrowserWindowInterface* bwi, actions::ActionItem* item,
actions::ActionInvocationContext context) {
brave::OpenContainerMenuOnPageActionView(bwi);
},
bwi))
.SetActionId(kActionShowPartitionedStorage)
.SetEnabled(true)
.Build());
}
#endif
}
+6
View File
@@ -21,6 +21,7 @@
#include "base/i18n/time_formatting.h"
#include "base/logging.h"
#include "base/memory/scoped_refptr.h"
#include "base/notimplemented.h"
#include "base/notreached.h"
#include "base/path_service.h"
#include "base/strings/utf_string_conversions.h"
@@ -1195,6 +1196,11 @@ void OpenUrlInContainer(BrowserWindowInterface* browser_window,
Navigate(&params);
}
void OpenContainerMenuOnPageActionView(BrowserWindowInterface* browser_window) {
// TODO(https://github.com/brave/brave-browser/issues/53350)
NOTIMPLEMENTED();
}
#endif
} // namespace brave
+4
View File
@@ -163,6 +163,10 @@ void OpenTabUrlInContainer(BrowserWindowInterface* browser_window,
void OpenUrlInContainer(BrowserWindowInterface* browser_window,
const GURL& url,
const containers::mojom::ContainerPtr& container);
// Opens the container menu on the page action view if the active tab is in a
// container.
void OpenContainerMenuOnPageActionView(BrowserWindowInterface* browser);
#endif
} // namespace brave
@@ -11,6 +11,7 @@
#include "chrome/test/views/chrome_views_test_base.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/events/event.h"
#include "ui/views/style/platform_style.h"
#include "ui/views/test/views_test_utils.h"
#include "ui/views/view_class_properties.h"
@@ -188,4 +189,30 @@ TEST_F(PageActionViewTest, OverrideHeightIgnoreSizeBounds) {
views::LayoutAlignment::kCenter);
}
TEST_F(PageActionViewTest, OverrideTriggerableEventUsesCallback) {
EXPECT_CALL(*model(), GetOverrideTriggerableEvent())
.WillRepeatedly(Return(ui::EF_RIGHT_MOUSE_BUTTON));
ui::MouseEvent left_press(ui::EventType::kMousePressed, gfx::Point(5, 5),
gfx::Point(5, 5), base::TimeTicks::Now(),
ui::EF_LEFT_MOUSE_BUTTON, ui::EF_LEFT_MOUSE_BUTTON);
ui::MouseEvent right_press(ui::EventType::kMousePressed, gfx::Point(5, 5),
gfx::Point(5, 5), base::TimeTicks::Now(),
ui::EF_RIGHT_MOUSE_BUTTON,
ui::EF_RIGHT_MOUSE_BUTTON);
page_action_view()->OnPageActionModelChanged(*model());
EXPECT_FALSE(page_action_view()->IsTriggerableEvent(left_press));
EXPECT_TRUE(page_action_view()->IsTriggerableEvent(right_press));
testing::Mock::VerifyAndClearExpectations(model());
EXPECT_CALL(*model(), GetOverrideTriggerableEvent())
.WillRepeatedly(Return(std::nullopt));
page_action_view()->OnPageActionModelChanged(*model());
EXPECT_TRUE(page_action_view()->IsTriggerableEvent(left_press));
EXPECT_FALSE(page_action_view()->IsTriggerableEvent(right_press));
testing::Mock::VerifyAndClearExpectations(model());
}
} // namespace page_actions
@@ -16,6 +16,7 @@
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/actions/chrome_action_id.h"
#include "content/public/browser/web_contents.h"
#include "ui/events/event_constants.h"
#include "ui/gfx/text_elider.h"
namespace page_actions {
@@ -84,6 +85,8 @@ void PartitionedStoragePageActionController::UpdatePageAction() {
page_action_controller_->ClearOverrideChipColors(
kActionShowPartitionedStorage);
page_action_controller_->ClearOverrideHeight(kActionShowPartitionedStorage);
page_action_controller_->SetOverrideTriggerableEvent(
kActionShowPartitionedStorage, std::nullopt);
return;
}
@@ -109,6 +112,8 @@ void PartitionedStoragePageActionController::UpdatePageAction() {
page_action_controller_->OverrideChipColors(
kActionShowPartitionedStorage, model->background_color(), SK_ColorWHITE);
page_action_controller_->SetOverrideHeight(kActionShowPartitionedStorage, 20);
page_action_controller_->SetOverrideTriggerableEvent(
kActionShowPartitionedStorage, ui::EF_RIGHT_MOUSE_BUTTON);
}
} // namespace page_actions
@@ -38,4 +38,11 @@ void PageActionControllerImpl::ClearOverrideHeight(
FindPageActionModel(action_id).SetOverrideHeight(PassKey(), std::nullopt);
}
void PageActionControllerImpl::SetOverrideTriggerableEvent(
actions::ActionId action_id,
std::optional<int> event_flags) {
FindPageActionModel(action_id).SetOverrideTriggerableEvent(PassKey(),
event_flags);
}
} // namespace page_actions
@@ -54,4 +54,18 @@ std::optional<int> PageActionModel::GetOverrideHeight() const {
return override_height_;
}
void PageActionModel::SetOverrideTriggerableEvent(
base::PassKey<PageActionController>,
std::optional<int> event_flags) {
if (override_triggerable_event_flags_ == event_flags) {
return;
}
override_triggerable_event_flags_ = event_flags;
NotifyChange(Property::kOverrideTriggerableEvent);
}
std::optional<int> PageActionModel::GetOverrideTriggerableEvent() const {
return override_triggerable_event_flags_;
}
} // namespace page_actions
@@ -8,6 +8,7 @@
#include <algorithm>
#include "chrome/browser/ui/views/page_action/page_action_model.h"
#include "ui/events/event_constants.h"
#include "ui/gfx/geometry/rect.h"
#include "ui/views/layout/layout_types.h"
#include "ui/views/layout/proposed_layout.h"
@@ -16,9 +17,11 @@
#define GetMinimumSize GetMinimumSize_Chromium
#define OnNewActiveController OnNewActiveController_Chromium
#define OnPageActionModelChanged OnPageActionModelChanged_Chromium
#include <chrome/browser/ui/views/page_action/page_action_view.cc>
#undef OnPageActionModelChanged
#undef OnNewActiveController
#undef GetMinimumSize
@@ -135,4 +138,17 @@ void PageActionView::OnNewActiveController(PageActionController* controller) {
OnPageActionModelVisualRefresh(observation_.GetSource());
}
void PageActionView::OnPageActionModelChanged(
const PageActionModelInterface& model) {
PageActionView::OnPageActionModelChanged_Chromium(model);
const PageActionModelInterface* source = observation_.GetSource();
if (source) {
// ui::EF_LEFT_MOUSE_BUTTON is the default triggerable event flags of Button
// class.
SetTriggerableEventFlags(source->GetOverrideTriggerableEvent().value_or(
ui::EF_LEFT_MOUSE_BUTTON));
}
}
} // namespace page_actions
@@ -7,6 +7,7 @@
#define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_PAGE_ACTION_PAGE_ACTION_VIEW_H_
#include "chrome/browser/ui/views/location_bar/icon_label_bubble_view.h"
#include "chrome/browser/ui/views/page_action/page_action_model_observer.h"
#include "ui/views/view.h"
#include "ui/views/widget/native_widget_delegate.h"
@@ -26,6 +27,11 @@
void OnPageActionModelVisualRefresh(PageActionModelInterface* model); \
bool ShouldAlwaysShowLabel()
// Make a OnPageActionModelChanged wrapper
#define OnPageActionModelChanged(...) \
OnPageActionModelChanged_Chromium(__VA_ARGS__); \
void OnPageActionModelChanged(__VA_ARGS__)
// Make a GetMinimumSize wrapper
#define GetMinimumSize() \
GetMinimumSize_Chromium() const; \
@@ -40,6 +46,7 @@
#undef OnNewActiveController
#undef GetMinimumSize
#undef OnPageActionModelChanged
#undef ShouldShowLabelAfterAnimation
#endif // BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_PAGE_ACTION_PAGE_ACTION_VIEW_H_
@@ -39,6 +39,10 @@ class MockPageActionController : public MockPageActionController_Chromium {
ClearOverrideHeight,
(actions::ActionId action_id),
(override));
MOCK_METHOD(void,
SetOverrideTriggerableEvent,
(actions::ActionId action_id, std::optional<int> event_flags),
(override));
};
} // namespace page_actions
@@ -41,6 +41,14 @@ class MockPageActionModel : public MockPageActionModel_Chromium {
(base::PassKey<PageActionController>, std::optional<int>),
(override));
MOCK_METHOD(std::optional<int>, GetOverrideHeight, (), (const, override));
MOCK_METHOD(void,
SetOverrideTriggerableEvent,
(base::PassKey<PageActionController>, std::optional<int>),
(override));
MOCK_METHOD(std::optional<int>,
GetOverrideTriggerableEvent,
(),
(const, override));
};
} // namespace page_actions
@@ -1,8 +1,8 @@
diff --git a/chrome/browser/ui/views/page_action/page_action_controller.h b/chrome/browser/ui/views/page_action/page_action_controller.h
index f8b60561923541d940cbc6c7fdee6fa0d025584f..04e8d51f72cbd59f23ba1b706fa27565b05e4efd 100644
index f8b60561923541d940cbc6c7fdee6fa0d025584f..69bd78e2632769ee82bf473c668576c660109869 100644
--- a/chrome/browser/ui/views/page_action/page_action_controller.h
+++ b/chrome/browser/ui/views/page_action/page_action_controller.h
@@ -109,6 +109,22 @@ class PageActionController {
@@ -109,6 +109,27 @@ class PageActionController {
public:
virtual ~PageActionController() = default;
@@ -21,11 +21,16 @@ index f8b60561923541d940cbc6c7fdee6fa0d025584f..04e8d51f72cbd59f23ba1b706fa27565
+ virtual void SetOverrideHeight(actions::ActionId action_id,
+ int height) = 0;
+ virtual void ClearOverrideHeight(actions::ActionId action_id) = 0;
+
+ // When set, `PageActionView::IsTriggerableEvent` uses this callback instead of
+ // the default logic. Pass nullopt to clear.
+ virtual void SetOverrideTriggerableEvent(actions::ActionId action_id,
+ std::optional<int> event_flags) = 0;
+
// Requests that the page action be shown or hidden.
virtual void Show(actions::ActionId action_id) = 0;
virtual void Hide(actions::ActionId action_id) = 0;
@@ -246,6 +262,16 @@ class PageActionControllerImpl : public PageActionController,
@@ -246,6 +267,18 @@ class PageActionControllerImpl : public PageActionController,
PageActionControllerImpl& operator=(const PageActionControllerImpl&) = delete;
~PageActionControllerImpl() override;
@@ -38,6 +43,8 @@ index f8b60561923541d940cbc6c7fdee6fa0d025584f..04e8d51f72cbd59f23ba1b706fa27565
+ void SetOverrideHeight(actions::ActionId action_id,
+ int height) override;
+ void ClearOverrideHeight(actions::ActionId action_id) override;
+ void SetOverrideTriggerableEvent(actions::ActionId action_id,
+ std::optional<int> event_flags) override;
+
void Initialize(
tabs::TabInterface& tab_interface,
@@ -1,8 +1,8 @@
diff --git a/chrome/browser/ui/views/page_action/page_action_model.h b/chrome/browser/ui/views/page_action/page_action_model.h
index a3d1abf58e71b25edb1487e2cf92c5587539d85b..daffa064f8e97b9ba1e6f0100dfd70cd1a491c28 100644
index a3d1abf58e71b25edb1487e2cf92c5587539d85b..e928a6a2c80bac025e6a64258fdc3c693e08562e 100644
--- a/chrome/browser/ui/views/page_action/page_action_model.h
+++ b/chrome/browser/ui/views/page_action/page_action_model.h
@@ -33,6 +33,19 @@ class PageActionModelInterface {
@@ -33,6 +33,22 @@ class PageActionModelInterface {
public:
PageActionModelInterface() = default;
virtual ~PageActionModelInterface() = default;
@@ -14,15 +14,18 @@ index a3d1abf58e71b25edb1487e2cf92c5587539d85b..daffa064f8e97b9ba1e6f0100dfd70cd
+ bool always_show) = 0;
+ virtual void SetOverrideHeight(base::PassKey<PageActionController>,
+ std::optional<int> height) = 0;
+ virtual void SetOverrideTriggerableEvent(base::PassKey<PageActionController>,
+ std::optional<int> event_flags) = 0;
+
+ virtual std::optional<SkColor> GetOverrideBackgroundColor() const = 0;
+ virtual std::optional<SkColor> GetOverrideForegroundColor() const = 0;
+ virtual bool GetAlwaysShowLabel() const = 0;
+ virtual std::optional<int> GetOverrideHeight() const = 0;
+ virtual std::optional<int> GetOverrideTriggerableEvent() const = 0;
virtual void AddObserver(PageActionModelObserver* observer) = 0;
virtual void RemoveObserver(PageActionModelObserver* observer) = 0;
@@ -120,6 +133,27 @@ class PageActionModel : public PageActionModelInterface {
@@ -120,6 +136,31 @@ class PageActionModel : public PageActionModelInterface {
PageActionModel& operator=(const PageActionModel&) = delete;
~PageActionModel() override;
@@ -34,29 +37,34 @@ index a3d1abf58e71b25edb1487e2cf92c5587539d85b..daffa064f8e97b9ba1e6f0100dfd70cd
+ bool always_show) override;
+ void SetOverrideHeight(base::PassKey<PageActionController>,
+ std::optional<int> height_px) override;
+ void SetOverrideTriggerableEvent(base::PassKey<PageActionController>,
+ std::optional<int> event_flags) override;
+ std::optional<SkColor> GetOverrideBackgroundColor() const override;
+ std::optional<SkColor> GetOverrideForegroundColor() const override;
+ bool GetAlwaysShowLabel() const override;
+ std::optional<int> GetOverrideHeight() const override;
+ std::optional<int> GetOverrideTriggerableEvent() const override;
+
+ private:
+ std::optional<SkColor> override_background_color_;
+ std::optional<SkColor> override_foreground_color_;
+ bool always_show_label_ = false;
+ std::optional<int> override_height_;
+ std::optional<int> override_triggerable_event_flags_;
+
+ public:
+
void AddObserver(PageActionModelObserver* observer) override;
void RemoveObserver(PageActionModelObserver* observer) override;
@@ -210,6 +244,9 @@ class PageActionModel : public PageActionModelInterface {
@@ -210,6 +251,10 @@ class PageActionModel : public PageActionModelInterface {
// per-property reentrancy checks.
enum class Property {
kShowRequested,
+ kAlwaysShowLabel,
+ kOverrideChipColors,
+ kOverrideHeight,
+ kOverrideTriggerableEvent,
kShouldShowSuggestionChip,
kSuggestionChipConfig,
kTabActive,
@@ -22,7 +22,12 @@ replace = '''\1
// Override the height of the PageActionView
virtual void SetOverrideHeight(actions::ActionId action_id,
int height) = 0;
virtual void ClearOverrideHeight(actions::ActionId action_id) = 0;'''
virtual void ClearOverrideHeight(actions::ActionId action_id) = 0;
// When set, `PageActionView::IsTriggerableEvent` uses this callback instead of
// the default logic. Pass nullopt to clear.
virtual void SetOverrideTriggerableEvent(actions::ActionId action_id,
std::optional<int> event_flags) = 0;'''
[[substitution]]
@@ -38,4 +43,6 @@ replace = '''\1
void ClearOverrideChipColors(actions::ActionId action_id) override;
void SetOverrideHeight(actions::ActionId action_id,
int height) override;
void ClearOverrideHeight(actions::ActionId action_id) override;'''
void ClearOverrideHeight(actions::ActionId action_id) override;
void SetOverrideTriggerableEvent(actions::ActionId action_id,
std::optional<int> event_flags) override;'''
@@ -15,11 +15,14 @@ replace = '''\1
bool always_show) = 0;
virtual void SetOverrideHeight(base::PassKey<PageActionController>,
std::optional<int> height) = 0;
virtual void SetOverrideTriggerableEvent(base::PassKey<PageActionController>,
std::optional<int> event_flags) = 0;
virtual std::optional<SkColor> GetOverrideBackgroundColor() const = 0;
virtual std::optional<SkColor> GetOverrideForegroundColor() const = 0;
virtual bool GetAlwaysShowLabel() const = 0;
virtual std::optional<int> GetOverrideHeight() const = 0;'''
virtual std::optional<int> GetOverrideHeight() const = 0;
virtual std::optional<int> GetOverrideTriggerableEvent() const = 0;'''
[[substitution]]
description = 'Implementing the interfaces in PageActionModel'
@@ -34,16 +37,20 @@ replace = '''\1
bool always_show) override;
void SetOverrideHeight(base::PassKey<PageActionController>,
std::optional<int> height_px) override;
void SetOverrideTriggerableEvent(base::PassKey<PageActionController>,
std::optional<int> event_flags) override;
std::optional<SkColor> GetOverrideBackgroundColor() const override;
std::optional<SkColor> GetOverrideForegroundColor() const override;
bool GetAlwaysShowLabel() const override;
std::optional<int> GetOverrideHeight() const override;
std::optional<int> GetOverrideTriggerableEvent() const override;
private:
std::optional<SkColor> override_background_color_;
std::optional<SkColor> override_foreground_color_;
bool always_show_label_ = false;
std::optional<int> override_height_;
std::optional<int> override_triggerable_event_flags_;
public:'''
@@ -53,4 +60,5 @@ re_pattern = '(\s+kShowRequested,)'
replace = '''\1
kAlwaysShowLabel,
kOverrideChipColors,
kOverrideHeight,'''
kOverrideHeight,
kOverrideTriggerableEvent,'''