[Files app] Refactor copy/cut commands
Reverse the dependency from FileTransferController to
FileManagerCommands, because:
1. Some logic was duplicated from FileManagerCommands. in
FileTransferController, reversing allows to remove those dupes.
2. FileTransferController was mutating commands enable and visible
statuses, which was hard to detect/debug. Now all command enable/
visibility is in FileManagerCommands.
3. Better detect the entries from the event target in the same way used
in FileManagerCommands.
There is no change in behaviour for users, so this is covered by
current tests.
Bug: 889153
Change-Id: I077d2c89f585876e5de39e2f1b0149a411177907
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1571266
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Reviewed-by: Stuart Langley <slangley@chromium.org>
Auto-Submit: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#655107}
diff --git a/ui/file_manager/file_manager/foreground/js/file_manager.js b/ui/file_manager/file_manager/foreground/js/file_manager.js
index c7277df..22258d5 100644
--- a/ui/file_manager/file_manager/foreground/js/file_manager.js
+++ b/ui/file_manager/file_manager/foreground/js/file_manager.js
@@ -673,9 +673,7 @@
assert(this.fileBrowserBackground_.progressCenter),
assert(this.fileOperationManager_), assert(this.metadataModel_),
assert(this.thumbnailModel_), assert(this.directoryModel_),
- assert(this.volumeManager_), assert(this.selectionHandler_),
- CommandUtil.shouldShowMenuItemsForEntry.bind(
- null, assert(this.volumeManager_)));
+ assert(this.volumeManager_), assert(this.selectionHandler_));
};
/**
diff --git a/ui/file_manager/file_manager/foreground/js/file_manager_commands.js b/ui/file_manager/file_manager/foreground/js/file_manager_commands.js
index c1c9610c..b241f7e4 100644
--- a/ui/file_manager/file_manager/foreground/js/file_manager_commands.js
+++ b/ui/file_manager/file_manager/foreground/js/file_manager_commands.js
@@ -326,6 +326,24 @@
};
/**
+ * Checks if the handler should ignore the current event, eg. since there is
+ * a popup dialog currently opened.
+ *
+ * @param {!Document} doc
+ * @return {boolean} True if the event should be ignored, false otherwise.
+ */
+CommandUtil.shouldIgnoreEvents = function(doc) {
+ // Do not handle commands, when a dialog is shown. Do not use querySelector
+ // as it's much slower, and this method is executed often.
+ const dialogs = doc.getElementsByClassName('cr-dialog-container');
+ if (dialogs.length !== 0 && dialogs[0].classList.contains('shown')) {
+ return true;
+ }
+
+ return false; // Do not ignore.
+};
+
+/**
* Handle of the command events.
* @param {!CommandHandlerDeps} fileManager Classes |CommandHalder| depends.
* @param {!FileSelectionHandler} selectionHandler
@@ -477,31 +495,12 @@
};
/**
- * Checks if the handler should ignore the current event, eg. since there is
- * a popup dialog currently opened.
- *
- * @return {boolean} True if the event should be ignored, false otherwise.
- * @private
- */
-CommandHandler.prototype.shouldIgnoreEvents_ = function() {
- // Do not handle commands, when a dialog is shown. Do not use querySelector
- // as it's much slower, and this method is executed often.
- const dialogs =
- this.fileManager_.document.getElementsByClassName('cr-dialog-container');
- if (dialogs.length !== 0 && dialogs[0].classList.contains('shown')) {
- return true;
- }
-
- return false; // Do not ignore.
-};
-
-/**
* Handles command events.
* @param {!Event} event Command event.
* @private
*/
CommandHandler.prototype.onCommand_ = function(event) {
- if (this.shouldIgnoreEvents_()) {
+ if (CommandUtil.shouldIgnoreEvents(assert(this.fileManager_.document))) {
return;
}
const handler = CommandHandler.COMMANDS_[event.command.id];
@@ -515,7 +514,7 @@
* @private
*/
CommandHandler.prototype.onCanExecute_ = function(event) {
- if (this.shouldIgnoreEvents_()) {
+ if (CommandUtil.shouldIgnoreEvents(assert(this.fileManager_.document))) {
return;
}
const handler = CommandHandler.COMMANDS_[event.command.id];
@@ -1176,8 +1175,82 @@
* @param {!CommandHandlerDeps} fileManager CommandHandlerDeps to use.
*/
canExecute: function(event, fileManager) {
- event.canExecute =
- fileManager.document.queryCommandEnabled(event.command.id);
+ const command = event.command;
+ const target = event.target;
+ const isMove = command.id === 'cut';
+ const volumeManager = fileManager.volumeManager;
+ command.setHidden(false);
+
+ /** @returns {boolean} If the operation is allowed in the Directory Tree. */
+ function canDoDirectoryTree() {
+ let entry;
+ if (target.entry) {
+ entry = target.entry;
+ } else if (target.selectedItem && target.selectedItem.entry) {
+ entry = target.selectedItem.entry;
+ } else {
+ return false;
+ }
+
+ if (!CommandUtil.shouldShowMenuItemsForEntry(volumeManager, entry)) {
+ command.setHidden(true);
+ return false;
+ }
+
+ // For MyFiles/Downloads we only allow copy.
+ if (isMove && CommandUtil.isDownloads(volumeManager, entry)) {
+ return false;
+ }
+
+ // Cut is unavailable on Shared Drive roots.
+ if (util.isTeamDriveRoot(entry)) {
+ return false;
+ }
+
+ const metadata =
+ fileManager.metadataModel.getCache([entry], ['canCopy', 'canDelete']);
+ assert(metadata.length === 1);
+
+ if (!isMove) {
+ return metadata[0].canCopy !== false;
+ }
+
+ // We need to check source volume is writable for move operation.
+ const volumeInfo = volumeManager.getVolumeInfo(entry);
+ return !volumeInfo.isReadOnly && metadata[0].canCopy !== false &&
+ metadata[0].canDelete !== false;
+ }
+
+ /** @returns {boolean} If the operation is allowed in the File List. */
+ function canDoFileList() {
+ if (CommandUtil.shouldIgnoreEvents(assert(fileManager.document))) {
+ return false;
+ }
+ if (!fileManager.getSelection().entries.every(
+ CommandUtil.shouldShowMenuItemsForEntry.bind(
+ null, volumeManager))) {
+ command.setHidden(true);
+ return false;
+ }
+
+ // For MyFiles/Downloads we only allow copy.
+ if (isMove &&
+ fileManager.getSelection().entries.some(
+ CommandUtil.isDownloads.bind(null, volumeManager))) {
+ return false;
+ }
+
+ const fileTransferController = fileManager.fileTransferController;
+ return isMove ? fileTransferController.canCutOrDrag() :
+ fileTransferController.canCopyOrDrag();
+ }
+
+ const canDo =
+ fileManager.ui.directoryTree.contains(/** @type {Node} */ (target)) ?
+ canDoDirectoryTree() :
+ canDoFileList();
+ event.canExecute = canDo;
+ command.disabled = !canDo;
}
});
diff --git a/ui/file_manager/file_manager/foreground/js/file_transfer_controller.js b/ui/file_manager/file_manager/foreground/js/file_transfer_controller.js
index 51d9afe..04fdde0 100644
--- a/ui/file_manager/file_manager/foreground/js/file_transfer_controller.js
+++ b/ui/file_manager/file_manager/foreground/js/file_transfer_controller.js
@@ -32,15 +32,13 @@
* @param {!DirectoryModel} directoryModel Directory model instance.
* @param {!VolumeManager} volumeManager Volume manager instance.
* @param {!FileSelectionHandler} selectionHandler Selection handler.
- * @param {function((!Entry|!FakeEntry)): boolean} shouldShowCommandFor
* @struct
* @constructor
*/
function FileTransferController(
doc, listContainer, directoryTree, multiProfileShareDialog,
confirmationCallback, progressCenter, fileOperationManager, metadataModel,
- thumbnailModel, directoryModel, volumeManager, selectionHandler,
- shouldShowCommandFor) {
+ thumbnailModel, directoryModel, volumeManager, selectionHandler) {
/**
* @private {!Document}
* @const
@@ -109,12 +107,6 @@
this.progressCenter_ = progressCenter;
/**
- * @private {function((!Entry|!FakeEntry)): boolean}
- * @const
- */
- this.shouldShowCommandFor_ = shouldShowCommandFor;
-
- /**
* The array of pending task ID.
* @type {Array<string>}
*/
@@ -1051,8 +1043,8 @@
}
const dt = event.dataTransfer;
- const canCopy = this.canCopyOrDrag_();
- const canCut = this.canCutOrDrag_();
+ const canCopy = this.canCopyOrDrag();
+ const canCut = this.canCutOrDrag();
if (canCopy || canCut) {
if (canCopy && canCut) {
this.cutOrCopy_(dt, 'all');
@@ -1321,19 +1313,6 @@
};
/**
- * @return {boolean} Returns true if there is a dialog showing that we should
- * treat as modal, false otherwise.
- * @private
- */
-FileTransferController.prototype.isModalDialogBeingDisplayed_ = () => {
- // Do not allow Cut or Copy if a modal dialog is being displayed.
- if (document.querySelector('.cr-dialog-container.shown') !== null) {
- return true;
- }
- return false;
-};
-
-/**
* @param {boolean} isMove True for move operation.
* @param {!Event} event
* @private
@@ -1415,68 +1394,16 @@
*/
FileTransferController.prototype.canCutOrCopy_ = function(isMove) {
const command = isMove ? this.cutCommand_ : this.copyCommand_;
- command.setHidden(false);
-
- if (document.activeElement instanceof DirectoryTree) {
- const selectedItem = document.activeElement.selectedItem;
- if (!selectedItem) {
- return false;
- }
- const entry = selectedItem.entry;
-
- if (!this.shouldShowCommandFor_(entry)) {
- command.setHidden(true);
- return false;
- }
-
- // For MyFiles/Downloads we only allow copy.
- if (isMove && this.isDownloads_(entry)) {
- return false;
- }
-
- // Cut is unavailable on Shared Drive roots.
- if (util.isTeamDriveRoot(entry)) {
- return false;
- }
-
- const metadata =
- this.metadataModel_.getCache([entry], ['canCopy', 'canDelete']);
- assert(metadata.length === 1);
-
- if (!isMove) {
- return metadata[0].canCopy !== false;
- }
-
- // We need to check source volume is writable for move operation.
- const volumeInfo = this.volumeManager_.getVolumeInfo(entry);
- return !volumeInfo.isReadOnly && metadata[0].canCopy !== false &&
- metadata[0].canDelete !== false;
- }
-
- if (this.isModalDialogBeingDisplayed_()) {
- return false;
- }
- if (!this.selectionHandler_.selection.entries.every(
- this.shouldShowCommandFor_)) {
- command.setHidden(true);
- return false;
- }
-
- // For MyFiles/Downloads we only allow copy.
- if (isMove &&
- this.selectionHandler_.selection.entries.some(this.isDownloads_, this)) {
- return false;
- }
-
- return isMove ? this.canCutOrDrag_() : this.canCopyOrDrag_();
+ command.canExecuteChange(this.document_.activeElement);
+ return !command.disabled;
};
/**
* @return {boolean} Returns true if some files are selected and all the file
* on drive is available to be copied. Otherwise, returns false.
- * @private
+ * @public
*/
-FileTransferController.prototype.canCopyOrDrag_ = function() {
+FileTransferController.prototype.canCopyOrDrag = function() {
if (!this.selectionHandler_.isAvailable()) {
return false;
}
@@ -1502,9 +1429,9 @@
/**
* @return {boolean} Returns true if the current directory is not read only.
- * @private
+ * @public
*/
-FileTransferController.prototype.canCutOrDrag_ = function() {
+FileTransferController.prototype.canCutOrDrag = function() {
if (this.directoryModel_.isReadOnly() ||
!this.selectionHandler_.isAvailable() ||
this.selectionHandler_.selection.entries.length <= 0) {
@@ -1829,26 +1756,3 @@
}
}, 100);
};
-
-/**
- * Returns True if entry is MyFiles>Downloads.
- * @param {(!Entry|!FakeEntry)} entry Entry or a fake entry.
- * @return {boolean}
- */
-FileTransferController.prototype.isDownloads_ = function(entry) {
- if (util.isFakeEntry(entry)) {
- return false;
- }
-
- const volumeInfo = this.volumeManager_.getVolumeInfo(entry);
- if (!volumeInfo) {
- return false;
- }
-
- if (util.isMyFilesVolumeEnabled() &&
- volumeInfo.volumeType === VolumeManagerCommon.RootType.DOWNLOADS &&
- entry.fullPath === '/Downloads') {
- return true;
- }
- return false;
-};
