Skip to content

Commit

Permalink
[FilesTrash] Update strings to be UX compliant
Browse files Browse the repository at this point in the history
The Trash strings [1] have been approved by UX and this largely
implements them. One of the strings required a "Restore" button on the
dialog so this updates the dialog to perform restoration when it is
clicked.

[1] http://shortn/_NxiWJ2VJ22

Bug: b:245853299
Test: browser_tests --gtest_filter=*trash*
Change-Id: If2318f89aea429190082101292019065e808e6ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3920252
Commit-Queue: Ben Reich <benreich@chromium.org>
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1052185}
  • Loading branch information
ben-reich authored and Chromium LUCI CQ committed Sep 28, 2022
1 parent a7593fc commit f528300
Show file tree
Hide file tree
Showing 27 changed files with 169 additions and 54 deletions.
7 changes: 5 additions & 2 deletions chrome/browser/ash/file_manager/file_manager_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1258,7 +1258,7 @@ WRAPPED_INSTANTIATE_TEST_SUITE_P(
// TestCase("transferDismissedErrorIsRemembered"),
TestCase("transferNotSupportedOperationHasNoRemainingTimeText"),
TestCase("transferUpdateSamePanelItem"),
TestCase("transferShowPendingMessageForZeroRemainingTime")));
TestCase("transferShowPreparingMessageForZeroRemainingTime")));

WRAPPED_INSTANTIATE_TEST_SUITE_P(
DLP, /* dlp.js */
Expand Down Expand Up @@ -1732,7 +1732,10 @@ WRAPPED_INSTANTIATE_TEST_SUITE_P(
.EnableTrash(),
TestCase("trashTogglingTrashEnabledPrefUpdatesDirectoryTree")
.EnableTrash(),
TestCase("trashCantRestoreWhenParentDoesntExist").EnableTrash()));
TestCase("trashCantRestoreWhenParentDoesntExist").EnableTrash(),
TestCase(
"trashPressingEnterOnFileInTrashRootShowsDialogWithRestoreButton")
.EnableTrash()));

WRAPPED_INSTANTIATE_TEST_SUITE_P(
AndroidPhotos, /* android_photos.js */
Expand Down
23 changes: 17 additions & 6 deletions chrome/browser/ash/file_manager/file_manager_string_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -717,12 +717,22 @@ void AddStringsGeneric(base::Value::Dict* dict) {
IDS_FILE_BROWSER_RESTORE_FROM_TRASH_BUTTON_LABEL);
SET_STRING("RESTORE_FROM_TRASH_ERROR",
IDS_FILE_BROWSER_RESTORE_FROM_TRASH_ERROR);
SET_STRING("OPEN_TRASHED_FILES_ERROR",
IDS_FILE_BROWSER_OPEN_TRASHED_FILES_ERROR);
SET_STRING("RESTORE_FROM_TRASH_FILE_NAME",
IDS_FILE_BROWSER_RESTORE_FROM_TRASH_FILE_NAME);
SET_STRING("RESTORE_FROM_TRASH_ITEMS_REMAINING",
IDS_FILE_BROWSER_RESTORE_FROM_TRASH_ITEMS_REMAINING);
SET_STRING("OPEN_TRASHED_FILE_ERROR_TITLE",
IDS_FILE_BROWSER_OPEN_TRASHED_FILE_ERROR_TITLE);
SET_STRING("OPEN_TRASHED_FILE_ERROR_DESC",
IDS_FILE_BROWSER_OPEN_TRASHED_FILE_ERROR_DESC);
SET_STRING("OPEN_TRASHED_FILES_ERROR_TITLE",
IDS_FILE_BROWSER_OPEN_TRASHED_FILES_ERROR_TITLE);
SET_STRING("OPEN_TRASHED_FILES_ERROR_DESC",
IDS_FILE_BROWSER_OPEN_TRASHED_FILES_ERROR_DESC);
SET_STRING("RESTORING_FROM_TRASH_FILE_NAME",
IDS_FILE_BROWSER_RESTORING_FROM_TRASH_FILE_NAME);
SET_STRING("RESTORING_FROM_TRASH_ITEMS_REMAINING",
IDS_FILE_BROWSER_RESTORING_FROM_TRASH_ITEMS_REMAINING);
SET_STRING("RESTORE_TRASH_FILE_NAME",
IDS_FILE_BROWSER_RESTORE_TRASH_FILE_NAME);
SET_STRING("RESTORE_TRASH_MANY_ITEMS",
IDS_FILE_BROWSER_RESTORE_TRASH_MANY_ITEMS);
SET_STRING("UNPIN_FOLDER_BUTTON_LABEL",
IDS_FILE_BROWSER_UNPIN_FOLDER_BUTTON_LABEL);
SET_STRING("RENAME_BUTTON_LABEL", IDS_FILE_BROWSER_RENAME_BUTTON_LABEL);
Expand Down Expand Up @@ -875,6 +885,7 @@ void AddStringsGeneric(base::Value::Dict* dict) {
SET_STRING("TYPE_COLUMN_LABEL", IDS_FILE_BROWSER_TYPE_COLUMN_LABEL);
SET_STRING("UNDO_DELETE_ACTION_LABEL",
IDS_FILE_BROWSER_UNDO_DELETE_ACTION_LABEL);
SET_STRING("RESTORE_ACTION_LABEL", IDS_FILE_BROWSER_RESTORE_ACTION_LABEL);
SET_STRING("UNDO_DELETE_ONE", IDS_FILE_BROWSER_UNDO_DELETE_ONE);
SET_STRING("UNDO_DELETE_SOME", IDS_FILE_BROWSER_UNDO_DELETE_SOME);
SET_STRING("MOVE_TO_TRASH_FILE_NAME",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,9 @@ std::u16string GetIOTaskMessage(Profile* profile,
multiple_file_message_id = IDS_FILE_BROWSER_ZIP_ITEMS_REMAINING;
break;
case OperationType::kRestoreToDestination:
single_file_message_id = IDS_FILE_BROWSER_RESTORE_FROM_TRASH_FILE_NAME;
single_file_message_id = IDS_FILE_BROWSER_RESTORING_FROM_TRASH_FILE_NAME;
multiple_file_message_id =
IDS_FILE_BROWSER_RESTORE_FROM_TRASH_ITEMS_REMAINING;
IDS_FILE_BROWSER_RESTORING_FROM_TRASH_ITEMS_REMAINING;
break;
case OperationType::kTrash:
single_file_message_id = IDS_FILE_BROWSER_MOVE_TO_TRASH_FILE_NAME;
Expand Down
40 changes: 29 additions & 11 deletions ui/chromeos/file_manager_strings.grdp
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@
Delete
</message>
<message name="IDS_FILE_BROWSER_MOVE_TO_TRASH_BUTTON_LABEL" desc="Context menu item that moves the currently-selected file(s) to the trash / recycle bin.">
Move to Trash
Move to trash
</message>
<message name="IDS_FILE_BROWSER_RESTORE_FROM_TRASH_BUTTON_LABEL" desc="Context menu item that restores the currently-selected file(s) from the trash / recycle bin.">
Restore from trash
Expand Down Expand Up @@ -717,11 +717,17 @@
<message name="IDS_FILE_BROWSER_DELETE_ERROR" desc="Message informing about error while deleting an item or items.">
An error occurred. Some items may not have been deleted.
</message>
<message name="IDS_FILE_BROWSER_RESTORE_FROM_TRASH_FILE_NAME" desc="File Manager status message shown when restoring items from trash / recycle bin.">
Restoring "<ph name="FILENAME">$1<ex>photo.jpg</ex></ph>"...
<message name="IDS_FILE_BROWSER_RESTORING_FROM_TRASH_FILE_NAME" desc="File Manager status message shown when restoring items from trash / recycle bin.">
Restoring "<ph name="FILENAME">$1<ex>photo.jpg</ex></ph>"
</message>
<message name="IDS_FILE_BROWSER_RESTORE_FROM_TRASH_ITEMS_REMAINING" desc="File Manager status message shown when restoring items from trash / recycle bin. 'Item' is used here as a generic term for file or directory.">
Restoring <ph name="COUNT">$1<ex>10</ex></ph> items...
<message name="IDS_FILE_BROWSER_RESTORING_FROM_TRASH_ITEMS_REMAINING" desc="File Manager status message shown when restoring items from trash / recycle bin. 'Item' is used here as a generic term for file or directory.">
Restoring <ph name="COUNT">$1<ex>10</ex></ph> items
</message>
<message name="IDS_FILE_BROWSER_RESTORE_TRASH_FILE_NAME" desc="When a file has been successfully restored.">
<ph name="FILE_NAME">$1<ex>file.txt</ex></ph> restored
</message>
<message name="IDS_FILE_BROWSER_RESTORE_TRASH_MANY_ITEMS" desc="Shown when a number of files have been successfully restored.">
<ph name="NUMBER_OF_ITEMS">$1<ex>3</ex></ph> items restored
</message>
<message name="IDS_FILE_BROWSER_TRASH_DELETED_FOREVER" desc="Message informing that items in the trash / recycle bin are deleted forever after 30 days.">
Files in the trash for more than 30 days will be automatically deleted.
Expand All @@ -732,8 +738,17 @@
<message name="IDS_FILE_BROWSER_TRASH_UNEXPECTED_ERROR" desc="Message informing about error while sending an item to the trash / recycle bin.">
An error occurred. Some items may not have been trashed.
</message>
<message name="IDS_FILE_BROWSER_OPEN_TRASHED_FILES_ERROR" desc="Message informing users they can't open files from the Trash folder">
Cannot open trashed files. Please restore the files before opening.
<message name="IDS_FILE_BROWSER_OPEN_TRASHED_FILE_ERROR_TITLE" desc="Title of message informing users they can't open files from the Trash folder">
This item is in your trash
</message>
<message name="IDS_FILE_BROWSER_OPEN_TRASHED_FILE_ERROR_DESC" desc="Description of message informing users they can't open files from the Trash folder">
Restore the item or drag it to a new folder outside of trash
</message>
<message name="IDS_FILE_BROWSER_OPEN_TRASHED_FILES_ERROR_TITLE" desc="Title of message informing users they can't open files from the Trash folder">
These items are in your trash
</message>
<message name="IDS_FILE_BROWSER_OPEN_TRASHED_FILES_ERROR_DESC" desc="Description of message informing users they can't open files from the Trash folder">
Restore the items or drag them to a new folder outside of trash
</message>
<message name="IDS_FILE_BROWSER_ERROR_PROGRESS_SUMMARY_PLURAL" desc="Summary message for multiple errors above the progress bar. This message may be placed at the next to ohter *_PROGERSS_SUMMARY messages.">
<ph name="COUNT">$1<ex>10</ex></ph> errors.
Expand Down Expand Up @@ -820,20 +835,23 @@
Are you sure you want to delete <ph name="NUMBER_OF_ITEMS">$1<ex>3</ex></ph> items?
</message>
<message name="IDS_FILE_BROWSER_UNDO_DELETE_ONE" desc="Confirm to a user that they have deleted a single file and offer to undo">
<ph name="FILE_NAME">$1<ex>file.txt</ex></ph> moved to Trash
<ph name="FILE_NAME">$1<ex>file.txt</ex></ph> moved to trash
</message>
<message name="IDS_FILE_BROWSER_UNDO_DELETE_SOME" desc="Confirm to a user that they have deleted multiple files and offer to undo">
<ph name="NUMBER_OF_ITEMS">$1<ex>3</ex></ph> items moved to Trash
<ph name="NUMBER_OF_ITEMS">$1<ex>3</ex></ph> items moved to trash
</message>
<message name="IDS_FILE_BROWSER_MOVE_TO_TRASH_FILE_NAME" desc="Showing progress that a single item is being moved to trash.">
Moving <ph name="FILE_NAME">$1<ex>file.txt</ex></ph> to the Trash
Moving <ph name="FILE_NAME">$1<ex>file.txt</ex></ph> to the trash
</message>
<message name="IDS_FILE_BROWSER_MOVE_TO_TRASH_ITEMS_REMAINING" desc="Showing progress that multiple items are being moved to trash.">
Moving <ph name="NUMBER_OF_ITEMS">$1<ex>3</ex></ph> items to the Trash
Moving <ph name="NUMBER_OF_ITEMS">$1<ex>3</ex></ph> items to the trash
</message>
<message name="IDS_FILE_BROWSER_UNDO_DELETE_ACTION_LABEL" desc="Undo label shown on toast to undo delete operation">
Undo
</message>
<message name="IDS_FILE_BROWSER_RESTORE_ACTION_LABEL" desc="Restore label shown on dialog when trying to open files that are in Trash">
Restore
</message>
<message name="IDS_FILE_BROWSER_CONFIRM_EMPTY_TRASH_TITLE" desc="Asks the user if they are sure they want to empty the trash / recylce bin.">
Permanently delete these files?
</message>
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1e93bd4f6d64fbbfe3fca033736cf6433ad0468b
d2c61777bf8ca4bb12118daebfb303dcab9bfd12
Original file line number Diff line number Diff line change
@@ -1 +1 @@
c7825f63e488e8abc2585ed232a1e11a6e531bc8
e5e84753cb3702630f25554adb28fe74d8e9be9b
Original file line number Diff line number Diff line change
@@ -1 +1 @@
204fd5bdf103e24178f944ec3605ef6f3972e205
f4faee276c78bcf488904edcb1cf460c7fbba983

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0d6b84b64ff981da3fbd088d5f774442e2da2923
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0d6b84b64ff981da3fbd088d5f774442e2da2923
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
3da596f14cf51d4cdb976ea75a461910c3b2b3d6
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
3da596f14cf51d4cdb976ea75a461910c3b2b3d6
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
3da596f14cf51d4cdb976ea75a461910c3b2b3d6

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
a5f12b33ed95b78ec5a30188dc2b5398c32cf8aa
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
4cda8439d02251d47c4133d741549a14a3a0cce1
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
a5586690695ed5a74c6e74823a9f353bc17be1ce
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
7f32f01c56c298484169547cd353e41b7526aae2
Original file line number Diff line number Diff line change
@@ -1 +1 @@
017eced14b54edccdc9d245c616a353986add671
2627b3e6217cf6dbf43e7930ff1e043e95a3e553
Original file line number Diff line number Diff line change
@@ -1 +1 @@
7a22c169686c746a234ac85be8a154dfa51f4c3a
1c986b0881d8579ae646b7a84b79b0b577661087
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ export class FileOperationHandler {
return strf('DELETE_FILE_NAME', name);
case util.FileOperationType.RESTORE:
case util.FileOperationType.RESTORE_TO_DESTINATION:
return strf('RESTORE_FROM_TRASH_FILE_NAME', name);
return strf('RESTORING_FROM_TRASH_FILE_NAME', name);
default:
console.warn(
`Unexpected operation type: ${event.status.operationType}`);
Expand All @@ -460,7 +460,7 @@ export class FileOperationHandler {
return strf('DELETE_ITEMS_REMAINING', remainNumber);
case util.FileOperationType.RESTORE:
case util.FileOperationType.RESTORE_TO_DESTINATION:
return strf('RESTORE_FROM_TRASH_ITEMS_REMAINING', remainNumber);
return strf('RESTORING_FROM_TRASH_ITEMS_REMAINING', remainNumber);
default:
console.warn(
`Unexpected operation type: ${event.status.operationType}`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import {assertInstanceof} from 'chrome://resources/js/assert.js';

import {DialogType} from '../../common/js/dialog_type.js';
import {metrics} from '../../common/js/metrics.js';
import {TrashEntry} from '../../common/js/trash.js';
import {str, util} from '../../common/js/util.js';
import {VolumeManagerCommon} from '../../common/js/volume_manager_types.js';
import {DirectoryChangeEvent} from '../../externs/directory_change_event.js';
Expand All @@ -17,6 +20,7 @@ import {DirectoryModel} from './directory_model.js';
import {FileSelectionHandler} from './file_selection.js';
import {NamingController} from './naming_controller.js';
import {TaskController} from './task_controller.js';
import {Command} from './ui/command.js';
import {FileManagerUI} from './ui/file_manager_ui.js';
import {FileTapHandler} from './ui/file_tap_handler.js';
import {ListContainer} from './ui/list_container.js';
Expand Down Expand Up @@ -249,14 +253,13 @@ export class MainWindowComponent {
if (!listItem || !listItem.selected || selection.totalCount !== 1) {
return false;
}

const entry = selection.entries[0];
// A TrashEntry must have a key on it called `restoreEntry` and thus we use
// that as a signal this is a TrashEntry and should not be traversable.
if (entry.restoreEntry) {
this.ui_.alertDialog.show(str('OPEN_TRASHED_FILES_ERROR'), null, null);
const trashEntries = /** @type {!Array<!TrashEntry>} */ (
selection.entries.filter(util.isTrashEntry));
if (trashEntries.length > 0) {
this.showFailedToOpenTrashItemDialog_(trashEntries);
return false;
}
const entry = selection.entries[0];
if (entry.isDirectory) {
this.directoryModel_.changeDirectoryEntry(
/** @type {!DirectoryEntry} */ (entry));
Expand All @@ -277,8 +280,14 @@ export class MainWindowComponent {
// be restored first.
if (this.directoryModel_.getCurrentRootType() ===
VolumeManagerCommon.RootType.TRASH) {
this.ui_.alertDialog.show(str('OPEN_TRASHED_FILES_ERROR'), null, null);
return false;
const selection = this.selectionHandler_.selection;
if (!selection) {
return true;
}
const trashEntries = /** @type {!Array<!TrashEntry>} */ (
selection.entries.filter(util.isTrashEntry));
this.showFailedToOpenTrashItemDialog_(trashEntries);
return true;
}
this.taskController_.getFileTasks()
.then(tasks => {
Expand All @@ -300,6 +309,26 @@ export class MainWindowComponent {
return false;
}

/**
* Show a confirm dialog that shows whether the current selection can't be
* opened and offer to restore instead.
* @param {!Array<!TrashEntry>} trashEntries The current selection.
*/
showFailedToOpenTrashItemDialog_(trashEntries) {
let msgTitle = str('OPEN_TRASHED_FILE_ERROR_TITLE');
let msgDesc = str('OPEN_TRASHED_FILE_ERROR_DESC');
if (trashEntries.length > 1) {
msgTitle = str('OPEN_TRASHED_FILES_ERROR_TITLE');
msgDesc = str('OPEN_TRASHED_FILES_ERROR_DESC');
}
const restoreCommand = assertInstanceof(
document.getElementById('restore-from-trash'), Command);
this.ui_.restoreConfirmDialog.showWithTitle(msgTitle, msgDesc, () => {
restoreCommand.canExecuteChange(this.ui_.listContainer.currentList);
restoreCommand.execute(this.ui_.listContainer.currentList);
});
}

/**
* Handles click/keyup event on the toggle-view button.
* @param {Event} event Click or keyup event.
Expand Down Expand Up @@ -426,7 +455,8 @@ export class MainWindowComponent {
case 'Enter': // Enter => Change directory or perform default action.
const selection = this.selectionHandler_.selection;
if (selection.totalCount === 1 && selection.entries[0].isDirectory &&
!DialogType.isFolderDialog(this.dialogType_)) {
!DialogType.isFolderDialog(this.dialogType_) &&
!selection.entries.some(util.isTrashEntry)) {
const item = this.ui_.listContainer.currentList.getListItemByIndex(
selection.indexes[0]);
// If the item is in renaming process, we don't allow to change
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,14 @@ export class FileManagerUI {
this.emptyTrashConfirmDialog = new FilesConfirmDialog(this.element);
this.emptyTrashConfirmDialog.setOkLabel(str('EMPTY_TRASH_DELETE_FOREVER'));

/**
* Restore dialog when trying to open files that are in the trash
* @type {!FilesConfirmDialog}
* @const
*/
this.restoreConfirmDialog = new FilesConfirmDialog(this.element);
this.restoreConfirmDialog.setOkLabel(str('RESTORE_ACTION_LABEL'));

/**
* Confirm dialog for file move operation.
* @type {!FilesConfirmDialog}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export class ProgressCenterPanel {
}
if (item.type === ProgressItemType.RESTORE_TO_DESTINATION ||
item.type === ProgressItemType.RESTORE) {
return strf('RESTORE_FROM_TRASH_FILE_NAME', source);
return strf('RESTORING_FROM_TRASH_FILE_NAME', source);
}
return item.message;
}
Expand All @@ -106,7 +106,7 @@ export class ProgressCenterPanel {
}
if (item.type === ProgressItemType.RESTORE_TO_DESTINATION ||
item.type === ProgressItemType.RESTORE) {
return strf('RESTORE_FROM_TRASH_ITEMS_REMAINING', count);
return strf('RESTORING_FROM_TRASH_ITEMS_REMAINING', count);
}
return item.message;
break;
Expand Down Expand Up @@ -188,7 +188,9 @@ export class ProgressCenterPanel {
}
if (item.type === ProgressItemType.RESTORE_TO_DESTINATION ||
item.type === ProgressItemType.RESTORE) {
return strf('RESTORE_FROM_TRASH_FILE_NAME', source);
return item.state == ProgressItemState.PROGRESSING ?
strf('RESTORING_FROM_TRASH_FILE_NAME', source) :
strf('RESTORE_TRASH_FILE_NAME', source);
}
return item.message;
}
Expand Down Expand Up @@ -222,7 +224,9 @@ export class ProgressCenterPanel {
}
if (item.type === ProgressItemType.RESTORE_TO_DESTINATION ||
item.type === ProgressItemType.RESTORE) {
return strf('RESTORE_FROM_TRASH_ITEMS_REMAINING', count);
return item.state == ProgressItemState.PROGRESSING ?
strf('RESTORING_FROM_TRASH_ITEMS_REMAINING', count) :
strf('RESTORE_TRASH_MANY_ITEMS', count);
}
return item.message;
break;
Expand Down Expand Up @@ -267,7 +271,7 @@ export class ProgressCenterPanel {
// Return empty string for invalid remaining time in non progressing
// state.
return item.state === ProgressItemState.PROGRESSING ?
str('PENDING_LABEL') :
str('PREPARING_LABEL') :
'';
}

Expand Down
Loading

0 comments on commit f528300

Please sign in to comment.