Discharge7443/brave-core
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove padding around container page action icon and text (#36393)

Browse Source 
* Remove padding around container page action icon and text

We don't need theses padding on the chip for partitioned storage page action.
This commit is contained in:
Sangwoo Ko
2026-06-01 07:53:51 +09:00
committed by GitHub
parent d7779f4c4d
commit d7cf8bd4eb
12 changed files with 121 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
browser/ui/views/page_action/page_action_view_unittest.cc
+22
View File
@@ -266,4 +266,26 @@ TEST_F(PageActionViewTest, OverrideBackgroundColorReturnsModelValue) {
EXPECT_EQ(page_action_view()->GetBackgroundColor(), SK_ColorRED);
}
TEST_F(PageActionViewTest, OverrideBorderReturnsModelValue) {
EXPECT_CALL(*model(), GetOverrideBorder())
.WillRepeatedly(
Return(std::optional<gfx::Insets>(gfx::Insets::TLBR(1, 2, 3, 4))));
page_action_view()->OnPageActionModelChanged(*model());
EXPECT_EQ(page_action_view()->GetBorder()->GetInsets(),
gfx::Insets::TLBR(1, 2, 3, 4));
testing::Mock::VerifyAndClearExpectations(model());
// Even empty border should be respected
EXPECT_CALL(*model(), GetOverrideBorder())
.WillRepeatedly(Return(std::optional<gfx::Insets>(gfx::Insets())));
page_action_view()->OnPageActionModelChanged(*model());
EXPECT_EQ(page_action_view()->GetBorder()->GetInsets(),
gfx::Insets::TLBR(0, 0, 0, 0));
}
} // namespace page_actions
browser/ui/views/page_action/partitioned_storage_page_action_controller.cc
+5
View File
@@ -178,6 +178,7 @@ void PartitionedStoragePageActionController::UpdatePageAction() {
page_action_controller_->ClearOverrideHeight(kActionShowPartitionedStorage);
page_action_controller_->SetOverrideTriggerableEvent(
kActionShowPartitionedStorage, std::nullopt);
page_action_controller_->ClearOverrideBorder(kActionShowPartitionedStorage);
return;
}
@@ -207,6 +208,10 @@ void PartitionedStoragePageActionController::UpdatePageAction() {
page_action_controller_->SetOverrideHeight(kActionShowPartitionedStorage, 20);
page_action_controller_->SetOverrideTriggerableEvent(
kActionShowPartitionedStorage, ui::EF_RIGHT_MOUSE_BUTTON);
// Sets empty insets so that we can get rid of additional padding around icon
// and text.
page_action_controller_->SetOverrideBorder(kActionShowPartitionedStorage,
gfx::Insets());
}
void PartitionedStoragePageActionController::OnPartitionedStorageMenuClosed() {
chromium_src/chrome/browser/ui/page_actions/page_action_controller.cc
+11
View File
@@ -52,6 +52,17 @@ void PageActionControllerImpl::SetOverrideTriggerableEvent(
PageActionPassKey(), event_flags);
}
void PageActionControllerImpl::SetOverrideBorder(actions::ActionId action_id,
const gfx::Insets& border) {
FindPageActionModel(action_id).SetOverrideBorder(PageActionPassKey(), border);
}
void PageActionControllerImpl::ClearOverrideBorder(
actions::ActionId action_id) {
FindPageActionModel(action_id).SetOverrideBorder(PageActionPassKey(),
std::nullopt);
}
std::unique_ptr<PageActionModelInterface> PageActionControllerImpl::CreateModel(
actions::ActionId action_id,
bool is_ephemeral) {
chromium_src/chrome/browser/ui/page_actions/page_action_controller.h
+7
View File
@@ -6,6 +6,10 @@
#ifndef BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_PAGE_ACTIONS_PAGE_ACTION_CONTROLLER_H_
#define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_PAGE_ACTIONS_PAGE_ACTION_CONTROLLER_H_
namespace gfx {
class Insets;
} // namespace gfx
namespace page_actions {
class PageActionControllerImpl;
} // namespace page_actions
@@ -32,6 +36,9 @@ class PageActionControllerImpl
void ClearOverrideHeight(actions::ActionId action_id);
void SetOverrideTriggerableEvent(actions::ActionId action_id,
std::optional<int> event_flags);
void SetOverrideBorder(actions::ActionId action_id,
const gfx::Insets& border);
void ClearOverrideBorder(actions::ActionId action_id);
// chromium_impl::PageActionControllerImpl:
std::unique_ptr<PageActionModelInterface> CreateModel(
chromium_src/chrome/browser/ui/page_actions/page_action_model.cc
+13
View File
@@ -69,4 +69,17 @@ std::optional<int> PageActionModel::GetOverrideTriggerableEvent() const {
return override_triggerable_event_flags_;
}
void PageActionModel::SetOverrideBorder(PageActionPassKey,
std::optional<gfx::Insets> border) {
if (override_border_ == border) {
return;
}
override_border_ = border;
NotifyChange(Property::kOverrideBorder);
}
std::optional<gfx::Insets> PageActionModel::GetOverrideBorder() const {
return override_border_;
}
} // namespace page_actions
chromium_src/chrome/browser/ui/page_actions/page_action_model.h
+6
View File
@@ -6,6 +6,8 @@
#ifndef BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_PAGE_ACTIONS_PAGE_ACTION_MODEL_H_
#define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_PAGE_ACTIONS_PAGE_ACTION_MODEL_H_
#include "ui/gfx/geometry/insets.h"
namespace page_actions {
class PageActionModel;
} // namespace page_actions
@@ -32,11 +34,14 @@ class PageActionModel : public chromium_impl::PageActionModel {
std::optional<int> height_px) override;
void SetOverrideTriggerableEvent(PageActionPassKey,
std::optional<int> event_flags) override;
void SetOverrideBorder(PageActionPassKey,
std::optional<gfx::Insets> border) 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;
std::optional<gfx::Insets> GetOverrideBorder() const override;
private:
std::optional<SkColor> override_background_color_;
@@ -44,6 +49,7 @@ class PageActionModel : public chromium_impl::PageActionModel {
bool always_show_label_ = false;
std::optional<int> override_height_;
std::optional<int> override_triggerable_event_flags_;
std::optional<gfx::Insets> override_border_;
};
} // namespace page_actions
chromium_src/chrome/browser/ui/page_actions/test_support/mock_page_action_model.h
+8
View File
@@ -46,6 +46,14 @@ class MockPageActionModel : public MockPageActionModel_Chromium {
GetOverrideTriggerableEvent,
(),
(const, override));
MOCK_METHOD(void,
SetOverrideBorder,
(PageActionPassKey, std::optional<gfx::Insets>),
(override));
MOCK_METHOD(std::optional<gfx::Insets>,
GetOverrideBorder,
(),
(const, override));
};
} // namespace page_actions
chromium_src/chrome/browser/ui/views/page_action/page_action_view.cc
+14
View File
@@ -15,6 +15,17 @@
#include "ui/views/style/platform_style.h"
#include "ui/views/view_class_properties.h"
namespace {
void MaybeOverrideBorder(const page_actions::PageActionModelInterface* source,
gfx::Insets& border_insets) {
if (source && source->GetOverrideBorder().has_value()) {
border_insets = source->GetOverrideBorder().value();
}
}
} // namespace
#define GetMinimumSize GetMinimumSize_Chromium
#define OnNewActiveController OnNewActiveController_Chromium
#define OnPageActionModelChanged OnPageActionModelChanged_Chromium
@@ -71,6 +82,8 @@ void PageActionView::OnPageActionModelVisualRefresh(
} else {
ClearProperty(views::kCrossAxisAlignmentKey);
}
UpdateBorder();
}
gfx::Size PageActionView::GetSizeForLabelWidth(int label_width) const {
@@ -153,6 +166,7 @@ void PageActionView::OnPageActionModelChanged(
SetTriggerableEventFlags(source->GetOverrideTriggerableEvent().value_or(
ui::EF_LEFT_MOUSE_BUTTON));
}
UpdateBorder();
}
} // namespace page_actions
patches/chrome-browser-ui-page_actions-page_action_model.h.patch
+10 -6
View File
@@ -1,8 +1,8 @@
diff --git a/chrome/browser/ui/page_actions/page_action_model.h b/chrome/browser/ui/page_actions/page_action_model.h
index 7dacb805a0c88fc8d78095f29550e70375639634..3924212242cd280f61a77fe97bf2c83353aff35b 100644
index 7dacb805a0c88fc8d78095f29550e70375639634..a8bb1e8007b1bf5ed6fc755084cc6e21bae7058e 100644
--- a/chrome/browser/ui/page_actions/page_action_model.h
+++ b/chrome/browser/ui/page_actions/page_action_model.h
@@ -118,9 +118,25 @@ class PageActionModelInterface {
@@ -118,9 +118,28 @@ class PageActionModelInterface {
virtual PageActionColorSource GetColorSource() const = 0;
virtual bool IsEphemeral() const = 0;
@@ -16,11 +16,14 @@ index 7dacb805a0c88fc8d78095f29550e70375639634..3924212242cd280f61a77fe97bf2c833
+ std::optional<int> height) = 0;
+ virtual void SetOverrideTriggerableEvent(PageActionPassKey,
+ std::optional<int> event_flags) = 0;
+ virtual void SetOverrideBorder(PageActionPassKey,
+ std::optional<gfx::Insets> border) = 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 std::optional<gfx::Insets> GetOverrideBorder() const = 0;
};
// PageActionModel represents the page action's state, scoped to a single tab.
@@ -28,7 +31,7 @@ index 7dacb805a0c88fc8d78095f29550e70375639634..3924212242cd280f61a77fe97bf2c833
class PageActionModel : public PageActionModelInterface {
public:
explicit PageActionModel(actions::ActionId action_id,
@@ -217,6 +233,7 @@ class PageActionModel : public PageActionModelInterface {
@@ -217,6 +236,7 @@ class PageActionModel : public PageActionModelInterface {
bool IsEphemeral() const override;
private:
@@ -36,7 +39,7 @@ index 7dacb805a0c88fc8d78095f29550e70375639634..3924212242cd280f61a77fe97bf2c833
// Identifies which property triggered a NotifyChange call, used for
// per-property reentrancy checks.
enum class Property {
@@ -239,7 +256,11 @@ class PageActionModel : public PageActionModelInterface {
@@ -239,7 +259,12 @@ class PageActionModel : public PageActionModelInterface {
kAnchoredMessageActionIcon,
kIsAnchoredMessageShowing,
kAnchoredMessageIcon,
@@ -45,11 +48,12 @@ index 7dacb805a0c88fc8d78095f29550e70375639634..3924212242cd280f61a77fe97bf2c833
+ kOverrideChipColors,
+ kOverrideHeight,
+ kOverrideTriggerableEvent,
+ kMaxValue = kOverrideTriggerableEvent,
+ kOverrideBorder,
+ kMaxValue = kOverrideBorder,
};
using PropertySet =
base::EnumSet<Property, Property::kShowRequested, Property::kMaxValue>;
@@ -339,6 +360,7 @@ class PageActionModel : public PageActionModelInterface {
@@ -339,6 +364,7 @@ class PageActionModel : public PageActionModelInterface {
base::ObserverList<PageActionModelObserver> observer_list_;
};
patches/chrome-browser-ui-views-page_action-page_action_view.cc.patch
+9 -1
View File
@@ -1,5 +1,5 @@
diff --git a/chrome/browser/ui/views/page_action/page_action_view.cc b/chrome/browser/ui/views/page_action/page_action_view.cc
index 271f5b9adba10ee60aa45b4c00e1bfb541fd05fd..1a20372af0948f87a8d94a99ea287c039e10d126 100644
index 271f5b9adba10ee60aa45b4c00e1bfb541fd05fd..8051adc18865ad5e5d465de28336e51545b45ab8 100644
--- a/chrome/browser/ui/views/page_action/page_action_view.cc
+++ b/chrome/browser/ui/views/page_action/page_action_view.cc
@@ -167,7 +167,7 @@ void PageActionView::OnPageActionModelChanged(
@@ -11,3 +11,11 @@ index 271f5b9adba10ee60aa45b4c00e1bfb541fd05fd..1a20372af0948f87a8d94a99ea287c03
if (model.GetShouldAnimateChipIn()) {
AnimateIn(/*string_id=*/std::nullopt);
} else {
@@ -232,6 +232,7 @@ void PageActionView::UpdateBorder() {
!observation_.GetSource()->GetImage().IsVectorIcon()) {
border_insets = GetInsetsForNonVectorIcon();
}
+ MaybeOverrideBorder(observation_.GetSource(), border_insets);
SetBorder(views::CreateEmptyBorder(border_insets));
}
rewrite/chrome/browser/ui/page_actions/page_action_model.h.toml
+6 -2
View File
@@ -26,11 +26,14 @@ replace = '''\1
std::optional<int> height) = 0;
virtual void SetOverrideTriggerableEvent(PageActionPassKey,
std::optional<int> event_flags) = 0;
virtual void SetOverrideBorder(PageActionPassKey,
std::optional<gfx::Insets> border) = 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;\2'''
virtual std::optional<int> GetOverrideTriggerableEvent() const = 0;
virtual std::optional<gfx::Insets> GetOverrideBorder() const = 0;\2'''
[[substitution]]
description = '''
@@ -46,7 +49,8 @@ replace = '''\1 kAlwaysShowLabel,
kOverrideChipColors,
kOverrideHeight,
kOverrideTriggerableEvent,
kMaxValue = kOverrideTriggerableEvent,'''
kOverrideBorder,
kMaxValue = kOverrideBorder,'''
[[substitution]]
description = '''Wrap PageActionModel in a chromium_impl namespace.
rewrite/chrome/browser/ui/views/page_action/page_action_view.cc.toml
+10
View File
@@ -7,3 +7,13 @@
description = 'Adding code to force show the label if it should be always shown in OnPageActionModelChanged()'
re_pattern = 'else if \((model.ShouldShowSuggestionChip\(\))\)'
replace = 'else if (\1 || model.GetAlwaysShowLabel())'
[[substitution]]
description = '''
Call MaybeOverrideBorder() in UpdateBorder() right before the SetBorder() call
in it.
'''
re_pattern = '(void PageActionView::UpdateBorder\(\).*?)(SetBorder\()'
re_flags = ['DOTALL']
replace = '''\1MaybeOverrideBorder(observation_.GetSource(), border_insets);
\2'''