Skip to content

Commit ca477fe

Browse files
aguscruizaguscruiz
committed
Address feedback
1 parent 6a79645 commit ca477fe

File tree

8 files changed

+43
-130
lines changed

8 files changed

+43
-130
lines changed

‎browser/ui/views/bookmarks/brave_bookmark_bar_view.cc‎

Lines changed: 16 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -30,60 +30,45 @@ BraveBookmarkBarView::BraveBookmarkBarView(Browser* browser,
3030

3131
BraveBookmarkBarView::~BraveBookmarkBarView() = default;
3232

33-
void BraveBookmarkBarView::AddedToWidget() {
34-
BookmarkBarView::AddedToWidget();
35-
paint_as_active_subscription_ =
36-
GetWidget()->RegisterPaintAsActiveChangedCallback(base::BindRepeating(
37-
&BraveBookmarkBarView::UpdateAppearanceForTheme,
38-
weak_ptr_factory_.GetWeakPtr()));
39-
}
40-
41-
void BraveBookmarkBarView::RemovedFromWidget() {
42-
paint_as_active_subscription_ = {};
43-
BookmarkBarView::RemovedFromWidget();
44-
}
45-
46-
namespace {
47-
48-
ui::ColorId GetFolderIconColorId(views::Widget* widget) {
49-
const bool is_active = widget && widget->ShouldPaintAsActive();
50-
return is_active ? kColorToolbarButtonIcon : kColorToolbarButtonIconInactive;
51-
}
52-
53-
} // namespace
54-
5533
void BraveBookmarkBarView::ConfigureButton(
5634
const bookmarks::BookmarkNode* node,
5735
views::LabelButton* button) {
5836
BookmarkBarView::ConfigureButton(node, button);
59-
if (node->is_folder() && GetColorProvider()) {
37+
if (node->is_folder()) {
6038
button->SetImageModel(
6139
views::Button::STATE_NORMAL,
6240
chrome::GetBookmarkFolderIcon(chrome::BookmarkFolderIconType::kNormal,
63-
GetFolderIconColorId(GetWidget())));
41+
kColorToolbarButtonIcon));
42+
button->SetImageModel(
43+
views::Button::STATE_DISABLED,
44+
chrome::GetBookmarkFolderIcon(chrome::BookmarkFolderIconType::kNormal,
45+
kColorToolbarButtonIconInactive));
6446
}
6547
}
6648

6749
void BraveBookmarkBarView::UpdateAppearanceForTheme() {
6850
BookmarkBarView::UpdateAppearanceForTheme();
69-
if (!GetColorProvider()) {
70-
return;
71-
}
72-
73-
const ui::ColorId color_id = GetFolderIconColorId(GetWidget());
7451

7552
if (all_bookmarks_button_) {
7653
all_bookmarks_button_->SetImageModel(
7754
views::Button::STATE_NORMAL,
7855
chrome::GetBookmarkFolderIcon(chrome::BookmarkFolderIconType::kNormal,
79-
color_id));
56+
kColorToolbarButtonIcon));
57+
all_bookmarks_button_->SetImageModel(
58+
views::Button::STATE_DISABLED,
59+
chrome::GetBookmarkFolderIcon(chrome::BookmarkFolderIconType::kNormal,
60+
kColorToolbarButtonIconInactive));
8061
}
8162

8263
if (managed_bookmarks_button_) {
8364
managed_bookmarks_button_->SetImageModel(
8465
views::Button::STATE_NORMAL,
8566
chrome::GetBookmarkFolderIcon(chrome::BookmarkFolderIconType::kManaged,
86-
color_id));
67+
kColorToolbarButtonIcon));
68+
managed_bookmarks_button_->SetImageModel(
69+
views::Button::STATE_DISABLED,
70+
chrome::GetBookmarkFolderIcon(chrome::BookmarkFolderIconType::kManaged,
71+
kColorToolbarButtonIconInactive));
8772
}
8873
}
8974

‎browser/ui/views/bookmarks/brave_bookmark_bar_view.h‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@
66
#ifndef BRAVE_BROWSER_UI_VIEWS_BOOKMARKS_BRAVE_BOOKMARK_BAR_VIEW_H_
77
#define BRAVE_BROWSER_UI_VIEWS_BOOKMARKS_BRAVE_BOOKMARK_BAR_VIEW_H_
88

9-
#include "base/callback_list.h"
10-
#include "base/memory/weak_ptr.h"
119
#include "chrome/browser/ui/views/bookmarks/bookmark_bar_view.h"
1210
#include "components/prefs/pref_member.h"
1311
#include "ui/base/metadata/metadata_header_macros.h"
@@ -25,19 +23,13 @@ class BraveBookmarkBarView : public BookmarkBarView {
2523
views::LabelButton* button) override;
2624
void UpdateAppearanceForTheme() override;
2725

28-
// views::View:
29-
void AddedToWidget() override;
30-
void RemovedFromWidget() override;
31-
3226
private:
3327
// Note that so-called "Others button" is renamed to "All bookmarks button"
3428
void OnShowAllBookmarksButtonPrefChanged();
3529

3630
void MaybeUpdateOtherAndManagedButtonsVisibility();
3731

3832
BooleanPrefMember show_all_bookmarks_button_pref_;
39-
base::CallbackListSubscription paint_as_active_subscription_;
40-
base::WeakPtrFactory<BraveBookmarkBarView> weak_ptr_factory_{this};
4133
};
4234

4335
#endif // BRAVE_BROWSER_UI_VIEWS_BOOKMARKS_BRAVE_BOOKMARK_BAR_VIEW_H_

‎browser/ui/views/toolbar/bookmark_button.cc‎

Lines changed: 6 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -31,34 +31,16 @@ void BraveBookmarkButton::SetToggled(bool on) {
3131
UpdateImageAndText();
3232
}
3333

34-
void BraveBookmarkButton::AddedToWidget() {
35-
ToolbarButton::AddedToWidget();
36-
paint_as_active_subscription_ =
37-
GetWidget()->RegisterPaintAsActiveChangedCallback(
38-
base::BindRepeating(&BraveBookmarkButton::UpdateImageAndText,
39-
weak_ptr_factory_.GetWeakPtr()));
40-
}
41-
42-
void BraveBookmarkButton::RemovedFromWidget() {
43-
paint_as_active_subscription_ = {};
44-
ToolbarButton::RemovedFromWidget();
45-
}
46-
4734
void BraveBookmarkButton::UpdateImageAndText() {
48-
const ui::ColorProvider* color_provider = GetColorProvider();
49-
if (!color_provider) {
50-
return;
51-
}
52-
53-
const bool is_active = GetWidget() && GetWidget()->ShouldPaintAsActive();
54-
const ui::ColorId color_id = is_active ? kColorToolbarButtonIcon
55-
: kColorToolbarButtonIconInactive;
56-
SkColor icon_color = color_provider->GetColor(color_id);
5735
const gfx::VectorIcon& icon = active_ ? omnibox::kStarActiveChromeRefreshIcon
5836
: omnibox::kStarChromeRefreshIcon;
37+
SetImageModel(views::Button::STATE_NORMAL,
38+
ui::ImageModel::FromVectorIcon(icon, kColorToolbarButtonIcon,
39+
GetIconSize()));
5940
SetImageModel(
60-
views::Button::STATE_NORMAL,
61-
ui::ImageModel::FromVectorIcon(icon, icon_color, GetIconSize()));
41+
views::Button::STATE_DISABLED,
42+
ui::ImageModel::FromVectorIcon(icon, kColorToolbarButtonIconInactive,
43+
GetIconSize()));
6244

6345
int tooltip_id = active_ ? IDS_TOOLTIP_STARRED : IDS_TOOLTIP_STAR;
6446
SetTooltipText(l10n_util::GetStringUTF16(tooltip_id));

‎browser/ui/views/toolbar/bookmark_button.h‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@
66
#ifndef BRAVE_BROWSER_UI_VIEWS_TOOLBAR_BOOKMARK_BUTTON_H_
77
#define BRAVE_BROWSER_UI_VIEWS_TOOLBAR_BOOKMARK_BUTTON_H_
88

9-
#include "base/callback_list.h"
10-
#include "base/memory/weak_ptr.h"
119
#include "chrome/browser/ui/views/toolbar/toolbar_button.h"
1210
#include "ui/base/metadata/metadata_header_macros.h"
1311

@@ -22,14 +20,8 @@ class BraveBookmarkButton : public ToolbarButton {
2220
void SetToggled(bool on);
2321
void UpdateImageAndText();
2422

25-
// views::View:
26-
void AddedToWidget() override;
27-
void RemovedFromWidget() override;
28-
2923
private:
3024
bool active_ = false;
31-
base::CallbackListSubscription paint_as_active_subscription_;
32-
base::WeakPtrFactory<BraveBookmarkButton> weak_ptr_factory_{this};
3325
};
3426

3527
#endif // BRAVE_BROWSER_UI_VIEWS_TOOLBAR_BOOKMARK_BUTTON_H_

‎browser/ui/views/toolbar/brave_vpn_button.cc‎

Lines changed: 6 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -198,19 +198,6 @@ BraveVPNButton::BraveVPNButton(Browser* browser)
198198

199199
BraveVPNButton::~BraveVPNButton() = default;
200200

201-
void BraveVPNButton::AddedToWidget() {
202-
ToolbarButton::AddedToWidget();
203-
paint_as_active_subscription_ =
204-
GetWidget()->RegisterPaintAsActiveChangedCallback(
205-
base::BindRepeating(&BraveVPNButton::UpdateColorsAndInsets,
206-
weak_ptr_factory_.GetWeakPtr()));
207-
}
208-
209-
void BraveVPNButton::RemovedFromWidget() {
210-
paint_as_active_subscription_ = {};
211-
ToolbarButton::RemovedFromWidget();
212-
}
213-
214201
void BraveVPNButton::OnConnectionStateChanged(ConnectionState state) {
215202
if (IsErrorState() && (state == ConnectionState::CONNECTING ||
216203
state == ConnectionState::DISCONNECTING)) {
@@ -284,16 +271,12 @@ void BraveVPNButton::UpdateColorsAndInsets() {
284271
kLeoProductVpnIcon, GetIconSize(), GetIconColor())));
285272
gfx::ImageSkia vpn_image(std::move(image_source), kImageSizeWithBadge);
286273

287-
const bool is_active = GetWidget() && GetWidget()->ShouldPaintAsActive();
288-
if (is_active) {
289-
SetImageModel(views::Button::STATE_NORMAL,
290-
ui::ImageModel::FromImageSkia(vpn_image));
291-
} else {
292-
SetImageModel(views::Button::STATE_NORMAL,
293-
ui::ImageModel::FromImageSkia(
294-
gfx::ImageSkiaOperations::CreateTransparentImage(
295-
vpn_image, kBraveDisabledControlAlpha / 255.0)));
296-
}
274+
SetImageModel(views::Button::STATE_NORMAL,
275+
ui::ImageModel::FromImageSkia(vpn_image));
276+
SetImageModel(views::Button::STATE_DISABLED,
277+
ui::ImageModel::FromImageSkia(
278+
gfx::ImageSkiaOperations::CreateTransparentImage(
279+
vpn_image, kBraveDisabledControlAlpha / 255.0)));
297280
}
298281

299282
SkColor BraveVPNButton::GetIconColor() {

‎browser/ui/views/toolbar/brave_vpn_button.h‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
#include <optional>
1111
#include <string>
1212

13-
#include "base/callback_list.h"
1413
#include "base/memory/raw_ptr.h"
1514
#include "brave/components/brave_vpn/browser/brave_vpn_service_observer.h"
1615
#include "chrome/browser/ui/views/toolbar/toolbar_button.h"
@@ -46,10 +45,6 @@ class BraveVPNButton : public ToolbarButton,
4645
brave_vpn::mojom::PurchasedState state,
4746
const std::optional<std::string>& description) override;
4847

49-
// views::View:
50-
void AddedToWidget() override;
51-
void RemovedFromWidget() override;
52-
5348
private:
5449
friend class brave_vpn::BraveVpnButtonUnitTest;
5550

@@ -82,7 +77,6 @@ class BraveVPNButton : public ToolbarButton,
8277
raw_ptr<Browser, DanglingUntriaged> browser_ = nullptr;
8378
raw_ptr<brave_vpn::BraveVpnService, DanglingUntriaged> service_ = nullptr;
8479
raw_ptr<views::MenuButtonController> menu_button_controller_ = nullptr;
85-
base::CallbackListSubscription paint_as_active_subscription_;
8680
base::WeakPtrFactory<BraveVPNButton> weak_ptr_factory_{this};
8781
};
8882

‎browser/ui/views/toolbar/wallet_button.cc‎

Lines changed: 15 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -157,16 +157,6 @@ void WalletButton::AddedToWidget() {
157157
if (notification_source_) {
158158
notification_source_->Init();
159159
}
160-
paint_as_active_subscription_ =
161-
GetWidget()->RegisterPaintAsActiveChangedCallback(
162-
base::BindRepeating(&WalletButton::UpdateImageAndText,
163-
weak_ptr_factory_.GetWeakPtr(),
164-
/*activated=*/false));
165-
}
166-
167-
void WalletButton::RemovedFromWidget() {
168-
paint_as_active_subscription_ = {};
169-
ToolbarButton::RemovedFromWidget();
170160
}
171161

172162
void WalletButton::OnWalletPressed(const ui::Event& event) {
@@ -217,20 +207,21 @@ void WalletButton::UpdateImageAndText(bool activated) {
217207
return;
218208
}
219209

220-
const bool is_active = GetWidget() && GetWidget()->ShouldPaintAsActive();
221-
222-
ui::ColorId color_id = kColorToolbarButtonIcon;
223-
if (activated) {
224-
color_id = kColorToolbarButtonActivated;
225-
} else if (!is_active) {
226-
color_id = kColorToolbarButtonIconInactive;
227-
}
210+
ui::ColorId color_id =
211+
activated ? static_cast<ui::ColorId>(kColorToolbarButtonActivated)
212+
: static_cast<ui::ColorId>(kColorToolbarButtonIcon);
228213

229214
if (counter_ == 0) {
230215
SetImageModel(views::Button::STATE_NORMAL,
231216
ui::ImageModel::FromVectorIcon(
232217
kLeoProductBraveWalletIcon,
233218
color_provider->GetColor(color_id), GetIconSize()));
219+
SetImageModel(
220+
views::Button::STATE_DISABLED,
221+
ui::ImageModel::FromVectorIcon(
222+
kLeoProductBraveWalletIcon,
223+
color_provider->GetColor(kColorToolbarButtonIconInactive),
224+
GetIconSize()));
234225
return;
235226
}
236227

@@ -255,15 +246,12 @@ void WalletButton::UpdateImageAndText(bool activated) {
255246
text, brave::kBadgeTextColor, brave::kBadgeNotificationBG));
256247
gfx::ImageSkia badge_image(std::move(image_source), preferred_size);
257248

258-
if (is_active) {
259-
SetImageModel(views::Button::STATE_NORMAL,
260-
ui::ImageModel::FromImageSkia(badge_image));
261-
} else {
262-
SetImageModel(views::Button::STATE_NORMAL,
263-
ui::ImageModel::FromImageSkia(
264-
gfx::ImageSkiaOperations::CreateTransparentImage(
265-
badge_image, kBraveDisabledControlAlpha / 255.0)));
266-
}
249+
SetImageModel(views::Button::STATE_NORMAL,
250+
ui::ImageModel::FromImageSkia(badge_image));
251+
SetImageModel(views::Button::STATE_DISABLED,
252+
ui::ImageModel::FromImageSkia(
253+
gfx::ImageSkiaOperations::CreateTransparentImage(
254+
badge_image, kBraveDisabledControlAlpha / 255.0)));
267255
}
268256

269257
void WalletButton::ShowWalletBubble() {

‎browser/ui/views/toolbar/wallet_button.h‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
#include <memory>
1010
#include <string>
1111

12-
#include "base/callback_list.h"
1312
#include "base/memory/raw_ptr.h"
1413
#include "brave/browser/ui/views/toolbar/wallet_button_notification_source.h"
1514
#include "chrome/browser/ui/views/toolbar/toolbar_button.h"
@@ -41,7 +40,6 @@ class WalletButton : public ToolbarButton {
4140
private:
4241
// views::View:
4342
void AddedToWidget() override;
44-
void RemovedFromWidget() override;
4543

4644
std::string GetBadgeText();
4745
void OnWalletPressed(const ui::Event& event);
@@ -61,7 +59,6 @@ class WalletButton : public ToolbarButton {
6159
size_t counter_ = 0;
6260
bool show_suggest_badge_ = false;
6361

64-
base::CallbackListSubscription paint_as_active_subscription_;
6562
base::WeakPtrFactory<WalletButton> weak_ptr_factory_{this};
6663
};
6764

0 commit comments

Comments
 (0)