Format URLs in results in a terser way in Picker.
Picker shows a list of browsing history results. These results have the URL on a secondary label. The UX requirement is to shorten the URL by stripping out unnecessary info. Follow the same pattern in the Omnibox code by using url_formatter::FormatUrl. Fixed: b:350373388, b:345526919 Change-Id: I70c1f6e0a53ddeba854df6a9ae9d149a0ca718fd Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5686274 Commit-Queue: Darren Shen <shend@chromium.org> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> Reviewed-by: Chris Thompson <cthomp@chromium.org> Reviewed-by: Michael Cui <mlcui@google.com> Cr-Commit-Position: refs/heads/main@{#1326485}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
304403c9cf
commit
6f5b97df6c
@ -3361,6 +3361,7 @@ component("ash") {
|
||||
"//components/soda:constants",
|
||||
"//components/strings",
|
||||
"//components/sync",
|
||||
"//components/url_formatter",
|
||||
"//components/url_matcher",
|
||||
"//components/user_education/common",
|
||||
"//components/user_education/common:events",
|
||||
|
1
ash/DEPS
1
ash/DEPS
@ -28,6 +28,7 @@ include_rules = [
|
||||
"+components/sync_preferences/testing_pref_service_syncable.h",
|
||||
"+components/ui_devtools",
|
||||
"+components/ukm",
|
||||
"+components/url_formatter",
|
||||
"+components/url_matcher",
|
||||
"+components/user_education/common",
|
||||
"+components/user_education/views",
|
||||
|
@ -18,6 +18,7 @@
|
||||
#include "ash/strings/grit/ash_strings.h"
|
||||
#include "ash/style/style_util.h"
|
||||
#include "ash/style/typography.h"
|
||||
#include "base/strings/string_util.h"
|
||||
#include "third_party/skia/include/core/SkPath.h"
|
||||
#include "third_party/skia/include/core/SkScalar.h"
|
||||
#include "ui/base/l10n/l10n_util.h"
|
||||
@ -296,6 +297,13 @@ ui::ImageModel PickerListItemView::GetPrimaryImageForTesting() const {
|
||||
return ui::ImageModel();
|
||||
}
|
||||
|
||||
std::u16string_view PickerListItemView::GetSecondaryTextForTesting() const {
|
||||
if (secondary_label_ == nullptr) {
|
||||
return base::EmptyString16();
|
||||
}
|
||||
return secondary_label_->GetText();
|
||||
}
|
||||
|
||||
void PickerListItemView::UpdateIconWithPreview() {
|
||||
views::AsViewClass<LeadingIconImageView>(leading_icon_view_)
|
||||
->SetCircularMaskEnabled(true);
|
||||
|
@ -77,6 +77,7 @@ class ASH_EXPORT PickerListItemView : public PickerItemView {
|
||||
}
|
||||
std::u16string GetPrimaryTextForTesting() const;
|
||||
ui::ImageModel GetPrimaryImageForTesting() const;
|
||||
std::u16string_view GetSecondaryTextForTesting() const;
|
||||
|
||||
private:
|
||||
void UpdateIconWithPreview();
|
||||
|
@ -30,6 +30,7 @@
|
||||
#include "chromeos/components/editor_menu/public/cpp/icon.h"
|
||||
#include "chromeos/ui/base/file_icon_util.h"
|
||||
#include "chromeos/ui/vector_icons/vector_icons.h"
|
||||
#include "components/url_formatter/url_formatter.h"
|
||||
#include "components/vector_icons/vector_icons.h"
|
||||
#include "ui/base/l10n/l10n_util.h"
|
||||
#include "ui/base/metadata/metadata_impl_macros.h"
|
||||
@ -127,6 +128,15 @@ const gfx::VectorIcon& GetIconForCaseTransformType(
|
||||
}
|
||||
}
|
||||
|
||||
std::u16string FormatBrowsingHistoryUrl(const GURL& url) {
|
||||
return url_formatter::FormatUrl(
|
||||
url,
|
||||
url_formatter::kFormatUrlOmitDefaults |
|
||||
url_formatter::kFormatUrlOmitHTTPS |
|
||||
url_formatter::kFormatUrlOmitTrivialSubdomains,
|
||||
base::UnescapeRule::SPACES, nullptr, nullptr, nullptr);
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
PickerSectionView::PickerSectionView(
|
||||
@ -211,7 +221,7 @@ std::unique_ptr<PickerItemView> PickerSectionView::CreateItemFromResult(
|
||||
auto item_view = std::make_unique<PickerListItemView>(
|
||||
std::move(select_result_callback));
|
||||
item_view->SetPrimaryText(data.title);
|
||||
item_view->SetSecondaryText(base::UTF8ToUTF16(data.url.spec()));
|
||||
item_view->SetSecondaryText(FormatBrowsingHistoryUrl(data.url));
|
||||
item_view->SetLeadingIcon(data.icon);
|
||||
return item_view;
|
||||
},
|
||||
|
@ -160,5 +160,41 @@ TEST_F(PickerSectionViewTest, ClearsItems) {
|
||||
EXPECT_THAT(section_view.item_views_for_testing(), IsEmpty());
|
||||
}
|
||||
|
||||
class PickerSectionViewUrlFormattingTest
|
||||
: public PickerSectionViewTest,
|
||||
public testing::WithParamInterface<std::pair<GURL, std::u16string>> {};
|
||||
|
||||
INSTANTIATE_TEST_SUITE_P(
|
||||
,
|
||||
PickerSectionViewUrlFormattingTest,
|
||||
testing::Values(
|
||||
std::make_pair(GURL("http://foo.com/bar"), u"foo.com/bar"),
|
||||
std::make_pair(GURL("https://foo.com/bar"), u"foo.com/bar"),
|
||||
std::make_pair(GURL("https://www.foo.com/bar"), u"foo.com/bar"),
|
||||
std::make_pair(GURL("chrome://version"), u"chrome://version"),
|
||||
std::make_pair(GURL("chrome-extension://aaa"),
|
||||
u"chrome-extension://aaa"),
|
||||
std::make_pair(GURL("file://a/b/c"), u"file://a/b/c")));
|
||||
|
||||
TEST_P(PickerSectionViewUrlFormattingTest, AddingHistoryResultFormatsUrl) {
|
||||
MockPickerAssetFetcher asset_fetcher;
|
||||
PickerPreviewBubbleController preview_controller;
|
||||
PickerSubmenuController submenu_controller;
|
||||
PickerSectionView section_view(kDefaultSectionWidth, &asset_fetcher,
|
||||
&submenu_controller);
|
||||
|
||||
section_view.AddResult(PickerSearchResult::BrowsingHistory(
|
||||
GetParam().first, u"title", /*icon=*/{}),
|
||||
&preview_controller, base::DoNothing());
|
||||
|
||||
base::span<const raw_ptr<PickerItemView>> items =
|
||||
section_view.item_views_for_testing();
|
||||
ASSERT_THAT(items, SizeIs(1));
|
||||
EXPECT_TRUE(views::IsViewClass<PickerListItemView>(items[0]));
|
||||
EXPECT_EQ(views::AsViewClass<PickerListItemView>(items[0])
|
||||
->GetSecondaryTextForTesting(),
|
||||
GetParam().second);
|
||||
}
|
||||
|
||||
} // namespace
|
||||
} // namespace ash
|
||||
|
Reference in New Issue
Block a user