Skip to content

Commit

Permalink
[UI] Remove click_target_id from RichHoverButton constructor
Browse files Browse the repository at this point in the history
click_target_id isn't always needed (even today some calling sites were
providing 0) and in addition, calling sites
can set it when required.

This is a purely mechanical CL.

Bug: 1382018
Change-Id: Ib8f3a39490663bbd2962fc384ca8412969790940
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4023075
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1070713}
  • Loading branch information
mohamedamir authored and Chromium LUCI CQ committed Nov 12, 2022
1 parent 603f475 commit ef7a797
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 48 deletions.
2 changes: 0 additions & 2 deletions chrome/browser/ui/views/controls/rich_hover_button.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ RichHoverButton::RichHoverButton(
const ui::ImageModel& main_image_icon,
const std::u16string& title_text,
const std::u16string& secondary_text,
int click_target_id,
const std::u16string& tooltip_text,
const std::u16string& subtitle_text,
absl::optional<ui::ImageModel> action_image_icon,
Expand Down Expand Up @@ -148,7 +147,6 @@ RichHoverButton::RichHoverButton(
SetBorder(views::CreateEmptyBorder(layout_provider->GetInsetsMetric(
ChromeInsetsMetric::INSETS_PAGE_INFO_HOVER_BUTTON)));

SetID(click_target_id);
SetTooltipText(tooltip_text);
UpdateAccessibleName();

Expand Down
1 change: 0 additions & 1 deletion chrome/browser/ui/views/controls/rich_hover_button.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ class RichHoverButton : public HoverButton {
const ui::ImageModel& main_image_icon,
const std::u16string& title_text,
const std::u16string& secondary_text,
int click_target_id,
const std::u16string& tooltip_text,
const std::u16string& subtitle_text,
absl::optional<ui::ImageModel> action_image_icon = absl::nullopt,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ PageInfoAdPersonalizationContentView::PageInfoAdPersonalizationContentView(
PageInfoViewFactory::GetSiteSettingsIcon(),
l10n_util::GetStringUTF16(
IDS_PAGE_INFO_AD_PERSONALIZATION_SUBPAGE_MANAGE_BUTTON),
std::u16string(), 0,
std::u16string(),
/*tooltip_text=*/std::u16string(), std::u16string(),
PageInfoViewFactory::GetLaunchIcon()));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,11 @@ void PageInfoCookiesContentView::InitCookiesDialogButton() {
icon,
l10n_util::GetStringUTF16(IDS_PAGE_INFO_COOKIES_DIALOG_BUTTON_TITLE),
std::u16string(),
PageInfoViewFactory::VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_COOKIE_DIALOG,

tooltip, /*subtitle_text=*/u" ",
PageInfoViewFactory::GetLaunchIcon()));
cookies_dialog_button_->SetID(
PageInfoViewFactory::VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_COOKIE_DIALOG);
cookies_dialog_button_->SetProperty(views::kElementIdentifierKey,
kCookieDialogButton);
}
Expand Down Expand Up @@ -350,12 +352,13 @@ void PageInfoCookiesContentView::InitFpsButton(bool is_managed) {
base::Unretained(this)),
PageInfoViewFactory::GetFpsIcon(),
l10n_util::GetStringUTF16(IDS_PAGE_INFO_COOKIES), std::u16string(),
PageInfoViewFactory::VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_FPS_SETTINGS,
tooltip, /*secondary_text=*/u" ",
PageInfoViewFactory::GetLaunchIcon(),
is_managed ? absl::optional<ui::ImageModel>(
PageInfoViewFactory::GetEnforcedByPolicyIcon())
: absl::nullopt));
fps_button_->SetID(
PageInfoViewFactory::VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_FPS_SETTINGS);
}

void PageInfoCookiesContentView::FpsSettingsButtonClicked(ui::Event const&) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,16 @@ void PageInfoHistoryController::UpdateRow(base::Time last_visit) {
std::unique_ptr<views::View> PageInfoHistoryController::CreateHistoryButton(
std::u16string last_visit) {
// TODO(crbug.com/1275042): Use correct icons and strings (tooltip).
return std::make_unique<RichHoverButton>(
auto button = std::make_unique<RichHoverButton>(
base::BindRepeating(&PageInfoHistoryController::OpenHistoryPage,
weak_factory_.GetWeakPtr()),
PageInfoViewFactory::GetHistoryIcon(),
l10n_util::GetStringUTF16(IDS_PAGE_INFO_HISTORY), last_visit,
PageInfoViewFactory::VIEW_ID_PAGE_INFO_HISTORY_BUTTON,

/*tooltip_text=*/std::u16string(), std::u16string(),
PageInfoViewFactory::GetLaunchIcon());
button->SetID(PageInfoViewFactory::VIEW_ID_PAGE_INFO_HISTORY_BUTTON);
return button;
}

void PageInfoHistoryController::OpenHistoryPage() {
Expand Down
84 changes: 47 additions & 37 deletions chrome/browser/ui/views/page_info/page_info_main_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,10 @@ PageInfoMainView::PageInfoMainView(
PageInfoViewFactory::GetSiteSettingsIcon(),
/*title_text=*/l10n_util::GetStringUTF16(link_text_id),
std::u16string(),
PageInfoViewFactory::VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_SITE_SETTINGS,
/*tooltip_text=*/l10n_util::GetStringUTF16(tooltip_text_id),
std::u16string(), PageInfoViewFactory::GetLaunchIcon()));
site_settings_link_->SetID(
PageInfoViewFactory::VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_SITE_SETTINGS);
}

if (base::FeatureList::IsEnabled(page_info::kPageInfoHistoryDesktop)) {
Expand Down Expand Up @@ -149,29 +150,32 @@ void PageInfoMainView::EnsureCookieInfo() {

if (base::FeatureList::IsEnabled(page_info::kPageInfoCookiesSubpage)) {
// Create a simple cookie button, that opens a cookies subpage.
cookie_button_ = site_settings_view_->AddChildView(std::make_unique<
RichHoverButton>(
base::BindRepeating(&PageInfoNavigationHandler::OpenCookiesPage,
base::Unretained(navigation_handler_)),
icon, l10n_util::GetStringUTF16(IDS_PAGE_INFO_COOKIES_HEADER),
std::u16string(),
PageInfoViewFactory::VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_COOKIES_SUBPAGE,
tooltip, std::u16string(),
PageInfoViewFactory::GetOpenSubpageIcon()));
cookie_button_ =
site_settings_view_->AddChildView(std::make_unique<RichHoverButton>(
base::BindRepeating(&PageInfoNavigationHandler::OpenCookiesPage,
base::Unretained(navigation_handler_)),
icon, l10n_util::GetStringUTF16(IDS_PAGE_INFO_COOKIES_HEADER),
std::u16string(), tooltip, std::u16string(),
PageInfoViewFactory::GetOpenSubpageIcon()));
cookie_button_->SetID(
PageInfoViewFactory::
VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_COOKIES_SUBPAGE);

} else {
// Create the cookie button, leaving the secondary text blank since the
// cookie count is not yet known.
cookie_button_ = site_settings_view_->AddChildView(std::make_unique<
RichHoverButton>(
base::BindRepeating(
[](PageInfoMainView* view) {
view->HandleMoreInfoRequest(view->cookie_button_);
},
this),
icon, l10n_util::GetStringUTF16(IDS_PAGE_INFO_COOKIES),
/*secondary_text=*/u"",
PageInfoViewFactory::VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_COOKIE_DIALOG,
tooltip, std::u16string(), PageInfoViewFactory::GetLaunchIcon()));
cookie_button_ =
site_settings_view_->AddChildView(std::make_unique<RichHoverButton>(
base::BindRepeating(
[](PageInfoMainView* view) {
view->HandleMoreInfoRequest(view->cookie_button_);
},
this),
icon, l10n_util::GetStringUTF16(IDS_PAGE_INFO_COOKIES),
/*secondary_text=*/u"", tooltip, std::u16string(),
PageInfoViewFactory::GetLaunchIcon()));
cookie_button_->SetID(
PageInfoViewFactory::VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_COOKIE_DIALOG);
}
cookie_button_->SetProperty(views::kElementIdentifierKey,
kCookieButtonElementId);
Expand Down Expand Up @@ -365,11 +369,13 @@ void PageInfoMainView::SetIdentityInfo(const IdentityInfo& identity_info) {
base::Unretained(navigation_handler_)),
PageInfoViewFactory::GetConnectionSecureIcon(), std::u16string(),
std::u16string(),
PageInfoViewFactory::
VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_SECURITY_INFORMATION,

l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_SUBPAGE_BUTTON),
std::u16string(), PageInfoViewFactory::GetOpenSubpageIcon())
.release());
connection_button_->SetID(
PageInfoViewFactory::
VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_SECURITY_INFORMATION);
connection_button_->SetTitleText(security_description->summary);

// Show "About this site" section only if connection is secure, because
Expand Down Expand Up @@ -604,9 +610,10 @@ std::unique_ptr<views::View> PageInfoMainView::CreateAboutThisSiteSection(
PageInfoViewFactory::GetAboutThisPageIcon(),
l10n_util::GetStringUTF16(IDS_PAGE_INFO_ABOUT_THIS_PAGE_TITLE),
std::u16string(),
PageInfoViewFactory::VIEW_ID_PAGE_INFO_ABOUT_THIS_SITE_BUTTON,
l10n_util::GetStringUTF16(IDS_PAGE_INFO_ABOUT_THIS_PAGE_TOOLTIP),
description, PageInfoViewFactory::GetLaunchIcon()));
about_this_site_button->SetID(
PageInfoViewFactory::VIEW_ID_PAGE_INFO_ABOUT_THIS_SITE_BUTTON);
} else {
// The kPageInfoAboutThisSiteDescriptionPlaceholder feature must only be
// enabled together with kPageInfoAboutThisSiteMoreInfo
Expand All @@ -624,10 +631,12 @@ std::unique_ptr<views::View> PageInfoMainView::CreateAboutThisSiteSection(
PageInfoViewFactory::GetAboutThisSiteIcon(),
l10n_util::GetStringUTF16(IDS_PAGE_INFO_ABOUT_THIS_SITE_HEADER),
std::u16string(),
PageInfoViewFactory::VIEW_ID_PAGE_INFO_ABOUT_THIS_SITE_BUTTON,

l10n_util::GetStringUTF16(IDS_PAGE_INFO_ABOUT_THIS_SITE_TOOLTIP),
base::UTF8ToUTF16(info.description().description()),
PageInfoViewFactory::GetOpenSubpageIcon()));
about_this_site_button->SetID(
PageInfoViewFactory::VIEW_ID_PAGE_INFO_ABOUT_THIS_SITE_BUTTON);
}
about_this_site_button->SetSubtitleMultiline(false);
return about_this_site_section;
Expand All @@ -639,18 +648,19 @@ PageInfoMainView::CreateAdPersonalizationSection() {
ads_personalization_section
->SetLayoutManager(std::make_unique<views::FlexLayout>())
->SetOrientation(views::LayoutOrientation::kVertical);
ads_personalization_section->AddChildView(std::make_unique<RichHoverButton>(
base::BindRepeating(
[](PageInfoMainView* view) {
view->navigation_handler_->OpenAdPersonalizationPage();
},
this),
PageInfoViewFactory::GetAdPersonalizationIcon(),
l10n_util::GetStringUTF16(IDS_PAGE_INFO_AD_PERSONALIZATION_HEADER),
std::u16string(),
PageInfoViewFactory::VIEW_ID_PAGE_INFO_AD_PERSONALIZATION_BUTTON,
l10n_util::GetStringUTF16(IDS_PAGE_INFO_AD_PERSONALIZATION_TOOLTIP),
std::u16string(), PageInfoViewFactory::GetOpenSubpageIcon()));
ads_personalization_section
->AddChildView(std::make_unique<RichHoverButton>(
base::BindRepeating(
[](PageInfoMainView* view) {
view->navigation_handler_->OpenAdPersonalizationPage();
},
this),
PageInfoViewFactory::GetAdPersonalizationIcon(),
l10n_util::GetStringUTF16(IDS_PAGE_INFO_AD_PERSONALIZATION_HEADER),
std::u16string(),
l10n_util::GetStringUTF16(IDS_PAGE_INFO_AD_PERSONALIZATION_TOOLTIP),
std::u16string(), PageInfoViewFactory::GetOpenSubpageIcon()))
->SetID(PageInfoViewFactory::VIEW_ID_PAGE_INFO_AD_PERSONALIZATION_BUTTON);

return ads_personalization_section;
}
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ PageInfoPermissionContentView::PageInfoPermissionContentView(
PageInfoViewFactory::GetSiteSettingsIcon(),
l10n_util::GetStringUTF16(
IDS_PAGE_INFO_PERMISSIONS_SUBPAGE_MANAGE_BUTTON),
std::u16string(), 0,
std::u16string(),
l10n_util::GetStringUTF16(
IDS_PAGE_INFO_PERMISSIONS_SUBPAGE_MANAGE_BUTTON_TOOLTIP),
std::u16string(), PageInfoViewFactory::GetLaunchIcon()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,12 @@ void PageInfoSecurityContentView::SetIdentityInfo(
},
this),
icon, l10n_util::GetStringUTF16(title_id), std::u16string(),
PageInfoViewFactory::
VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_CERTIFICATE_VIEWER,

tooltip, subtitle_text, PageInfoViewFactory::GetLaunchIcon())
.release());
certificate_button_->SetID(
PageInfoViewFactory::
VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_CERTIFICATE_VIEWER);
}

if (identity_info.show_change_password_buttons) {
Expand Down

0 comments on commit ef7a797

Please sign in to comment.