Skip to content

Commit

Permalink
[FilesBulkPinning] Wire up cant-pin icon for unavailable item types
Browse files Browse the repository at this point in the history
There are a number of item types that can't be pinned, this wires up a
new inline status icon to show them for bulk pinning.

Bug: b:278002300
Test: browser_tests --gtest_filter=*drive*
Validate-Test-Flakiness: skip
Change-Id: I822e7012ad6ed0b214834dcc9edee15de23e8ec2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4554597
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Commit-Queue: Ben Reich <benreich@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1147693}
  • Loading branch information
ben-reich authored and Chromium LUCI CQ committed May 23, 2023
1 parent 5b8335d commit f9d4c73
Show file tree
Hide file tree
Showing 17 changed files with 188 additions and 42 deletions.
2 changes: 2 additions & 0 deletions chrome/browser/ash/file_manager/file_manager_browsertest.cc
Expand Up @@ -1568,6 +1568,8 @@ WRAPPED_INSTANTIATE_TEST_SUITE_P(
TestCase("driveFoldersRetainPinnedPropertyWhenBulkPinningEnabled")
.EnableBulkPinning(),
TestCase("drivePinToggleIsEnabledInSharedWithMeWhenBulkPinningEnabled")
.EnableBulkPinning(),
TestCase("driveCantPinItemsShouldHaveClassNameAndGetUpdatedWhenCanPin")
.EnableBulkPinning()
// TODO(b/189173190): Enable
// TestCase("driveEnableDocsOfflineDialog"),
Expand Down
28 changes: 28 additions & 0 deletions chrome/browser/ash/file_manager/file_manager_browsertest_base.cc
Expand Up @@ -443,6 +443,7 @@ struct AddEntriesMessage {
bool pinned = false; // Whether the file should be pinned.
bool available_offline = false; // Whether the file is available_offline.
std::string alternate_url; // Entry's alternate URL on Drive.
bool can_pin = true; // Whether the file can be pinned.

TestEntryInfo& SetSharedOption(SharedOption option) {
shared_option = option;
Expand Down Expand Up @@ -532,6 +533,7 @@ struct AddEntriesMessage {
&TestEntryInfo::available_offline);
converter->RegisterStringField("alternateUrl",
&TestEntryInfo::alternate_url);
converter->RegisterBoolField("canPin", &TestEntryInfo::can_pin);
}

// Maps |value| to an EntryType. Returns true on success.
Expand Down Expand Up @@ -1391,6 +1393,7 @@ class DriveFsTestVolume : public TestVolume {
entry.folder_feature.is_external_media;
metadata.alternate_url = entry.alternate_url;
metadata.shortcut = (entry.entry_type == AddEntriesMessage::LINK);
metadata.can_pin = entry.can_pin;
fake_drivefs_helper_->fake_drivefs().SetMetadata(std::move(metadata));

ASSERT_TRUE(UpdateModifiedTime(entry));
Expand Down Expand Up @@ -1454,6 +1457,22 @@ class DriveFsTestVolume : public TestVolume {
return fake_drivefs_helper_->fake_drivefs().IsItemPinned(path);
}

void SetCanPin(const std::string& path, bool can_pin) {
ASSERT_TRUE(fake_drivefs_helper_->fake_drivefs().SetCanPin(path, can_pin));

const base::FilePath file_path(path);
absl::optional<drivefs::FakeDriveFs::FileMetadata> metadata =
fake_drivefs_helper_->fake_drivefs().GetItemMetadata(file_path);
ASSERT_TRUE(metadata.has_value()) << "No file metadata with path: " << path;

std::vector<drivefs::mojom::FileChangePtr> file_changes;
file_changes.emplace_back(absl::in_place, file_path,
drivefs::mojom::FileChange::Type::kModify,
metadata.value().stable_id);
auto& drivefs_delegate = fake_drivefs_helper_->fake_drivefs().delegate();
drivefs_delegate->OnFilesChanged(std::move(file_changes));
}

private:
base::RepeatingCallback<std::unique_ptr<drivefs::DriveFsBootstrapListener>()>
CreateDriveFsBootstrapListener() {
Expand Down Expand Up @@ -3544,6 +3563,15 @@ void FileManagerBrowserTestBase::OnCommand(const std::string& name,
return;
}

if (name == "setCanPin") {
const std::string* path = value.FindString("path");
ASSERT_TRUE(path) << "No supplied path to setCanPin";
absl::optional<bool> can_pin = value.FindBool("canPin");
ASSERT_TRUE(can_pin.has_value()) << "Need to supply canPin";
drive_volume_->SetCanPin(*path, can_pin.value());
return;
}

if (name == "sendDriveCloudDeleteEvent") {
const std::string* path = value.FindString("path");
ASSERT_TRUE(path) << "No supplied path to sendDriveFilesChangedEvent";
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/ash/file_manager/file_manager_string_util.cc
Expand Up @@ -222,6 +222,8 @@ void AddStringsForDrive(base::Value::Dict* dict) {
IDS_FILE_BROWSER_BULK_PINNING_NOT_ENOUGH_SPACE_LABEL);
SET_STRING("DRIVE_PREPARING_TO_SYNC",
IDS_FILE_BROWSER_BULK_PINNING_PREPARING_TO_SYNC);
SET_STRING("DRIVE_ITEM_UNAVAILABLE_OFFLINE",
IDS_FILE_BROWSER_BULK_PINNING_ITEM_UNAVAILABLE_OFFLINE);
}

void AddStringsForMediaView(base::Value::Dict* dict) {
Expand Down
16 changes: 15 additions & 1 deletion chromeos/ash/components/drivefs/fake_drivefs.cc
Expand Up @@ -360,6 +360,7 @@ void FakeDriveFs::SetMetadata(const FakeMetadata& metadata) {
stored_metadata.shared = metadata.shared;
stored_metadata.shortcut = metadata.shortcut;
stored_metadata.alternate_url = metadata.alternate_url;
stored_metadata.can_pin = metadata.can_pin;
}

void FakeDriveFs::DisplayConfirmDialog(
Expand All @@ -378,6 +379,16 @@ absl::optional<bool> FakeDriveFs::IsItemPinned(const std::string& path) {
return absl::nullopt;
}

bool FakeDriveFs::SetCanPin(const std::string& path, bool can_pin) {
for (auto& metadata : metadata_) {
if (metadata.first.value() == path) {
metadata.second.can_pin = can_pin;
return true;
}
}
return false;
}

absl::optional<FakeDriveFs::FileMetadata> FakeDriveFs::GetItemMetadata(
const base::FilePath& path) {
const auto& metadata = metadata_.find(path);
Expand Down Expand Up @@ -450,8 +461,11 @@ void FakeDriveFs::GetMetadata(const base::FilePath& path,

metadata->capabilities = stored_metadata.capabilities.Clone();
metadata->stable_id = stored_metadata.stable_id;
using CanPinStatus = mojom::FileMetadata::CanPinStatus;
metadata->can_pin =
(stored_metadata.can_pin) ? CanPinStatus::kOk : CanPinStatus::kDisabled;
if (stored_metadata.hosted) {
metadata->can_pin = mojom::FileMetadata::CanPinStatus::kDisabled;
metadata->can_pin = CanPinStatus::kDisabled;
}
if (stored_metadata.shortcut) {
metadata->shortcut_details = mojom::ShortcutDetails::New();
Expand Down
4 changes: 4 additions & 0 deletions chromeos/ash/components/drivefs/fake_drivefs.h
Expand Up @@ -41,6 +41,7 @@ struct FakeMetadata {
std::string doc_id;
std::string alternate_url;
bool shortcut = false;
bool can_pin = true;
};

class FakeDriveFsBootstrapListener : public DriveFsBootstrapListener {
Expand Down Expand Up @@ -125,6 +126,8 @@ class FakeDriveFs : public drivefs::mojom::DriveFs,

absl::optional<bool> IsItemPinned(const std::string& path);

bool SetCanPin(const std::string& path, bool can_pin);

struct FileMetadata {
FileMetadata();
FileMetadata(const FileMetadata&);
Expand All @@ -143,6 +146,7 @@ class FakeDriveFs : public drivefs::mojom::DriveFs,
int64_t stable_id = 0;
std::string alternate_url;
bool shortcut = false;
bool can_pin = true;
};

absl::optional<FakeDriveFs::FileMetadata> GetItemMetadata(
Expand Down
3 changes: 3 additions & 0 deletions ui/chromeos/file_manager_strings.grdp
Expand Up @@ -1816,4 +1816,7 @@
<message name="IDS_FILE_BROWSER_DRIVE_SYNC_ERROR_TITLE" translateable="false" desc="Title of system notification displayed when an error occurred while syncing Google Drive files">
Sync error
</message>
<message name="IDS_FILE_BROWSER_BULK_PINNING_ITEM_UNAVAILABLE_OFFLINE" translateable="false" desc="The text used when the bulk pinning of Google Drive items has failed due to insufficient space.">
Item unavailable offline
</message>
</grit-part>
1 change: 1 addition & 0 deletions ui/file_manager/file_manager/BUILD.gn
Expand Up @@ -61,6 +61,7 @@ generate_grd("build_static_grdp") {
"foreground/images/files/ui/bulk_pinning_done.svg",
"foreground/images/files/ui/bulk_pinning_offline.svg",
"foreground/images/files/ui/check.svg",
"foreground/images/files/ui/cant_pin.svg",
"foreground/images/files/ui/cloud_done.svg",
"foreground/images/files/ui/cloud_error.svg",
"foreground/images/files/ui/cloud_offline.svg",
Expand Down
17 changes: 12 additions & 5 deletions ui/file_manager/file_manager/foreground/css/file_manager.css
Expand Up @@ -1406,17 +1406,21 @@ body.files-ng .thumbnail-frame > .img-container {
#list-container.thumbnail-view li.pinned:not([renaming]):not(.dim-offline) .inline-status,
#list-container.thumbnail-view li.pinned:not([renaming]):not(.dim-offline)[data-sync-status=not_found] .inline-status xf-icon,
#list-container.thumbnail-view li:not([renaming])[data-sync-status]:not([data-sync-status=not_found]) .inline-status,
#list-container.thumbnail-view li:not([renaming])[data-sync-status]:not([data-sync-status=not_found]) .inline-status .progress {
#list-container.thumbnail-view li:not([renaming])[data-sync-status]:not([data-sync-status=not_found]) .inline-status .progress,
#list-container.thumbnail-view li.cant-pin:not([renaming]) .inline-status,
#list-container.thumbnail-view li.cant-pin:not([renaming]) .inline-status xf-icon[type=cant-pin] {
display: flex;
}

#list-container.thumbnail-view li.pinned .inline-status xf-icon[type=offline] {
#list-container.thumbnail-view li.pinned .inline-status xf-icon[type=offline],
#list-container.thumbnail-view li.cant-pin .inline-status xf-icon[type=cant-pin] {
--xf-icon-color: var(--cros-icon-color-secondary);
}

/* Only display outlines for entries with thumbnails. */
#list-container.thumbnail-view li:has(.thumbnail) .inline-status .progress,
#list-container.thumbnail-view li.pinned:has(.thumbnail) .inline-status xf-icon[type=offline] {
#list-container.thumbnail-view li.pinned:has(.thumbnail) .inline-status xf-icon[type=offline],
#list-container.thumbnail-view li.cant-pin:has(.thumbnail) .inline-status xf-icon[type=cant-pin] {
--xf-icon-color-outline: var(--cros-bg-color);
}

Expand Down Expand Up @@ -2362,12 +2366,15 @@ body.files-ng #list-container li .detail-thumbnail {
#list-container .list li.pinned:not([renaming]):not(.dim-offline) .inline-status,
#list-container .list li.pinned:not([renaming]):not(.dim-offline)[data-sync-status=not_found] .inline-status xf-icon,
#list-container .list li:not([renaming])[data-sync-status]:not([data-sync-status=not_found]) .inline-status,
#list-container .list li:not([renaming])[data-sync-status]:not([data-sync-status=not_found]) .inline-status .progress {
#list-container .list li:not([renaming])[data-sync-status]:not([data-sync-status=not_found]) .inline-status .progress,
#list-container .list li.cant-pin:not([renaming]) .inline-status,
#list-container .list li.cant-pin:not([renaming]) .inline-status xf-icon[type=cant-pin] {
display: flex;
}

#list-container .list li.encrypted .encryption-status xf-icon[type=encrypted],
#list-container .list li.pinned .inline-status xf-icon[type=offline] {
#list-container .list li.pinned .inline-status xf-icon[type=offline],
#list-container .list li.cant-pin .inline-status xf-icon[type=cant-pin] {
--xf-icon-color: var(--cros-icon-color-secondary);
}

Expand Down
14 changes: 10 additions & 4 deletions ui/file_manager/file_manager/foreground/css/file_manager_gm3.css
Expand Up @@ -1191,11 +1191,14 @@ body.files-ng .thumbnail-frame > .img-container {
#list-container.thumbnail-view li.pinned:not([renaming]):not(.dim-offline) .inline-status,
#list-container.thumbnail-view li.pinned:not([renaming]):not(.dim-offline)[data-sync-status=not_found] .inline-status xf-icon,
#list-container.thumbnail-view li:not([renaming])[data-sync-status]:not([data-sync-status=not_found]) .inline-status,
#list-container.thumbnail-view li:not([renaming])[data-sync-status]:not([data-sync-status=not_found]) .inline-status .progress {
#list-container.thumbnail-view li:not([renaming])[data-sync-status]:not([data-sync-status=not_found]) .inline-status .progress,
#list-container.thumbnail-view li.cant-pin:not([renaming]) .inline-status,
#list-container.thumbnail-view li.cant-pin:not([renaming]) .inline-status xf-icon[type=cant-pin] {
display: flex;
}

#list-container.thumbnail-view li.pinned .inline-status xf-icon[type=offline] {
#list-container.thumbnail-view li.pinned .inline-status xf-icon[type=offline],
#list-container.thumbnail-view li.cant-pin .inline-status xf-icon[type=cant-pin] {
--xf-icon-color: currentColor;
}

Expand Down Expand Up @@ -2121,12 +2124,15 @@ body.files-ng #list-container li .detail-checkmark {
#list-container .list li.pinned:not([renaming]):not(.dim-offline) .inline-status,
#list-container .list li.pinned:not([renaming]):not(.dim-offline)[data-sync-status=not_found] .inline-status xf-icon,
#list-container .list li:not([renaming])[data-sync-status]:not([data-sync-status=not_found]) .inline-status,
#list-container .list li:not([renaming])[data-sync-status]:not([data-sync-status=not_found]) .inline-status .progress {
#list-container .list li:not([renaming])[data-sync-status]:not([data-sync-status=not_found]) .inline-status .progress,
#list-container .list li.cant-pin:not([renaming]) .inline-status,
#list-container .list li.cant-pin:not([renaming]) .inline-status xf-icon[type=cant-pin] {
display: flex;
}

#list-container .list li.encrypted .encryption-status xf-icon[type=encrypted],
#list-container .list li.pinned .inline-status xf-icon[type=offline] {
#list-container .list li.pinned .inline-status xf-icon[type=offline],
#list-container .list li.cant-pin .inline-status xf-icon[type=cant-pin] {
--xf-icon-color: currentColor;
}

Expand Down
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions ui/file_manager/file_manager/foreground/js/constants.js
Expand Up @@ -134,6 +134,7 @@ constants.ICON_TYPES = {
BULK_PINNING_DONE: 'bulk_pinning_done',
BULK_PINNING_OFFLINE: 'bulk_pinning_offline',
CAMERA_FOLDER: 'camera-folder',
CANT_PIN: 'cant-pin',
CHECK: 'check',
CLOUD_DONE: 'cloud_done',
CLOUD_ERROR: 'cloud_error',
Expand Down
14 changes: 11 additions & 3 deletions ui/file_manager/file_manager/foreground/js/ui/file_grid.js
Expand Up @@ -811,9 +811,10 @@ export class FileGrid extends Grid {
const bottom = li.ownerDocument.createElement('div');
bottom.className = 'thumbnail-bottom';

const {contentMimeType, availableOffline, pinned} =
const {contentMimeType, availableOffline, pinned, canPin} =
this.metadataModel_.getCache(
[entry], ['contentMimeType', 'availableOffline', 'pinned'])[0] ||
[entry],
['contentMimeType', 'availableOffline', 'pinned', 'canPin'])[0] ||
{};

const locationInfo = this.volumeManager_.getLocationInfo(entry);
Expand All @@ -840,7 +841,14 @@ export class FileGrid extends Grid {

const inlineStatusIcon = li.ownerDocument.createElement('xf-icon');
inlineStatusIcon.size = 'extra_small';
inlineStatusIcon.type = 'offline';
if (util.isDriveFsBulkPinningEnabled() && !util.isNullOrUndefined(canPin) &&
!canPin) {
inlineStatusIcon.type = 'cant-pin';
li.classList.toggle('cant-pin', true);
} else {
inlineStatusIcon.type = 'offline';
li.classList.toggle('cant-pin', false);
}
inlineStatus.appendChild(inlineStatusIcon);

if (util.isInlineSyncStatusEnabled()) {
Expand Down
Expand Up @@ -1034,6 +1034,7 @@ export class FileTable extends Table {
'syncStatus',
'progress',
'shortcut',
'canPin',
])[0],
util.isTeamDriveRoot(entry));
listItem.toggleAttribute(
Expand Down
83 changes: 54 additions & 29 deletions ui/file_manager/file_manager/foreground/js/ui/file_table_list.js
Expand Up @@ -368,6 +368,7 @@ filelist.decorateListItem = (li, entry, metadataModel, volumeManager) => {
'progress',
'contentMimeType',
'shortcut',
'canPin',
])[0];
filelist.updateListItemExternalProps(
li, entry, externalProps, util.isTeamDriveRoot(entry));
Expand Down Expand Up @@ -571,37 +572,61 @@ filelist.updateListItemExternalProps =
}

const inlineStatus = li.querySelector('.inline-status');
if (inlineStatus) {
// Clear the inline status' aria label and set it to "in progress",
// "queued", or "available offline" with the respective order of
// precedence if applicable.
inlineStatus.setAttribute(
'aria-label',
externalProps.pinned ? str('OFFLINE_COLUMN_LABEL') : '');

const {syncStatus} = externalProps;
let progress = externalProps.progress ?? 0;
if (util.isInlineSyncStatusEnabled() && syncStatus) {
switch (syncStatus) {
case chrome.fileManagerPrivate.SyncStatus.QUEUED:
case chrome.fileManagerPrivate.SyncStatus.ERROR:
progress = 0;
inlineStatus.setAttribute('aria-label', str('QUEUED_LABEL'));
break;
case chrome.fileManagerPrivate.SyncStatus.IN_PROGRESS:
inlineStatus.setAttribute(
'aria-label',
`${str('IN_PROGRESS_LABEL')} - ${
(progress * 100).toFixed(0)}%`);
break;
default:
break;
}
if (!inlineStatus) {
return;
}

if (util.isDriveFsBulkPinningEnabled()) {
const inlineIcon = inlineStatus.querySelector('xf-icon');

if (!util.isNullOrUndefined(externalProps.canPin) &&
!externalProps.canPin) {
// Items that can't be pinned should show a dashed icon to indicate
// they cannot be used offline (e.g. google forms can't be made
// available offline).
li.classList.toggle('cant-pin', true);
inlineIcon.type = 'cant-pin';
inlineStatus.setAttribute(
'aria-label', str('DRIVE_ITEM_UNAVAILABLE_OFFLINE'));
return;
}

// In the event a previous item that could not be pinned has instead
// become pinnable, ensure the inline status icon is reset and the class
// is removed.
li.classList.toggle('cant-pin', false);
inlineIcon.type = 'offline';
}

li.setAttribute('data-sync-status', syncStatus);
li.querySelector('.progress')
.setAttribute('progress', progress.toFixed(2));
// Clear the inline status' aria label and set it to "in progress",
// "queued", or "available offline" with the respective order of
// precedence if applicable.
inlineStatus.setAttribute(
'aria-label',
externalProps.pinned ? str('OFFLINE_COLUMN_LABEL') : '');

const {syncStatus} = externalProps;
let progress = externalProps.progress ?? 0;
if (util.isInlineSyncStatusEnabled() && syncStatus) {
switch (syncStatus) {
case chrome.fileManagerPrivate.SyncStatus.QUEUED:
case chrome.fileManagerPrivate.SyncStatus.ERROR:
progress = 0;
inlineStatus.setAttribute('aria-label', str('QUEUED_LABEL'));
break;
case chrome.fileManagerPrivate.SyncStatus.IN_PROGRESS:
inlineStatus.setAttribute(
'aria-label',
`${str('IN_PROGRESS_LABEL')} - ${
(progress * 100).toFixed(0)}%`);
break;
default:
break;
}

li.setAttribute('data-sync-status', syncStatus);
li.querySelector('.progress')
.setAttribute('progress', progress.toFixed(2));
}
};

Expand Down
4 changes: 4 additions & 0 deletions ui/file_manager/file_manager/widgets/xf_icon.ts
Expand Up @@ -192,6 +192,10 @@ function getCSS() {
-webkit-mask-image: url(../foreground/images/volumes/camera.svg);
}
:host([type="cant-pin"]) span {
-webkit-mask-image: url(../foreground/images/files/ui/cant_pin.svg);
}
:host([type="computer"]) span {
-webkit-mask-image: url(../foreground/images/volumes/computer.svg);
}
Expand Down

0 comments on commit f9d4c73

Please sign in to comment.