Move preview description logic to a separate file.
1) This logic needs to be thoroughly tested, but testing it
through PickerListItemView or PickerPreviewBubbleController is
quite verbose.
2) This logic needs to be callable from PickerListItemView for
the accessibility description.
Bug: b:353591340
Change-Id: I21b64edbce84b5d785814702e9ccef20275ea170
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5753069
Commit-Queue: Darren Shen <shend@chromium.org>
Reviewed-by: Michael Cui <mlcui@google.com>
Cr-Commit-Position: refs/heads/main@{#1336416}
diff --git a/ash/BUILD.gn b/ash/BUILD.gn
index 32efbe0..220f4f6c 100644
--- a/ash/BUILD.gn
+++ b/ash/BUILD.gn
@@ -1126,6 +1126,8 @@
"picker/views/picker_preview_bubble.h",
"picker/views/picker_preview_bubble_controller.cc",
"picker/views/picker_preview_bubble_controller.h",
+ "picker/views/picker_preview_metadata.cc",
+ "picker/views/picker_preview_metadata.h",
"picker/views/picker_pseudo_focus.cc",
"picker/views/picker_pseudo_focus.h",
"picker/views/picker_pseudo_focus_handler.h",
@@ -3868,6 +3870,7 @@
"picker/views/picker_main_container_view_unittest.cc",
"picker/views/picker_positioning_unittest.cc",
"picker/views/picker_preview_bubble_controller_unittest.cc",
+ "picker/views/picker_preview_metadata_unittest.cc",
"picker/views/picker_pseudo_focus_unittest.cc",
"picker/views/picker_search_field_view_unittest.cc",
"picker/views/picker_search_results_view_unittest.cc",
diff --git a/ash/picker/views/picker_list_item_view.cc b/ash/picker/views/picker_list_item_view.cc
index 750b167..d15e3cf3 100644
--- a/ash/picker/views/picker_list_item_view.cc
+++ b/ash/picker/views/picker_list_item_view.cc
@@ -15,6 +15,7 @@
#include "ash/picker/views/picker_item_view.h"
#include "ash/picker/views/picker_preview_bubble.h"
#include "ash/picker/views/picker_preview_bubble_controller.h"
+#include "ash/picker/views/picker_preview_metadata.h"
#include "ash/public/cpp/holding_space/holding_space_image.h"
#include "ash/strings/grit/ash_strings.h"
#include "ash/style/style_util.h"
@@ -367,17 +368,19 @@
file_info_ = std::move(info);
if (preview_bubble_controller_ != nullptr) {
- // Update the bubble metadata if it's open.
- preview_bubble_controller_->UpdateBubbleMetadata(file_info_);
+ // Update the bubble main text if it's open.
+ preview_bubble_controller_->SetBubbleMainText(
+ PickerGetFilePreviewDescription(file_info_));
}
}
void PickerListItemView::ShowPreview() {
if (preview_bubble_controller_ != nullptr) {
- // Update the bubble metadata before showing it.
+ // Update the bubble main text before it becomes visible.
preview_bubble_controller_->ShowBubbleAfterDelay(async_preview_image_.get(),
file_path_, this);
- preview_bubble_controller_->UpdateBubbleMetadata(file_info_);
+ preview_bubble_controller_->SetBubbleMainText(
+ PickerGetFilePreviewDescription(file_info_));
}
}
diff --git a/ash/picker/views/picker_preview_bubble.cc b/ash/picker/views/picker_preview_bubble.cc
index 8fd262cc..58014c3 100644
--- a/ash/picker/views/picker_preview_bubble.cc
+++ b/ash/picker/views/picker_preview_bubble.cc
@@ -140,6 +140,12 @@
box_layout_view_->SetVisible(true);
}
+void PickerPreviewBubbleView::ClearText() {
+ eyebrow_label_->SetText(u"");
+ main_label_->SetText(u"");
+ box_layout_view_->SetVisible(false);
+}
+
void PickerPreviewBubbleView::OnThemeChanged() {
BubbleDialogDelegateView::OnThemeChanged();
set_color(GetColorProvider()->GetColor(kBackgroundColor));
diff --git a/ash/picker/views/picker_preview_bubble.h b/ash/picker/views/picker_preview_bubble.h
index 7e35d37..f1960c2 100644
--- a/ash/picker/views/picker_preview_bubble.h
+++ b/ash/picker/views/picker_preview_bubble.h
@@ -44,6 +44,7 @@
// Sets the text of the labels and makes them visible.
void SetText(const std::u16string& eyebrow_text,
const std::u16string& main_text);
+ void ClearText();
// BubbleDialogDelegateView overrides
void OnThemeChanged() override;
diff --git a/ash/picker/views/picker_preview_bubble_controller.cc b/ash/picker/views/picker_preview_bubble_controller.cc
index 715c271..1139cafa 100644
--- a/ash/picker/views/picker_preview_bubble_controller.cc
+++ b/ash/picker/views/picker_preview_bubble_controller.cc
@@ -11,15 +11,12 @@
#include "ash/picker/views/picker_preview_bubble.h"
#include "ash/public/cpp/holding_space/holding_space_image.h"
-#include "ash/strings/grit/ash_strings.h"
#include "base/check.h"
#include "base/files/file.h"
#include "base/functional/bind.h"
#include "base/functional/callback.h"
-#include "base/i18n/time_formatting.h"
#include "base/observer_list.h"
#include "base/time/time.h"
-#include "ui/base/l10n/l10n_util.h"
#include "ui/views/view.h"
#include "ui/views/widget/widget.h"
@@ -32,43 +29,6 @@
// Duration to wait before showing the preview bubble when it is requested.
constexpr base::TimeDelta kShowBubbleDelay = base::Milliseconds(600);
-// Taken from //chrome/browser/ash/app_list/search/files/justifications.cc.
-// Time limits for how last accessed or modified time maps to each justification
-// string.
-constexpr base::TimeDelta kJustNow = base::Minutes(15);
-
-std::u16string GetTimeString(base::Time timestamp) {
- const base::Time now = base::Time::Now();
- const base::Time midnight = now.LocalMidnight();
- if ((now - timestamp).magnitude() <= kJustNow) {
- return l10n_util::GetStringUTF16(
- IDS_FILE_SUGGESTION_JUSTIFICATION_TIME_NOW);
- }
-
- if (timestamp >= midnight && timestamp < midnight + base::Days(1)) {
- return base::TimeFormatTimeOfDay(timestamp);
- }
-
- return base::LocalizedTimeFormatWithPattern(timestamp, "MMMd");
-}
-
-std::u16string GetJustificationString(base::Time viewed, base::Time modified) {
- // Prefer "modified" over "viewed" if they are the same.
- if (modified >= viewed) {
- return l10n_util::GetStringFUTF16(
- IDS_FILE_SUGGESTION_JUSTIFICATION,
- l10n_util::GetStringUTF16(
- IDS_FILE_SUGGESTION_JUSTIFICATION_GENERIC_MODIFIED_ACTION),
- GetTimeString(modified));
- } else {
- return l10n_util::GetStringFUTF16(
- IDS_FILE_SUGGESTION_JUSTIFICATION,
- l10n_util::GetStringUTF16(
- IDS_FILE_SUGGESTION_JUSTIFICATION_YOU_VIEWED_ACTION),
- GetTimeString(viewed));
- }
-}
-
} // namespace
PickerPreviewBubbleController::PickerPreviewBubbleController() = default;
@@ -113,21 +73,17 @@
observers_.RemoveObserver(observer);
}
-void PickerPreviewBubbleController::UpdateBubbleMetadata(
- std::optional<base::File::Info> info) {
+void PickerPreviewBubbleController::SetBubbleMainText(
+ const std::u16string& text) {
if (bubble_view_ == nullptr) {
return;
}
- if (!info.has_value()) {
- return;
- }
- if (info->last_modified.is_null() && info->last_accessed.is_null()) {
- return;
- }
- bubble_view_->SetText(
- std::u16string(kEyebrowText),
- GetJustificationString(info->last_accessed, info->last_modified));
+ if (text.empty()) {
+ bubble_view_->ClearText();
+ } else {
+ bubble_view_->SetText(std::u16string(kEyebrowText), text);
+ }
}
void PickerPreviewBubbleController::OnWidgetDestroying(views::Widget* widget) {
diff --git a/ash/picker/views/picker_preview_bubble_controller.h b/ash/picker/views/picker_preview_bubble_controller.h
index fd6b002..c90fa01 100644
--- a/ash/picker/views/picker_preview_bubble_controller.h
+++ b/ash/picker/views/picker_preview_bubble_controller.h
@@ -64,10 +64,10 @@
void AddObserver(Observer* observer);
void RemoveObserver(Observer* observer);
- // Updates the bubble view metadata for the currently open bubble.
+ // Updates the bubble view labels for the currently open bubble.
// If the bubble is not shown, this does nothing.
- // If `info` is std::nullopt, then this clears the bubble view metadata.
- void UpdateBubbleMetadata(std::optional<base::File::Info> info);
+ // If `text` is empty, then the bubble view labels are hidden.
+ void SetBubbleMainText(const std::u16string& text);
// views::WidgetObserver:
void OnWidgetDestroying(views::Widget* widget) override;
diff --git a/ash/picker/views/picker_preview_bubble_controller_unittest.cc b/ash/picker/views/picker_preview_bubble_controller_unittest.cc
index 1a46ee2..e96bb59 100644
--- a/ash/picker/views/picker_preview_bubble_controller_unittest.cc
+++ b/ash/picker/views/picker_preview_bubble_controller_unittest.cc
@@ -242,8 +242,7 @@
SK_ColorBLUE);
}
-TEST_F(PickerPreviewBubbleControllerTest,
- ShowBubbleHidesLabelsBeforeFileInfoResolves) {
+TEST_F(PickerPreviewBubbleControllerTest, ShowBubbleHidesLabelsByDefault) {
std::unique_ptr<views::Widget> anchor_widget =
CreateAnchorWidget(GetContext());
PickerPreviewBubbleController controller;
@@ -259,7 +258,7 @@
}
TEST_F(PickerPreviewBubbleControllerTest,
- ShowBubbleHidesLabelsAfterFileInfoResolvesWithNullopt) {
+ SetBubbleMainTextHidesLabelsWithEmptyText) {
std::unique_ptr<views::Widget> anchor_widget =
CreateAnchorWidget(GetContext());
PickerPreviewBubbleController controller;
@@ -268,15 +267,14 @@
controller.ShowBubbleImmediatelyForTesting(
&async_preview_image,
anchor_widget->GetContentsView());
- controller.UpdateBubbleMetadata(std::nullopt);
+ controller.SetBubbleMainText(u"");
PickerPreviewBubbleView* bubble_view = controller.bubble_view_for_testing();
ViewDrawnWaiter().Wait(bubble_view);
EXPECT_FALSE(bubble_view->GetLabelsVisibleForTesting());
}
-TEST_F(PickerPreviewBubbleControllerTest,
- ShowBubbleHidesLabelsAfterFileInfoResolvesWithNullFileInfo) {
+TEST_F(PickerPreviewBubbleControllerTest, SetBubbleMainTextUpdatesBubbleText) {
std::unique_ptr<views::Widget> anchor_widget =
CreateAnchorWidget(GetContext());
PickerPreviewBubbleController controller;
@@ -285,123 +283,13 @@
controller.ShowBubbleImmediatelyForTesting(
&async_preview_image,
anchor_widget->GetContentsView());
- controller.UpdateBubbleMetadata(base::File::Info());
- PickerPreviewBubbleView* bubble_view = controller.bubble_view_for_testing();
- ViewDrawnWaiter().Wait(bubble_view);
-
- EXPECT_FALSE(bubble_view->GetLabelsVisibleForTesting());
-}
-
-TEST_F(PickerPreviewBubbleControllerTest, ShowBubbleShowsModifiedTitle) {
- std::unique_ptr<views::Widget> anchor_widget =
- CreateAnchorWidget(GetContext());
- PickerPreviewBubbleController controller;
- ash::HoldingSpaceImage async_preview_image = CreateUnresolvedAsyncImage();
-
- base::File::Info only_modified;
- EXPECT_TRUE(base::Time::FromString("23 Dec 2021 09:01:00",
- &only_modified.last_modified));
- controller.ShowBubbleImmediatelyForTesting(
- &async_preview_image,
- anchor_widget->GetContentsView());
- controller.UpdateBubbleMetadata(only_modified);
+ controller.SetBubbleMainText(u"Edited Dec 23");
PickerPreviewBubbleView* bubble_view = controller.bubble_view_for_testing();
ViewDrawnWaiter().Wait(bubble_view);
EXPECT_TRUE(bubble_view->GetLabelsVisibleForTesting());
EXPECT_EQ(bubble_view->GetEyebrowTextForTesting(), u"Last action");
- EXPECT_EQ(bubble_view->GetMainTextForTesting(), u"Edited · Dec 23");
-}
-
-TEST_F(PickerPreviewBubbleControllerTest, ShowBubbleShowsAccessedTitle) {
- std::unique_ptr<views::Widget> anchor_widget =
- CreateAnchorWidget(GetContext());
- PickerPreviewBubbleController controller;
- ash::HoldingSpaceImage async_preview_image = CreateUnresolvedAsyncImage();
-
- base::File::Info only_accessed;
- EXPECT_TRUE(base::Time::FromString("23 Dec 2021 09:01:00",
- &only_accessed.last_accessed));
- controller.ShowBubbleImmediatelyForTesting(
- &async_preview_image,
- anchor_widget->GetContentsView());
- controller.UpdateBubbleMetadata(only_accessed);
- PickerPreviewBubbleView* bubble_view = controller.bubble_view_for_testing();
- ViewDrawnWaiter().Wait(bubble_view);
-
- EXPECT_TRUE(bubble_view->GetLabelsVisibleForTesting());
- EXPECT_EQ(bubble_view->GetEyebrowTextForTesting(), u"Last action");
- EXPECT_EQ(bubble_view->GetMainTextForTesting(), u"You opened · Dec 23");
-}
-
-TEST_F(PickerPreviewBubbleControllerTest, ShowBubbleShowsModifiedTitleIfNewer) {
- std::unique_ptr<views::Widget> anchor_widget =
- CreateAnchorWidget(GetContext());
- PickerPreviewBubbleController controller;
- ash::HoldingSpaceImage async_preview_image = CreateUnresolvedAsyncImage();
-
- base::File::Info modified_newer;
- EXPECT_TRUE(base::Time::FromString("23 Dec 2021 09:01:00",
- &modified_newer.last_modified));
- EXPECT_TRUE(base::Time::FromString("23 Dec 2021 09:00:00",
- &modified_newer.last_accessed));
- controller.ShowBubbleImmediatelyForTesting(
- &async_preview_image,
- anchor_widget->GetContentsView());
- controller.UpdateBubbleMetadata(modified_newer);
- PickerPreviewBubbleView* bubble_view = controller.bubble_view_for_testing();
- ViewDrawnWaiter().Wait(bubble_view);
-
- EXPECT_TRUE(bubble_view->GetLabelsVisibleForTesting());
- EXPECT_EQ(bubble_view->GetEyebrowTextForTesting(), u"Last action");
- EXPECT_EQ(bubble_view->GetMainTextForTesting(), u"Edited · Dec 23");
-}
-
-TEST_F(PickerPreviewBubbleControllerTest, ShowBubbleShowsAccessedTitleIfNewer) {
- std::unique_ptr<views::Widget> anchor_widget =
- CreateAnchorWidget(GetContext());
- PickerPreviewBubbleController controller;
- ash::HoldingSpaceImage async_preview_image = CreateUnresolvedAsyncImage();
-
- base::File::Info accessed_newer;
- EXPECT_TRUE(base::Time::FromString("23 Dec 2021 09:00:00",
- &accessed_newer.last_modified));
- EXPECT_TRUE(base::Time::FromString("23 Dec 2021 09:01:00",
- &accessed_newer.last_accessed));
- controller.ShowBubbleImmediatelyForTesting(
- &async_preview_image,
- anchor_widget->GetContentsView());
- controller.UpdateBubbleMetadata(accessed_newer);
- PickerPreviewBubbleView* bubble_view = controller.bubble_view_for_testing();
- ViewDrawnWaiter().Wait(bubble_view);
-
- EXPECT_TRUE(bubble_view->GetLabelsVisibleForTesting());
- EXPECT_EQ(bubble_view->GetEyebrowTextForTesting(), u"Last action");
- EXPECT_EQ(bubble_view->GetMainTextForTesting(), u"You opened · Dec 23");
-}
-
-TEST_F(PickerPreviewBubbleControllerTest,
- ShowBubbleShowsModifiedTitleIfSameAsAccessed) {
- std::unique_ptr<views::Widget> anchor_widget =
- CreateAnchorWidget(GetContext());
- PickerPreviewBubbleController controller;
- ash::HoldingSpaceImage async_preview_image = CreateUnresolvedAsyncImage();
-
- base::File::Info modified_newer;
- EXPECT_TRUE(base::Time::FromString("23 Dec 2021 09:01:00",
- &modified_newer.last_modified));
- EXPECT_TRUE(base::Time::FromString("23 Dec 2021 09:01:00",
- &modified_newer.last_accessed));
- controller.ShowBubbleImmediatelyForTesting(
- &async_preview_image,
- anchor_widget->GetContentsView());
- controller.UpdateBubbleMetadata(modified_newer);
- PickerPreviewBubbleView* bubble_view = controller.bubble_view_for_testing();
- ViewDrawnWaiter().Wait(bubble_view);
-
- EXPECT_TRUE(bubble_view->GetLabelsVisibleForTesting());
- EXPECT_EQ(bubble_view->GetEyebrowTextForTesting(), u"Last action");
- EXPECT_EQ(bubble_view->GetMainTextForTesting(), u"Edited · Dec 23");
+ EXPECT_EQ(bubble_view->GetMainTextForTesting(), u"Edited Dec 23");
}
} // namespace
diff --git a/ash/picker/views/picker_preview_metadata.cc b/ash/picker/views/picker_preview_metadata.cc
new file mode 100644
index 0000000..7e2fbb39f
--- /dev/null
+++ b/ash/picker/views/picker_preview_metadata.cc
@@ -0,0 +1,66 @@
+// Copyright 2024 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ash/picker/views/picker_preview_metadata.h"
+
+#include "ash/strings/grit/ash_strings.h"
+#include "base/i18n/time_formatting.h"
+#include "ui/base/l10n/l10n_util.h"
+
+namespace ash {
+namespace {
+
+// Taken from //chrome/browser/ash/app_list/search/files/justifications.cc.
+// Time limits for how last accessed or modified time maps to each justification
+// string.
+constexpr base::TimeDelta kJustNow = base::Minutes(15);
+
+std::u16string GetTimeString(base::Time timestamp) {
+ const base::Time now = base::Time::Now();
+ const base::Time midnight = now.LocalMidnight();
+ if ((now - timestamp).magnitude() <= kJustNow) {
+ return l10n_util::GetStringUTF16(
+ IDS_FILE_SUGGESTION_JUSTIFICATION_TIME_NOW);
+ }
+
+ if (timestamp >= midnight && timestamp < midnight + base::Days(1)) {
+ return base::TimeFormatTimeOfDay(timestamp);
+ }
+
+ return base::LocalizedTimeFormatWithPattern(timestamp, "MMMd");
+}
+
+std::u16string GetJustificationString(base::Time viewed, base::Time modified) {
+ // Prefer "modified" over "viewed" if they are the same.
+ if (modified >= viewed) {
+ return l10n_util::GetStringFUTF16(
+ IDS_FILE_SUGGESTION_JUSTIFICATION,
+ l10n_util::GetStringUTF16(
+ IDS_FILE_SUGGESTION_JUSTIFICATION_GENERIC_MODIFIED_ACTION),
+ GetTimeString(modified));
+ } else {
+ return l10n_util::GetStringFUTF16(
+ IDS_FILE_SUGGESTION_JUSTIFICATION,
+ l10n_util::GetStringUTF16(
+ IDS_FILE_SUGGESTION_JUSTIFICATION_YOU_VIEWED_ACTION),
+ GetTimeString(viewed));
+ }
+}
+
+} // namespace
+
+std::u16string PickerGetFilePreviewDescription(
+ std::optional<base::File::Info> info) {
+ if (!info.has_value()) {
+ return u"";
+ }
+
+ if (info->last_modified.is_null() && info->last_accessed.is_null()) {
+ return u"";
+ }
+
+ return GetJustificationString(info->last_accessed, info->last_modified);
+}
+
+} // namespace ash
diff --git a/ash/picker/views/picker_preview_metadata.h b/ash/picker/views/picker_preview_metadata.h
new file mode 100644
index 0000000..a5194122
--- /dev/null
+++ b/ash/picker/views/picker_preview_metadata.h
@@ -0,0 +1,21 @@
+// Copyright 2024 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef ASH_PICKER_VIEWS_PICKER_PREVIEW_METADATA_H_
+#define ASH_PICKER_VIEWS_PICKER_PREVIEW_METADATA_H_
+
+#include <optional>
+#include <string>
+
+#include "ash/ash_export.h"
+#include "base/files/file.h"
+
+namespace ash {
+
+ASH_EXPORT std::u16string PickerGetFilePreviewDescription(
+ std::optional<base::File::Info> info);
+
+}
+
+#endif // ASH_PICKER_VIEWS_PICKER_PREVIEW_METADATA_H_
diff --git a/ash/picker/views/picker_preview_metadata_unittest.cc b/ash/picker/views/picker_preview_metadata_unittest.cc
new file mode 100644
index 0000000..8e411cbb
--- /dev/null
+++ b/ash/picker/views/picker_preview_metadata_unittest.cc
@@ -0,0 +1,68 @@
+// Copyright 2024 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ash/picker/views/picker_preview_metadata.h"
+
+#include <memory>
+
+#include "base/files/file.h"
+#include "base/time/time.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace ash {
+namespace {
+
+TEST(PickerPreviewMetadataTest, ReturnsLastModified) {
+ base::File::Info only_modified;
+ ASSERT_TRUE(base::Time::FromString("23 Dec 2021 09:01:00",
+ &only_modified.last_modified));
+
+ EXPECT_EQ(PickerGetFilePreviewDescription(only_modified), u"Edited · Dec 23");
+}
+
+TEST(PickerPreviewMetadataTest, ReturnsLastAccessed) {
+ base::File::Info only_accessed;
+ ASSERT_TRUE(base::Time::FromString("23 Dec 2021 09:01:00",
+ &only_accessed.last_accessed));
+
+ EXPECT_EQ(PickerGetFilePreviewDescription(only_accessed),
+ u"You opened · Dec 23");
+}
+
+TEST(PickerPreviewMetadataTest, ReturnsModifiedIfNewer) {
+ base::File::Info modified_newer;
+ ASSERT_TRUE(base::Time::FromString("23 Dec 2021 09:01:00",
+ &modified_newer.last_modified));
+ ASSERT_TRUE(base::Time::FromString("23 Dec 2021 09:00:00",
+ &modified_newer.last_accessed));
+
+ EXPECT_EQ(PickerGetFilePreviewDescription(modified_newer),
+ u"Edited · Dec 23");
+}
+
+TEST(PickerPreviewMetadataTest, ReturnsAccessedIfNewer) {
+ base::File::Info accessed_newer;
+ ASSERT_TRUE(base::Time::FromString("23 Dec 2021 09:00:00",
+ &accessed_newer.last_modified));
+ ASSERT_TRUE(base::Time::FromString("23 Dec 2021 09:01:00",
+ &accessed_newer.last_accessed));
+
+ EXPECT_EQ(PickerGetFilePreviewDescription(accessed_newer),
+ u"You opened · Dec 23");
+}
+
+TEST(PickerPreviewMetadataTest, ReturnsModifiedIfSameAsAccessed) {
+ base::File::Info modified_newer;
+ ASSERT_TRUE(base::Time::FromString("23 Dec 2021 09:01:00",
+ &modified_newer.last_modified));
+ ASSERT_TRUE(base::Time::FromString("23 Dec 2021 09:01:00",
+ &modified_newer.last_accessed));
+
+ EXPECT_EQ(PickerGetFilePreviewDescription(modified_newer),
+ u"Edited · Dec 23");
+}
+
+} // namespace
+} // namespace ash
