Commit 65d577fb authored by Luciano Pacheco's avatar Luciano Pacheco Committed by Commit Bot

Fix extra divider line in save as dialog

Change directory tree to the first item's section instead of static TOP
section to determine if the first section divider should appear.

Change the value of section-start attribute to be the current section
which is the correct value, instead of previous section.

While here modernized this function to use const/let instead of var.

Bug: 878531
Test: browsertest --gtest_filter='FileManagerJsTest.DirectoryTree*'
Change-Id: If8db170594608fef04913f3f6fe8235764eb03f7
Reviewed-on: https://chromium-review.googlesource.com/1248066
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Reviewed-by: default avatarSam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594604}
parent 2c3dd0ea
...@@ -1713,9 +1713,9 @@ DirectoryTree.prototype.updateAndSelectNewDirectory = function( ...@@ -1713,9 +1713,9 @@ DirectoryTree.prototype.updateAndSelectNewDirectory = function(
DirectoryTree.prototype.updateSubElementsFromList = function(recursive) { DirectoryTree.prototype.updateSubElementsFromList = function(recursive) {
// First, current items which is not included in the dataModel should be // First, current items which is not included in the dataModel should be
// removed. // removed.
for (var i = 0; i < this.items.length;) { for (let i = 0; i < this.items.length;) {
var found = false; let found = false;
for (var j = 0; j < this.dataModel.length; j++) { for (let j = 0; j < this.dataModel.length; j++) {
// Comparison by references, which is safe here, as model items are long // Comparison by references, which is safe here, as model items are long
// living. // living.
if (this.items[i].modelItem === this.dataModel.item(j)) { if (this.items[i].modelItem === this.dataModel.item(j)) {
...@@ -1733,18 +1733,20 @@ DirectoryTree.prototype.updateSubElementsFromList = function(recursive) { ...@@ -1733,18 +1733,20 @@ DirectoryTree.prototype.updateSubElementsFromList = function(recursive) {
} }
// Next, insert items which is in dataModel but not in current items. // Next, insert items which is in dataModel but not in current items.
var modelIndex = 0; let modelIndex = 0;
var itemIndex = 0; let itemIndex = 0;
// Starts with TOP_SECTION so first section doesn't get the separator line. // Initialize with first item's section so the first root doesn't get a
var previousSection = NavigationSection.TOP; // divider line at the top.
let previousSection = this.dataModel.item(modelIndex).section;
while (modelIndex < this.dataModel.length) { while (modelIndex < this.dataModel.length) {
const currentItem = this.items[itemIndex]; const currentItem = this.items[itemIndex];
if (itemIndex < this.items.length && if (itemIndex < this.items.length &&
currentItem.modelItem === this.dataModel.item(modelIndex)) { currentItem.modelItem === this.dataModel.item(modelIndex)) {
var modelItem = currentItem.modelItem; const modelItem = currentItem.modelItem;
if (previousSection !== modelItem.section) if (previousSection !== modelItem.section) {
currentItem.setAttribute('section-start', previousSection); currentItem.setAttribute('section-start', modelItem.section);
previousSection = modelItem.section; previousSection = modelItem.section;
}
if (recursive && currentItem instanceof VolumeItem) if (recursive && currentItem instanceof VolumeItem)
currentItem.updateSubDirectories(true); currentItem.updateSubDirectories(true);
// EntryListItem can contain volumes that might have been updated: ask // EntryListItem can contain volumes that might have been updated: ask
...@@ -1753,13 +1755,13 @@ DirectoryTree.prototype.updateSubElementsFromList = function(recursive) { ...@@ -1753,13 +1755,13 @@ DirectoryTree.prototype.updateSubElementsFromList = function(recursive) {
if (currentItem instanceof EntryListItem) if (currentItem instanceof EntryListItem)
currentItem.updateSubDirectories(true); currentItem.updateSubDirectories(true);
} else { } else {
var modelItem = this.dataModel.item(modelIndex); const modelItem = this.dataModel.item(modelIndex);
if (modelItem) { if (modelItem) {
var item = DirectoryTree.createDirectoryItem(modelItem, this); const item = DirectoryTree.createDirectoryItem(modelItem, this);
if (item) { if (item) {
this.addAt(item, itemIndex); this.addAt(item, itemIndex);
if (previousSection !== modelItem.section) if (previousSection !== modelItem.section)
item.setAttribute('section-start', previousSection); item.setAttribute('section-start', modelItem.section);
} }
previousSection = modelItem.section; previousSection = modelItem.section;
} }
......
...@@ -12,6 +12,8 @@ ...@@ -12,6 +12,8 @@
<script src="../../../../../../ui/webui/resources/js/cr/ui/array_data_model.js"></script> <script src="../../../../../../ui/webui/resources/js/cr/ui/array_data_model.js"></script>
<script src="../../../../../../ui/webui/resources/js/cr/ui/tree.js"></script> <script src="../../../../../../ui/webui/resources/js/cr/ui/tree.js"></script>
<script src="../../../../../../ui/webui/resources/js/load_time_data.js"></script> <script src="../../../../../../ui/webui/resources/js/load_time_data.js"></script>
<script src="../../../../../../ui/webui/resources/js/assert.js"></script>
<script src="../../../../../../ui/webui/resources/js/util.js"></script>
<script src="../../../common/js/async_util.js"></script> <script src="../../../common/js/async_util.js"></script>
<script src="../../../common/js/mock_entry.js"></script> <script src="../../../common/js/mock_entry.js"></script>
...@@ -29,6 +31,7 @@ ...@@ -29,6 +31,7 @@
<script src="../../../background/js/mock_volume_manager.js"></script> <script src="../../../background/js/mock_volume_manager.js"></script>
<script src="../mock_directory_model.js"></script> <script src="../mock_directory_model.js"></script>
<script src="../mock_navigation_list_model.js"></script> <script src="../mock_navigation_list_model.js"></script>
<script src="../mock_folder_shortcut_data_model.js"></script>
<script src="../navigation_list_model.js"></script> <script src="../navigation_list_model.js"></script>
<script src="../constants.js"></script> <script src="../constants.js"></script>
<script src="directory_tree.js"></script> <script src="directory_tree.js"></script>
......
...@@ -19,7 +19,8 @@ loadTimeData.data = { ...@@ -19,7 +19,8 @@ loadTimeData.data = {
DRIVE_OFFLINE_COLLECTION_LABEL: 'Offline', DRIVE_OFFLINE_COLLECTION_LABEL: 'Offline',
DRIVE_SHARED_WITH_ME_COLLECTION_LABEL: 'Shared with me', DRIVE_SHARED_WITH_ME_COLLECTION_LABEL: 'Shared with me',
REMOVABLE_DIRECTORY_LABEL: 'External Storage', REMOVABLE_DIRECTORY_LABEL: 'External Storage',
ARCHIVE_DIRECTORY_LABEL: 'Archives' ARCHIVE_DIRECTORY_LABEL: 'Archives',
MY_FILES_ROOT_LABEL: 'My files',
}; };
function setUp() { function setUp() {
...@@ -261,6 +262,64 @@ function testCreateDirectoryTreeWithEmptyTeamDrive(callback) { ...@@ -261,6 +262,64 @@ function testCreateDirectoryTreeWithEmptyTeamDrive(callback) {
callback); callback);
} }
/**
* Test case for updateSubElementsFromList setting section-start attribute.
*
* 'section-start' attribute is used to display a line divider between
* "sections" in the directory tree. This is calculated in NavigationListModel.
*/
function testUpdateSubElementsFromListSections() {
// Creates elements.
const parentElement = document.createElement('div');
const directoryTree = document.createElement('div');
parentElement.appendChild(directoryTree);
// Creates mocks.
const directoryModel = new MockDirectoryModel();
const volumeManager = new MockVolumeManager();
const fileOperationManager = {
addEventListener: function(name, callback) {}
};
const treeModel = new NavigationListModel(
volumeManager,
new MockFolderShortcutDataModel([]),
null, /* recentItem */
null, /* addNewServicesItem */
false /* opt_disableMyFilesNavigation */
);
const myFilesItem = treeModel.item(0);
const driveItem = treeModel.item(1);
assertEquals(NavigationSection.MY_FILES, myFilesItem.section);
assertEquals(NavigationSection.CLOUD, driveItem.section);
DirectoryTree.decorate(directoryTree, directoryModel, volumeManager,
null, fileOperationManager, true);
directoryTree.dataModel = treeModel;
directoryTree.updateSubElementsFromList(false);
// First element should not have section-start attribute, to not display a
// division line in the first section.
// My files:
assertEquals(null, directoryTree.items[0].getAttribute('section-start'));
// Drive should have section-start, because it's a new section but not the
// first section.
assertEquals(
NavigationSection.CLOUD,
directoryTree.items[1].getAttribute('section-start'));
// Regenerate so it re-calculates the 'section-start' without creating the
// DirectoryItem.
directoryTree.updateSubElementsFromList(false);
assertEquals(
NavigationSection.CLOUD,
directoryTree.items[1].getAttribute('section-start'));
}
/** /**
* Test case for updateSubElementsFromList. * Test case for updateSubElementsFromList.
* *
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment