0

holding space exp: Add new pinned files placeholder strings

Bug: 1363339
Change-Id: Ia680c0c518969b52d94778a2932bed80cee89c17
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3911013
Reviewed-by: David Black <dmblack@google.com>
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Angus McLean <angusmclean@google.com>
Cr-Commit-Position: refs/heads/main@{#1050798}
This commit is contained in:
Angus L. M. McLean IV
2022-09-23 22:34:33 +00:00
committed by Chromium LUCI CQ
parent 2a8cdb7bc4
commit 4bd4cfdd01
10 changed files with 87 additions and 19 deletions

@@ -1329,6 +1329,12 @@ This file contains the strings for ash.
<message name="IDS_ASH_HOLDING_SPACE_PINNED_EMPTY_PROMPT" desc="Prompt shown in the pinned files area of the holding space bubble if the user has no pinned files."> <message name="IDS_ASH_HOLDING_SPACE_PINNED_EMPTY_PROMPT" desc="Prompt shown in the pinned files area of the holding space bubble if the user has no pinned files.">
You can pin your important files here. Open Files app to get started. You can pin your important files here. Open Files app to get started.
</message> </message>
<message name="IDS_ASH_HOLDING_SPACE_PINNED_EMPTY_PROMPT_SUGGESTIONS" desc="Prompt shown in the pinned files area of the holding space bubble if the user has no pinned files and suggestions are enabled.">
You can pin important files, including Google Drive files. To pin, hover over an item or open Files and right-click an item.
</message>
<message name="IDS_ASH_HOLDING_SPACE_PINNED_EMPTY_PROMPT_SUGGESTIONS_DRIVE_DISABLED" desc="Prompt shown in the pinned files area of the holding space bubble if the user has no pinned files and suggestions are enabled, but Drive is disabled.">
You can pin important files. To pin, hover over an item or open Files and right-click an item.
</message>
<message name="IDS_ASH_HOLDING_SPACE_PINNED_FILES_APP_CHIP_TEXT" desc="Text for chip which opens the Files app shown in the pinned files area of the holding space bubble if the user has no pinned files."> <message name="IDS_ASH_HOLDING_SPACE_PINNED_FILES_APP_CHIP_TEXT" desc="Text for chip which opens the Files app shown in the pinned files area of the holding space bubble if the user has no pinned files.">
Open Files Open Files
</message> </message>

@@ -0,0 +1 @@
107945efc3eecefa6658f3607d41c9a36043111e

@@ -0,0 +1 @@
e0f5562bb0edba3c093f59f0d414a760344e946c

@@ -45,6 +45,10 @@ class ASH_PUBLIC_EXPORT HoldingSpaceClient {
virtual base::FilePath CrackFileSystemUrl( virtual base::FilePath CrackFileSystemUrl(
const GURL& file_system_url) const = 0; const GURL& file_system_url) const = 0;
// Returns the value of the `drive::prefs::kDisableDrive` pref, indicating
// whether Google Drive has been disabled.
virtual bool IsDriveDisabled() const = 0;
// Attempts to open the Downloads folder. // Attempts to open the Downloads folder.
// Success is returned via the supplied `callback`. // Success is returned via the supplied `callback`.
virtual void OpenDownloads(SuccessCallback callback) = 0; virtual void OpenDownloads(SuccessCallback callback) = 0;

@@ -43,6 +43,7 @@ class MockHoldingSpaceClient : public HoldingSpaceClient {
CrackFileSystemUrl, CrackFileSystemUrl,
(const GURL& file_system_url), (const GURL& file_system_url),
(const, override)); (const, override));
MOCK_METHOD(bool, IsDriveDisabled, (), (const, override));
MOCK_METHOD(void, OpenDownloads, (SuccessCallback callback), (override)); MOCK_METHOD(void, OpenDownloads, (SuccessCallback callback), (override));
MOCK_METHOD(void, OpenMyFiles, (SuccessCallback callback), (override)); MOCK_METHOD(void, OpenMyFiles, (SuccessCallback callback), (override));
MOCK_METHOD(void, MOCK_METHOD(void,

@@ -3452,6 +3452,10 @@ class HoldingSpaceTraySuggestionsFeatureTest
scoped_feature_list_.InitWithFeatures(enabled_features, disabled_features); scoped_feature_list_.InitWithFeatures(enabled_features, disabled_features);
} }
void SetDisableDrive(bool disable) {
ON_CALL(*client(), IsDriveDisabled).WillByDefault(testing::Return(disable));
}
bool IsHoldingSpacePredictabilityEnabled() const { bool IsHoldingSpacePredictabilityEnabled() const {
return std::get<0>(GetParam()); return std::get<0>(GetParam());
} }
@@ -3577,12 +3581,30 @@ TEST_P(HoldingSpaceTraySuggestionsFeatureTest,
kHoldingSpacePinnedFilesSectionPlaceholderLabelId)); kHoldingSpacePinnedFilesSectionPlaceholderLabelId));
ASSERT_TRUE(suggestions_placeholder_label); ASSERT_TRUE(suggestions_placeholder_label);
// TODO(https://crbug.com/1363339): Replace the placeholder text below when
// the final string is added.
std::u16string expected_text = std::u16string expected_text =
IsHoldingSpaceSuggestionsEnabled() IsHoldingSpaceSuggestionsEnabled()
? u"[i18n]You can pin important files here, from the Files app, as " ? l10n_util::GetStringUTF16(
u"well as from Google Slides, Docs, and Drive." IDS_ASH_HOLDING_SPACE_PINNED_EMPTY_PROMPT_SUGGESTIONS)
: l10n_util::GetStringUTF16(
IDS_ASH_HOLDING_SPACE_PINNED_EMPTY_PROMPT);
EXPECT_EQ(suggestions_placeholder_label->GetText(), expected_text);
// Also check to make sure that the label is adjusted when drive is disabled.
test_api()->Close();
SetDisableDrive(true);
test_api()->Show();
pinned_files_bubble = test_api()->GetPinnedFilesBubble();
ASSERT_TRUE(pinned_files_bubble);
suggestions_placeholder_label =
static_cast<views::Label*>(pinned_files_bubble->GetViewByID(
kHoldingSpacePinnedFilesSectionPlaceholderLabelId));
ASSERT_TRUE(suggestions_placeholder_label);
expected_text =
IsHoldingSpaceSuggestionsEnabled()
? l10n_util::GetStringUTF16(
IDS_ASH_HOLDING_SPACE_PINNED_EMPTY_PROMPT_SUGGESTIONS_DRIVE_DISABLED)
: l10n_util::GetStringUTF16( : l10n_util::GetStringUTF16(
IDS_ASH_HOLDING_SPACE_PINNED_EMPTY_PROMPT); IDS_ASH_HOLDING_SPACE_PINNED_EMPTY_PROMPT);
EXPECT_EQ(suggestions_placeholder_label->GetText(), expected_text); EXPECT_EQ(suggestions_placeholder_label->GetText(), expected_text);

@@ -25,6 +25,7 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/callback_helpers.h" #include "base/callback_helpers.h"
#include "build/branding_buildflags.h" #include "build/branding_buildflags.h"
#include "components/prefs/pref_service.h"
#include "components/vector_icons/vector_icons.h" #include "components/vector_icons/vector_icons.h"
#include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util.h"
#include "ui/color/color_id.h" #include "ui/color/color_id.h"
@@ -80,15 +81,15 @@ views::Builder<views::ImageView> CreateGSuiteIcon(const gfx::VectorIcon& icon) {
} }
#endif #endif
std::u16string GetPlaceholderText() { std::u16string GetPlaceholderText(bool drive_disabled) {
int message_id = IDS_ASH_HOLDING_SPACE_PINNED_EMPTY_PROMPT;
if (features::IsHoldingSpaceSuggestionsEnabled()) { if (features::IsHoldingSpaceSuggestionsEnabled()) {
// TODO(https://crbug.com/1363339): Replace the placeholder text below with message_id =
// the final proper internationalized string once the exact verbiage is drive_disabled
// decided. Also we'll need a separate string for when drive is disabled. ? IDS_ASH_HOLDING_SPACE_PINNED_EMPTY_PROMPT_SUGGESTIONS_DRIVE_DISABLED
return u"[i18n]You can pin important files here, from the Files app, as " : IDS_ASH_HOLDING_SPACE_PINNED_EMPTY_PROMPT_SUGGESTIONS;
u"well as from Google Slides, Docs, and Drive.";
} }
return l10n_util::GetStringUTF16(IDS_ASH_HOLDING_SPACE_PINNED_EMPTY_PROMPT); return l10n_util::GetStringUTF16(message_id);
} }
// FilesAppChip ---------------------------------------------------------------- // FilesAppChip ----------------------------------------------------------------
@@ -224,6 +225,9 @@ std::unique_ptr<views::View> PinnedFilesSection::CreatePlaceholder() {
if (!ShouldShowPlaceholder(prefs)) if (!ShouldShowPlaceholder(prefs))
return nullptr; return nullptr;
bool drive_disabled =
HoldingSpaceController::Get()->client()->IsDriveDisabled();
auto placeholder_builder = auto placeholder_builder =
views::Builder<views::BoxLayoutView>() views::Builder<views::BoxLayoutView>()
.SetCrossAxisAlignment(views::BoxLayout::CrossAxisAlignment::kStart) .SetCrossAxisAlignment(views::BoxLayout::CrossAxisAlignment::kStart)
@@ -232,24 +236,27 @@ std::unique_ptr<views::View> PinnedFilesSection::CreatePlaceholder() {
.AddChild( .AddChild(
views::Builder<views::Label>( views::Builder<views::Label>(
bubble_utils::CreateLabel(bubble_utils::LabelStyle::kBody, bubble_utils::CreateLabel(bubble_utils::LabelStyle::kBody,
GetPlaceholderText())) GetPlaceholderText(drive_disabled)))
.SetHorizontalAlignment(gfx::HorizontalAlignment::ALIGN_LEFT) .SetHorizontalAlignment(gfx::HorizontalAlignment::ALIGN_LEFT)
.SetMultiLine(true) .SetMultiLine(true)
.SetID(kHoldingSpacePinnedFilesSectionPlaceholderLabelId)); .SetID(kHoldingSpacePinnedFilesSectionPlaceholderLabelId));
// TODO(https://crbug.com/1361645): Also check if drive is disabled.
if (features::IsHoldingSpaceSuggestionsEnabled()) { if (features::IsHoldingSpaceSuggestionsEnabled()) {
#if BUILDFLAG(GOOGLE_CHROME_BRANDING) #if BUILDFLAG(GOOGLE_CHROME_BRANDING)
// G Suite icons. // G Suite icons.
placeholder_builder.AddChild( auto icons_builder =
views::Builder<views::BoxLayoutView>() views::Builder<views::BoxLayoutView>()
.SetOrientation(views::BoxLayout::Orientation::kHorizontal) .SetOrientation(views::BoxLayout::Orientation::kHorizontal)
.SetBetweenChildSpacing(kPlaceholderGSuiteIconSpacing) .SetBetweenChildSpacing(kPlaceholderGSuiteIconSpacing)
.SetID(kHoldingSpacePinnedFilesSectionPlaceholderGSuiteIconsId) .SetID(kHoldingSpacePinnedFilesSectionPlaceholderGSuiteIconsId);
.AddChild(CreateGSuiteIcon(vector_icons::kGoogleDriveIcon))
.AddChild(CreateGSuiteIcon(vector_icons::kGoogleSlidesIcon)) if (!drive_disabled)
.AddChild(CreateGSuiteIcon(vector_icons::kGoogleDocsIcon)) icons_builder.AddChild(CreateGSuiteIcon(vector_icons::kGoogleDriveIcon));
.AddChild(CreateGSuiteIcon(vector_icons::kGoogleSheetsIcon)));
icons_builder.AddChild(CreateGSuiteIcon(vector_icons::kGoogleSlidesIcon))
.AddChild(CreateGSuiteIcon(vector_icons::kGoogleDocsIcon))
.AddChild(CreateGSuiteIcon(vector_icons::kGoogleSheetsIcon));
placeholder_builder.AddChild(std::move(icons_builder));
#endif #endif
} else { } else {
// Files app chip. // Files app chip.

@@ -23,6 +23,8 @@
#include "chrome/browser/ui/ash/holding_space/holding_space_keyed_service.h" #include "chrome/browser/ui/ash/holding_space/holding_space_keyed_service.h"
#include "chrome/browser/ui/ash/holding_space/holding_space_keyed_service_factory.h" #include "chrome/browser/ui/ash/holding_space/holding_space_keyed_service_factory.h"
#include "chrome/browser/ui/ash/holding_space/holding_space_util.h" #include "chrome/browser/ui/ash/holding_space/holding_space_util.h"
#include "components/drive/drive_pref_names.h"
#include "components/prefs/pref_service.h"
#include "net/base/mime_util.h" #include "net/base/mime_util.h"
#include "storage/browser/file_system/file_system_context.h" #include "storage/browser/file_system/file_system_context.h"
@@ -140,6 +142,10 @@ base::FilePath HoldingSpaceClientImpl::CrackFileSystemUrl(
.path(); .path();
} }
bool HoldingSpaceClientImpl::IsDriveDisabled() const {
return profile_->GetPrefs()->GetBoolean(drive::prefs::kDisableDrive);
}
void HoldingSpaceClientImpl::OpenDownloads(SuccessCallback callback) { void HoldingSpaceClientImpl::OpenDownloads(SuccessCallback callback) {
auto file_path = file_manager::util::GetDownloadsFolderForProfile(profile_); auto file_path = file_manager::util::GetDownloadsFolderForProfile(profile_);
if (file_path.empty()) { if (file_path.empty()) {

@@ -30,6 +30,7 @@ class HoldingSpaceClientImpl : public HoldingSpaceClient {
void AddScreenshot(const base::FilePath& file_path) override; void AddScreenshot(const base::FilePath& file_path) override;
void CopyImageToClipboard(const HoldingSpaceItem&, SuccessCallback) override; void CopyImageToClipboard(const HoldingSpaceItem&, SuccessCallback) override;
base::FilePath CrackFileSystemUrl(const GURL& file_system_url) const override; base::FilePath CrackFileSystemUrl(const GURL& file_system_url) const override;
bool IsDriveDisabled() const override;
void OpenDownloads(SuccessCallback callback) override; void OpenDownloads(SuccessCallback callback) override;
void OpenItems(const std::vector<const HoldingSpaceItem*>& items, void OpenItems(const std::vector<const HoldingSpaceItem*>& items,
SuccessCallback callback) override; SuccessCallback callback) override;

@@ -21,10 +21,13 @@
#include "base/unguessable_token.h" #include "base/unguessable_token.h"
#include "chrome/browser/ash/file_manager/path_util.h" #include "chrome/browser/ash/file_manager/path_util.h"
#include "chrome/browser/extensions/component_loader.h" #include "chrome/browser/extensions/component_loader.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_manager.h" #include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/ui/ash/holding_space/holding_space_browsertest_base.h" #include "chrome/browser/ui/ash/holding_space/holding_space_browsertest_base.h"
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
#include "chrome/test/base/in_process_browser_test.h" #include "chrome/test/base/in_process_browser_test.h"
#include "components/drive/drive_pref_names.h"
#include "components/prefs/pref_service.h"
#include "content/public/test/browser_test.h" #include "content/public/test/browser_test.h"
#include "storage/browser/file_system/external_mount_points.h" #include "storage/browser/file_system/external_mount_points.h"
#include "ui/gfx/image/image_skia.h" #include "ui/gfx/image/image_skia.h"
@@ -149,6 +152,22 @@ IN_PROC_BROWSER_TEST_F(HoldingSpaceClientImplTest, CopyImageToClipboard) {
} }
} }
// Verifies that `HoldingSpaceClient::IsDriveDisabled()` works as intended.
IN_PROC_BROWSER_TEST_F(HoldingSpaceClientImplTest, IsDriveDisabled) {
ASSERT_TRUE(HoldingSpaceController::Get());
auto* holding_space_client = HoldingSpaceController::Get()->client();
ASSERT_TRUE(holding_space_client);
auto* prefs = GetProfile()->GetPrefs();
EXPECT_EQ(holding_space_client->IsDriveDisabled(),
prefs->GetBoolean(drive::prefs::kDisableDrive));
prefs->SetBoolean(drive::prefs::kDisableDrive, true);
EXPECT_EQ(holding_space_client->IsDriveDisabled(), true);
prefs->SetBoolean(drive::prefs::kDisableDrive, false);
EXPECT_EQ(holding_space_client->IsDriveDisabled(), false);
}
// Verifies that `HoldingSpaceClient::OpenDownloads()` works as intended. // Verifies that `HoldingSpaceClient::OpenDownloads()` works as intended.
IN_PROC_BROWSER_TEST_F(HoldingSpaceClientImplTest, OpenDownloads) { IN_PROC_BROWSER_TEST_F(HoldingSpaceClientImplTest, OpenDownloads) {
ASSERT_TRUE(HoldingSpaceController::Get()); ASSERT_TRUE(HoldingSpaceController::Get());