0

birch: Update subtitle strings for birch suggestion chips

Adjust them to match the latest Triggers & Priorities spreadsheet.
Change "last active" to only show tabs from the last 7 days. Provide
a "last visited" timestamp for "last active" so the subtitle can be
changed based on the last visit time. Also UX wants "hour" and "hours"
abbreviated as "hr".

Bug: b:345284203
Change-Id: Ib5857e2d52f94cfa298ea0a7c893dea89ab55fd2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5601969
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1311534}
This commit is contained in:
James Cook
2024-06-06 20:52:46 +00:00
committed by Chromium LUCI CQ
parent 4d1757e25c
commit 7f70003049
18 changed files with 84 additions and 34 deletions

@@ -8240,18 +8240,39 @@ To shut down the device, press and hold the power button on the device again.
</message> </message>
<message name="IDS_ASH_BIRCH_RECENT_TAB_SUBTITLE_PREFIX" desc="Start of the subtitle for the suggestion chip with recent tabs from other devices"> <message name="IDS_ASH_BIRCH_RECENT_TAB_SUBTITLE_PREFIX" desc="Start of the subtitle for the suggestion chip with recent tabs from other devices">
{HOURS, plural, {HOURS, plural,
=0 {&lt; 1 hour ago} =0 {Within 1 hr}
=1 {1 hour ago} =1 {1 hr ago}
other {# hours ago}} other {# hr ago}}
</message> </message>
<message name="IDS_ASH_BIRCH_RECENT_TAB_SUBTITLE_SUFFIX" desc="End of the subtitle for the suggestion chip with recent tabs from other devices"> <message name="IDS_ASH_BIRCH_RECENT_TAB_SUBTITLE_SUFFIX" desc="End of the subtitle for the suggestion chip with recent tabs from other devices">
From <ph name="SESSION_NAME">$1<ex>Chromebook</ex></ph> From <ph name="SESSION_NAME">$1<ex>Chromebook</ex></ph>
</message> </message>
<message name="IDS_ASH_BIRCH_LAST_ACTIVE_SUBTITLE" desc="The subtitle for a suggestion chip for a web site that was the last browser tab open"> <message name="IDS_ASH_BIRCH_SELF_SHARE_SUBTITLE_YESTERDAY" desc="Subtitle for the suggestion chip with tabs shared from other devices shared yesterday">
Last tab opened Yesterday
</message>
<message name="IDS_ASH_BIRCH_SELF_SHARE_SUBTITLE_PREFIX" desc="Start of the subtitle for the suggestion chip with tabs shared from other devices">
{HOURS, plural,
=0 {Within 1 hr}
=1 {1 hr ago}
other {# hr ago}}
</message>
<message name="IDS_ASH_BIRCH_SELF_SHARE_SUBTITLE_SUFFIX" desc="End of the subtitle for the suggestion chip with tabs shared from other devices">
Sent from <ph name="SESSION_NAME">$1<ex>Chromebook</ex></ph>
</message>
<message name="IDS_ASH_BIRCH_LAST_ACTIVE_SUBTITLE_YESTERDAY" desc="Subtitle for the suggestion chip with the last active tab">
Yesterday
</message>
<message name="IDS_ASH_BIRCH_LAST_ACTIVE_SUBTITLE_PREFIX" desc="Start of the subtitle for the suggestion chip with the last active tab">
{HOURS, plural,
=0 {Within 1 hr}
=1 {1 hr ago}
other {# hr ago}}
</message>
<message name="IDS_ASH_BIRCH_LAST_ACTIVE_SUBTITLE_SUFFIX" desc="End of the subtitle for the suggestion chip with the last active tab">
Continue browsing
</message> </message>
<message name="IDS_ASH_BIRCH_MOST_VISITED_SUBTITLE" desc="The subtitle for a suggestion chip for a web site that is frequently visited"> <message name="IDS_ASH_BIRCH_MOST_VISITED_SUBTITLE" desc="The subtitle for a suggestion chip for a web site that is frequently visited">
Frequently visited Frequently opened
</message> </message>
<message name="IDS_ASH_BIRCH_RELEASE_NOTES_TITLE" desc="Title for the suggestion chip that links to release notes"> <message name="IDS_ASH_BIRCH_RELEASE_NOTES_TITLE" desc="Title for the suggestion chip that links to release notes">
See what's new See what's new

@@ -1 +0,0 @@
3e06c120aa65574c07f1de65a71494377e628497

@@ -0,0 +1 @@
e3ce762e0dacb4478376c964b0a68b97085bcb2d

@@ -0,0 +1 @@
c4b3bb5c9dbf5fca713cc478a669f62deb45c760

@@ -0,0 +1 @@
402c277ad17665ab658bf38323227c0bfe13db67

@@ -1 +1 @@
3a000eed92d8ccbbea608e75852882f99d6da88a d577737930f37b76a60a1049716147af0f4210a2

@@ -1 +1 @@
16cf92e23d10cee54e919d8d89bad5fd5efb2dd0 82c68d097474a751befd8f75444d526fb0e86ff9

@@ -0,0 +1 @@
4293b3412f3eef1c7427a11a50088748c23a4651

@@ -0,0 +1 @@
d54bb160383d1e4a71c4a27e256dda0bef072329

@@ -0,0 +1 @@
d285cbd2fc42e09542b24a5050d91a00b2336308

@@ -574,8 +574,9 @@ std::u16string BirchTabItem::GetSubtitle(const std::string& session_name,
BirchLastActiveItem::BirchLastActiveItem(const std::u16string& title, BirchLastActiveItem::BirchLastActiveItem(const std::u16string& title,
const GURL& url, const GURL& url,
base::Time last_visit,
ui::ImageModel icon) ui::ImageModel icon)
: BirchItem(title, GetSubtitle()), url_(url), icon_(icon) {} : BirchItem(title, GetSubtitle(last_visit)), url_(url), icon_(icon) {}
BirchLastActiveItem::BirchLastActiveItem(BirchLastActiveItem&&) = default; BirchLastActiveItem::BirchLastActiveItem(BirchLastActiveItem&&) = default;
@@ -620,8 +621,23 @@ void BirchLastActiveItem::LoadIcon(LoadIconCallback callback) const {
} }
// static // static
std::u16string BirchLastActiveItem::GetSubtitle() { std::u16string BirchLastActiveItem::GetSubtitle(base::Time last_visit) {
return l10n_util::GetStringUTF16(IDS_ASH_BIRCH_LAST_ACTIVE_SUBTITLE); std::u16string prefix;
if (last_visit < base::Time::Now().LocalMidnight()) {
// TODO(jamescook): Add support for more than 1 day ago.
prefix =
l10n_util::GetStringUTF16(IDS_ASH_BIRCH_LAST_ACTIVE_SUBTITLE_YESTERDAY);
} else {
// Builds a string like "12 hours ago".
int hours = (base::Time::Now() - last_visit).InHours();
prefix = l10n_util::GetPluralStringFUTF16(
IDS_ASH_BIRCH_LAST_ACTIVE_SUBTITLE_PREFIX, hours);
}
// Builds a string like "Continue browsing".
std::u16string suffix =
l10n_util::GetStringUTF16(IDS_ASH_BIRCH_LAST_ACTIVE_SUBTITLE_SUFFIX);
return prefix + u" · " + suffix;
} }
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
@@ -754,18 +770,18 @@ std::u16string BirchSelfShareItem::GetSubtitle(
// Builds the string "Yesterday". We only show tabs within the last 24 hours // Builds the string "Yesterday". We only show tabs within the last 24 hours
// so we don't need to worry about days before yesterday. // so we don't need to worry about days before yesterday.
prefix = prefix =
l10n_util::GetStringUTF16(IDS_ASH_BIRCH_RECENT_TAB_SUBTITLE_YESTERDAY); l10n_util::GetStringUTF16(IDS_ASH_BIRCH_SELF_SHARE_SUBTITLE_YESTERDAY);
} else { } else {
// Builds a string like "12 hours ago". We only show tabs within the last // Builds a string like "12 hours ago". We only show tabs within the last
// 24 hours so we don't need to worry about a day count. // 24 hours so we don't need to worry about a day count.
const int hours = (base::Time::Now() - shared_time).InHours(); const int hours = (base::Time::Now() - shared_time).InHours();
prefix = l10n_util::GetPluralStringFUTF16( prefix = l10n_util::GetPluralStringFUTF16(
IDS_ASH_BIRCH_RECENT_TAB_SUBTITLE_PREFIX, hours); IDS_ASH_BIRCH_SELF_SHARE_SUBTITLE_PREFIX, hours);
} }
// Builds a string like "From Chromebook". // Builds a string like "Sent from Chromebook".
std::u16string suffix = l10n_util::GetStringFUTF16( std::u16string suffix = l10n_util::GetStringFUTF16(
IDS_ASH_BIRCH_RECENT_TAB_SUBTITLE_SUFFIX, device_name); IDS_ASH_BIRCH_SELF_SHARE_SUBTITLE_SUFFIX, device_name);
return prefix + u" · " + suffix; return prefix + u" · " + suffix;
} }

@@ -276,6 +276,7 @@ class ASH_EXPORT BirchLastActiveItem : public BirchItem {
public: public:
BirchLastActiveItem(const std::u16string& title, BirchLastActiveItem(const std::u16string& title,
const GURL& url, const GURL& url,
base::Time last_visit,
ui::ImageModel icon); ui::ImageModel icon);
BirchLastActiveItem(BirchLastActiveItem&&); BirchLastActiveItem(BirchLastActiveItem&&);
BirchLastActiveItem(const BirchLastActiveItem&); BirchLastActiveItem(const BirchLastActiveItem&);
@@ -293,7 +294,7 @@ class ASH_EXPORT BirchLastActiveItem : public BirchItem {
const GURL& url() const { return url_; } const GURL& url() const { return url_; }
private: private:
static std::u16string GetSubtitle(); static std::u16string GetSubtitle(base::Time last_visit);
GURL url_; GURL url_;
ui::ImageModel icon_; ui::ImageModel icon_;

@@ -424,7 +424,7 @@ TEST_F(BirchItemTest, Tab_Subtitle_Recent) {
/*timestamp=*/base::Time::Now() - base::Minutes(5), /*timestamp=*/base::Time::Now() - base::Minutes(5),
/*favicon_url=*/GURL(), /*session_name=*/"Chromebook", /*favicon_url=*/GURL(), /*session_name=*/"Chromebook",
BirchTabItem::DeviceFormFactor::kDesktop); BirchTabItem::DeviceFormFactor::kDesktop);
EXPECT_EQ(item.subtitle(), u"< 1 hour ago · From Chromebook"); EXPECT_EQ(item.subtitle(), u"Within 1 hr · From Chromebook");
} }
TEST_F(BirchItemTest, Tab_Subtitle_OneHour) { TEST_F(BirchItemTest, Tab_Subtitle_OneHour) {
@@ -432,7 +432,7 @@ TEST_F(BirchItemTest, Tab_Subtitle_OneHour) {
/*timestamp=*/base::Time::Now() - base::Minutes(65), /*timestamp=*/base::Time::Now() - base::Minutes(65),
/*favicon_url=*/GURL(), /*session_name=*/"Chromebook", /*favicon_url=*/GURL(), /*session_name=*/"Chromebook",
BirchTabItem::DeviceFormFactor::kDesktop); BirchTabItem::DeviceFormFactor::kDesktop);
EXPECT_EQ(item.subtitle(), u"1 hour ago · From Chromebook"); EXPECT_EQ(item.subtitle(), u"1 hr ago · From Chromebook");
} }
TEST_F(BirchItemTest, Tab_Subtitle_TwoHours) { TEST_F(BirchItemTest, Tab_Subtitle_TwoHours) {
@@ -440,7 +440,7 @@ TEST_F(BirchItemTest, Tab_Subtitle_TwoHours) {
/*timestamp=*/base::Time::Now() - base::Minutes(125), /*timestamp=*/base::Time::Now() - base::Minutes(125),
/*favicon_url=*/GURL(), /*session_name=*/"Chromebook", /*favicon_url=*/GURL(), /*session_name=*/"Chromebook",
BirchTabItem::DeviceFormFactor::kDesktop); BirchTabItem::DeviceFormFactor::kDesktop);
EXPECT_EQ(item.subtitle(), u"2 hours ago · From Chromebook"); EXPECT_EQ(item.subtitle(), u"2 hr ago · From Chromebook");
} }
TEST_F(BirchItemTest, Tab_Subtitle_Yesterday) { TEST_F(BirchItemTest, Tab_Subtitle_Yesterday) {
@@ -484,7 +484,7 @@ TEST_F(BirchItemTest, Tab_PerformAction_Histograms) {
} }
TEST_F(BirchItemTest, LastActive_PerformAction) { TEST_F(BirchItemTest, LastActive_PerformAction) {
BirchLastActiveItem item(u"item", GURL("http://example.com/"), BirchLastActiveItem item(u"item", GURL("http://example.com/"), base::Time(),
ui::ImageModel()); ui::ImageModel());
item.PerformAction(); item.PerformAction();
EXPECT_EQ(new_window_delegate_->last_opened_url_, EXPECT_EQ(new_window_delegate_->last_opened_url_,

@@ -526,7 +526,7 @@ TEST_F(BirchModelTest, DisablingPrefsClearsModel) {
model->SetRecentTabItems(std::move(tab_item_list)); model->SetRecentTabItems(std::move(tab_item_list));
std::vector<BirchLastActiveItem> last_active_list; std::vector<BirchLastActiveItem> last_active_list;
last_active_list.emplace_back(u"active", GURL("https://yahoo.com/"), last_active_list.emplace_back(u"active", GURL("https://yahoo.com/"),
ui::ImageModel()); base::Time(), ui::ImageModel());
model->SetLastActiveItems(std::move(last_active_list)); model->SetLastActiveItems(std::move(last_active_list));
std::vector<BirchMostVisitedItem> most_visited_list; std::vector<BirchMostVisitedItem> most_visited_list;
most_visited_list.emplace_back(u"visited", GURL("https://google.com/"), most_visited_list.emplace_back(u"visited", GURL("https://google.com/"),
@@ -1023,7 +1023,7 @@ TEST_F(BirchModelTest, ResponseAfterFirstTimeout) {
model->SetRecentTabItems(std::move(tab_item_list)); model->SetRecentTabItems(std::move(tab_item_list));
std::vector<BirchLastActiveItem> last_active_list; std::vector<BirchLastActiveItem> last_active_list;
last_active_list.emplace_back(u"active", GURL("https://yahoo.com/"), last_active_list.emplace_back(u"active", GURL("https://yahoo.com/"),
ui::ImageModel()); base::Time(), ui::ImageModel());
model->SetLastActiveItems(std::move(last_active_list)); model->SetLastActiveItems(std::move(last_active_list));
std::vector<BirchMostVisitedItem> most_visited_list; std::vector<BirchMostVisitedItem> most_visited_list;
most_visited_list.emplace_back(u"visited", GURL("https://google.com/"), most_visited_list.emplace_back(u"visited", GURL("https://google.com/"),
@@ -1092,7 +1092,7 @@ TEST_F(BirchModelTest, GetAllItems) {
model->SetRecentTabItems(std::move(tab_item_list)); model->SetRecentTabItems(std::move(tab_item_list));
std::vector<BirchLastActiveItem> last_active_list; std::vector<BirchLastActiveItem> last_active_list;
last_active_list.emplace_back(u"active", GURL("https://yahoo.com/"), last_active_list.emplace_back(u"active", GURL("https://yahoo.com/"),
ui::ImageModel()); base::Time(), ui::ImageModel());
model->SetLastActiveItems(std::move(last_active_list)); model->SetLastActiveItems(std::move(last_active_list));
std::vector<BirchMostVisitedItem> most_visited_list; std::vector<BirchMostVisitedItem> most_visited_list;
most_visited_list.emplace_back(u"visited", GURL("https://google.com/"), most_visited_list.emplace_back(u"visited", GURL("https://google.com/"),
@@ -1134,7 +1134,7 @@ TEST_F(BirchModelTest, SetItemListRecordsHistogram) {
model->SetRecentTabItems(std::move(tab_item_list)); model->SetRecentTabItems(std::move(tab_item_list));
std::vector<BirchLastActiveItem> last_active_list; std::vector<BirchLastActiveItem> last_active_list;
last_active_list.emplace_back(u"active", GURL("https://yahoo.com/"), last_active_list.emplace_back(u"active", GURL("https://yahoo.com/"),
ui::ImageModel()); base::Time(), ui::ImageModel());
model->SetLastActiveItems(std::move(last_active_list)); model->SetLastActiveItems(std::move(last_active_list));
model->SetFileSuggestItems(MakeFileItemList(/*item_count=*/1)); model->SetFileSuggestItems(MakeFileItemList(/*item_count=*/1));
std::vector<BirchWeatherItem> weather_item_list; std::vector<BirchWeatherItem> weather_item_list;
@@ -1566,8 +1566,9 @@ TEST_F(BirchModelTest, DuplicateLastActiveAndRecentTabItem) {
model->SetRecentTabItems(std::move(tab_item_list)); model->SetRecentTabItems(std::move(tab_item_list));
std::vector<BirchLastActiveItem> last_active_item_list; std::vector<BirchLastActiveItem> last_active_item_list;
last_active_item_list.emplace_back( last_active_item_list.emplace_back(u"last active",
u"last active", GURL("https://www.example.com/"), ui::ImageModel()); GURL("https://www.example.com/"),
base::Time(), ui::ImageModel());
model->SetLastActiveItems(std::move(last_active_item_list)); model->SetLastActiveItems(std::move(last_active_item_list));
// The last active item has the higher priority and hence is shown. // The last active item has the higher priority and hence is shown.
@@ -1702,8 +1703,9 @@ TEST_F(BirchModelTest, LastActiveItemShownByTime) {
// Create a last active item. // Create a last active item.
std::vector<BirchLastActiveItem> last_active_item_list; std::vector<BirchLastActiveItem> last_active_item_list;
last_active_item_list.emplace_back( last_active_item_list.emplace_back(u"last active",
u"last active", GURL("https://www.example.com/"), ui::ImageModel()); GURL("https://www.example.com/"),
base::Time(), ui::ImageModel());
model->SetLastActiveItems(std::move(last_active_item_list)); model->SetLastActiveItems(std::move(last_active_item_list));
// The first time we query for items, it is shown. // The first time we query for items, it is shown.

@@ -414,7 +414,7 @@ class BirchBarTestBase : public AshTestBase {
std::vector<BirchLastActiveItem> item_list; std::vector<BirchLastActiveItem> item_list;
for (size_t i = 0; i < num; ++i) { for (size_t i = 0; i < num; ++i) {
item_list.emplace_back(u"last active", GURL("https://yahoo.com/"), item_list.emplace_back(u"last active", GURL("https://yahoo.com/"),
ui::ImageModel()); base::Time(), ui::ImageModel());
item_list.back().set_ranking(1.0f); item_list.back().set_ranking(1.0f);
} }
birch_client_->SetLastActiveItems(item_list); birch_client_->SetLastActiveItems(item_list);

@@ -30,9 +30,11 @@ BirchLastActiveProvider::~BirchLastActiveProvider() = default;
void BirchLastActiveProvider::RequestBirchDataFetch() { void BirchLastActiveProvider::RequestBirchDataFetch() {
// Get the last active URL. The query results are sorted most-recent first, so // Get the last active URL. The query results are sorted most-recent first, so
// we only need to get the first entry to find the last active URL. // we only need to get the first entry to find the last active URL. We only
// care about URLs in the last week.
history::QueryOptions options; history::QueryOptions options;
options.max_count = 1; options.max_count = 1;
options.SetRecentDayRange(7);
history_service_->QueryHistory( history_service_->QueryHistory(
u"", options, u"", options,
base::BindOnce(&BirchLastActiveProvider::OnGotHistory, base::BindOnce(&BirchLastActiveProvider::OnGotHistory,
@@ -51,6 +53,7 @@ void BirchLastActiveProvider::OnGotHistory(history::QueryResults results) {
if (last_active.url() == previous_url_) { if (last_active.url() == previous_url_) {
std::vector<BirchLastActiveItem> last_active_items; std::vector<BirchLastActiveItem> last_active_items;
last_active_items.emplace_back(last_active.title(), last_active.url(), last_active_items.emplace_back(last_active.title(), last_active.url(),
last_active.last_visit(),
ui::ImageModel::FromImage(previous_image_)); ui::ImageModel::FromImage(previous_image_));
Shell::Get()->birch_model()->SetLastActiveItems( Shell::Get()->birch_model()->SetLastActiveItems(
std::move(last_active_items)); std::move(last_active_items));
@@ -62,13 +65,14 @@ void BirchLastActiveProvider::OnGotHistory(history::QueryResults results) {
last_active.url(), last_active.url(),
base::BindOnce(&BirchLastActiveProvider::OnGotFaviconImage, base::BindOnce(&BirchLastActiveProvider::OnGotFaviconImage,
weak_factory_.GetWeakPtr(), last_active.title(), weak_factory_.GetWeakPtr(), last_active.title(),
last_active.url()), last_active.url(), last_active.last_visit()),
&cancelable_task_tracker_); &cancelable_task_tracker_);
} }
void BirchLastActiveProvider::OnGotFaviconImage( void BirchLastActiveProvider::OnGotFaviconImage(
const std::u16string& title, const std::u16string& title,
const GURL& url, const GURL& url,
base::Time last_visit,
const favicon_base::FaviconImageResult& image_result) { const favicon_base::FaviconImageResult& image_result) {
// Don't show the result if there's no icon available (should be rare). // Don't show the result if there's no icon available (should be rare).
if (image_result.image.IsEmpty()) { if (image_result.image.IsEmpty()) {
@@ -77,7 +81,7 @@ void BirchLastActiveProvider::OnGotFaviconImage(
} }
// Populate the BirchModel with this URL. // Populate the BirchModel with this URL.
std::vector<BirchLastActiveItem> last_active_items; std::vector<BirchLastActiveItem> last_active_items;
last_active_items.emplace_back(title, url, last_active_items.emplace_back(title, url, last_visit,
ui::ImageModel::FromImage(image_result.image)); ui::ImageModel::FromImage(image_result.image));
Shell::Get()->birch_model()->SetLastActiveItems(std::move(last_active_items)); Shell::Get()->birch_model()->SetLastActiveItems(std::move(last_active_items));

@@ -46,6 +46,7 @@ class BirchLastActiveProvider : public BirchDataProvider {
// Callback from favicon service with the icon image. // Callback from favicon service with the icon image.
void OnGotFaviconImage(const std::u16string& title, void OnGotFaviconImage(const std::u16string& title,
const GURL& url, const GURL& url,
base::Time last_visit,
const favicon_base::FaviconImageResult& image_result); const favicon_base::FaviconImageResult& image_result);
void set_history_service_for_test(history::HistoryService* service) { void set_history_service_for_test(history::HistoryService* service) {

@@ -113,7 +113,7 @@ TEST_F(BirchLastActiveProviderTest, RequestBirchDataFetch) {
// Once the favicon is fetched the birch model is populated. // Once the favicon is fetched the birch model is populated.
provider.OnGotFaviconImage(u"title", GURL("http://example.com/"), provider.OnGotFaviconImage(u"title", GURL("http://example.com/"),
image_result); base::Time(), image_result);
auto* birch_model = Shell::Get()->birch_model(); auto* birch_model = Shell::Get()->birch_model();
EXPECT_EQ(birch_model->GetLastActiveItemsForTest().size(), 1u); EXPECT_EQ(birch_model->GetLastActiveItemsForTest().size(), 1u);