Skip to content

Commit

Permalink
Remove Team Drive root from directory tree instead of hiding
Browse files Browse the repository at this point in the history
Remove Team Drive root from the directory tree when user has no team
drive. Fixes a bug where users could navigate to Team Drive root via
the keyboard even though the root was hidden.

Bug: 864890
Change-Id: I2f6b59ca908ae609302cb873b6b182842cedc1c4
Reviewed-on: https://chromium-review.googlesource.com/1163352
Commit-Queue: Noel Gordon <noel@chromium.org>
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581114}
  • Loading branch information
Luciano Pacheco authored and Commit Bot committed Aug 7, 2018
1 parent d00fd14 commit ef43152
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 48 deletions.
116 changes: 81 additions & 35 deletions ui/file_manager/file_manager/foreground/js/ui/directory_tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -977,27 +977,68 @@ DriveVolumeItem.prototype.handleClick = function(e) {
};

/**
* Sets the hidden state of the given item depending on whether the user has any
* Team Drives.
* Creates Team Drives root if there is any team drive, if there is no team
* drive, then it removes the root.
*
* Since we don't currently support any functionality with just the grand root
* (e.g. you can't create a new team drive from the root yet), hide the grand
* root unless the user has at least one Team Drive. If there is at least one
* Team Drive, show it.
* (e.g. you can't create a new team drive from the root yet), remove/don't
* create the grand root so it can't be reached via keyboard.
* If there is at least one Team Drive, add/show the Team Drives trand root.
*
* @param {!DirectoryItem} teamDrivesGrandRootItem The item to show if there is
* at least one Team Drive, or hide if not.
* @return {!Promise<SubDirectoryItem>} Resolved with Team Drive Grand Root
* SubDirectoryItem instance, or undefined when it shouldn't exist.
* @private
*/
DriveVolumeItem.prototype.setHiddenForTeamDrivesGrandRoot_ = function(
teamDrivesGrandRootItem) {
var teamDriveEntry = this.volumeInfo_.teamDriveDisplayRoot;
if (!teamDriveEntry)
return;
var reader = teamDriveEntry.createReader();
reader.readEntries(function(results) {
metrics.recordSmallCount('TeamDrivesCount', results.length);
teamDrivesGrandRootItem.hidden = results.length == 0;
DriveVolumeItem.prototype.createTeamDrivesGrandRoot_ = function() {
return new Promise(resolve => {
const teamDriveGrandRoot = this.volumeInfo_.teamDriveDisplayRoot;
if (!teamDriveGrandRoot) {
// Team Drive is disabled.
resolve();
return;
}

let index;
for (var i = 0; i < this.items.length; i++) {
const entry = this.items[i] && this.items[i].entry;
if (entry && util.isSameEntry(entry, teamDriveGrandRoot)) {
index = i;
break;
}
}

const reader = teamDriveGrandRoot.createReader();
reader.readEntries(results => {
metrics.recordSmallCount('TeamDrivesCount', results.length);
// Only create grand root if there is at least 1 child/result.
if (results.length) {
if (index) {
this.items[index].hidden = false;
resolve(this.items[index]);
return;
}

// Create if it doesn't exist yet.
const label = util.getEntryLabel(
this.parentTree_.volumeManager_.getLocationInfo(
teamDriveGrandRoot),
teamDriveGrandRoot) ||
'';
const item = new SubDirectoryItem(
label, teamDriveGrandRoot, this, this.parentTree_);
this.addAt(item, 1);
item.updateSubDirectories(false);
resolve(item);
return;
} else {
// When there is no team drive, the grand root should be removed.
if (index && this.items[index].parentItem) {
this.items[index].parentItem.remove(this.items[index]);
}
resolve();
return;
}
});
});
};

Expand Down Expand Up @@ -1032,21 +1073,19 @@ DriveVolumeItem.prototype.updateSubDirectories = function(recursive) {
}

for (var i = 0; i < entries.length; i++) {
var item = new SubDirectoryItem(
util.getEntryLabel(
this.parentTree_.volumeManager_.getLocationInfo(entries[i]),
entries[i]) ||
'',
entries[i], this, this.parentTree_);

// Hide the team drives root in case we have no team drives.
if (entries[i] === teamDrivesDisplayRoot) {
item.hidden = true;
this.setHiddenForTeamDrivesGrandRoot_(item);
// Only create the team drives root if there is at least 1 team drive.
const entry = entries[i];
if (entry === teamDrivesDisplayRoot) {
this.createTeamDrivesGrandRoot_();
} else {
const label =
util.getEntryLabel(
this.parentTree_.volumeManager_.getLocationInfo(entry), entry) ||
'';
const item = new SubDirectoryItem(label, entry, this, this.parentTree_);
this.add(item);
item.updateSubDirectories(false);
}

this.add(item);
item.updateSubDirectories(false);
}
// When My files is disabled Drive should be expanded by default.
// TODO(crbug.com/850348): Remove this once flag is removed.
Expand All @@ -1065,11 +1104,18 @@ DriveVolumeItem.prototype.updateSubDirectories = function(recursive) {
DriveVolumeItem.prototype.updateItemByEntry = function(changedDirectoryEntry) {
// The first item is My Drive, and the second item is Team Drives.
// Keep in sync with |fixedEntries| in |updateSubDirectories|.
var index = util.isTeamDriveEntry(changedDirectoryEntry) ? 1 : 0;
this.items[index].updateItemByEntry(changedDirectoryEntry);
if (util.isTeamDrivesGrandRoot(changedDirectoryEntry) &&
this.volumeInfo_.teamDriveDisplayRoot) {
this.setHiddenForTeamDrivesGrandRoot_(this.items[index]);
const isTeamDriveChild = util.isTeamDriveEntry(changedDirectoryEntry);
const index = isTeamDriveChild ? 1 : 0;

// If Team Drive grand root has been removed and we receive an update for an
// team drive, we need to create the Team Drive grand root.
if (isTeamDriveChild) {
this.createTeamDrivesGrandRoot_().then(teamDriveGranRootItem => {
if (teamDriveGranRootItem)
this.items[index].updateItemByEntry(changedDirectoryEntry);
});
} else {
this.items[index].updateItemByEntry(changedDirectoryEntry);
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
<script src="../mock_directory_model.js"></script>
<script src="../mock_navigation_list_model.js"></script>
<script src="../navigation_list_model.js"></script>
<script src="../constants.js"></script>
<script src="directory_tree.js"></script>

<script src="directory_tree_unittest.js"></script>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ function testCreateDirectoryTree(callback) {
*
* Google Drive
* - My Drive
* - Team Drives
* - Team Drives (only if there is a child team drive).
* - Shared with me
* - Offline
* Downloads
Expand Down Expand Up @@ -194,8 +194,7 @@ function testCreateDirectoryTreeWithTeamDrive(callback) {

/**
* Test case for creating tree with empty Team Drives.
* Team Drives subtree should be hidden when the user don't have access to any
* Team Drive.
* The Team Drives subtree should be removed if the user has no team drives.
*
* @param {!function(boolean)} callback A callback function which is called with
* test result.
Expand Down Expand Up @@ -238,21 +237,19 @@ function testCreateDirectoryTreeWithEmptyTeamDrive(callback) {

reportPromise(
waitUntil(function() {
// Root entries under Drive volume is generated, plus Team Drives.
// Root entries under Drive volume is generated, Team Drives isn't
// included because it has no child.
// See testCreateDirectoryTreeWithTeamDrive for detail.
return driveItem.items.length == 4;
return driveItem.items.length == 3;
}).then(function() {
var teamDrivesItemFound = false;
for (var i = 0; i < driveItem.items.length; i++) {
if (driveItem.items[i].label == str('DRIVE_TEAM_DRIVES_LABEL')) {
assertEquals(
true, driveItem.items[i].hidden,
'Team Drives node should not be shown');
teamDrivesItemFound = true;
break;
}
}
assertTrue(teamDrivesItemFound, 'Team Drives node should be generated');
assertFalse(teamDrivesItemFound, 'Team Drives should NOT be generated');
}),
callback);
}
Expand Down Expand Up @@ -403,7 +400,7 @@ function testAddFirstTeamDrive(callback) {

reportPromise(
waitUntil(() => {
return driveItem.items.length == 4;
return driveItem.items.length == 3;
})
.then(() => {
window.webkitResolveLocalFileSystemURLEntries
Expand Down Expand Up @@ -434,7 +431,7 @@ function testAddFirstTeamDrive(callback) {

/**
* Test removing the last team drive for a user.
* Team Drives subtree should be hidden after the change notification is
* Team Drives subtree should be removed after the change notification is
* delivered.
*
* @param {!function(boolean)} callback A callback function which is called with
Expand Down Expand Up @@ -501,14 +498,15 @@ function testRemoveLastTeamDrive(callback) {
}
})
.then(() => {
// Wait team drive grand root to appear.
return waitUntil(() => {
for (var i = 0; i < driveItem.items.length; i++) {
if (driveItem.items[i].label ==
str('DRIVE_TEAM_DRIVES_LABEL')) {
return driveItem.items[i].hidden;
return false;
}
}
return false;
return true;
});
}),
callback);
Expand Down

0 comments on commit ef43152

Please sign in to comment.