Remove horizontal scrollbar on directory tree

CL:1599817 added horizontal scrolling to the directory tree, then
CL:1617041 was required to help with positioning of the eject button for
ejectable tree entries, then CL:1631535 fixed issues with positioning of
Android app picker link caused by the scroll, then CL:1623649 fixed
issues with the Polymer ripples being contained by the changes in use of
CSS 'position', then CL:1710706, CL:1712350 and CL:1714391 corrected RTL
rendering of the tree that was broken by the scroll change.

UX review agrees that horizontal scrolling is unwanted. Undo the CSS/JS
changes in the above set of CLs.

Re-enable ripple effect on ejectable items in the tree by removing the
use of 'visibility:hidden' on them.

Bug: 660334, 992819, 965870
Tests: browser_tests --gtest_filter="DirectoryTree*Scroll*"
Change-Id: I97a12c8da81f3e12040ec193f365c6ac944a4ddd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1767456
Commit-Queue: Alex Danilo <adanilo@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#693030}

chrome/browser/chromeos/file_manager/file_manager_browsertest.cc

@@ -523,6 +523,12 @@ WRAPPED_INSTANTIATE_TEST_SUITE_P(
         TestCase("cantOpenQuickViewWithMultipleFiles"),
         TestCase("openQuickViewFromDirectoryTree")));
 
+WRAPPED_INSTANTIATE_TEST_SUITE_P(
+    DirectoryTree, /* directory_tree.js */
+    FilesAppBrowserTest,
+    ::testing::Values(TestCase("directoryTreeHorizontalScroll"),
+                      TestCase("directoryTreeVerticalScroll")));
+
 WRAPPED_INSTANTIATE_TEST_SUITE_P(
     DirectoryTreeContextMenu, /* directory_tree_context_menu.js */
     FilesAppBrowserTest,
@@ -944,12 +950,6 @@ WRAPPED_INSTANTIATE_TEST_SUITE_P(
         TestCase("metadataLargeDrive").DisableDriveFs(),
         TestCase("metadataLargeDrive").EnableDriveFs()));
 
-WRAPPED_INSTANTIATE_TEST_SUITE_P(
-    NavigationList, /* navigation_list.js */
-    FilesAppBrowserTest,
-    ::testing::Values(TestCase("navigationListHorizontalScroll"),
-                      TestCase("navigationListVerticalScroll")));
-
 WRAPPED_INSTANTIATE_TEST_SUITE_P(
     Search, /* search.js */
     FilesAppBrowserTest,

ui/file_manager/file_manager/foreground/css/file_manager.css

@@ -100,6 +100,7 @@ a:focus {
   display: flex;
   flex: 1 1 auto;
   margin-top: 8px;
+  position: relative;
 }
 
 .dialog-navigation-list-footer {
@@ -124,7 +125,11 @@ div.splitter {
   bottom: 0;
   flex: none;
   left: 0;
-  min-width: 100%;
+  overflow-x: hidden;
+  overflow-y: auto;
+  position: absolute;   /* TODO(adanilo): crbug.com/212268 still needed? */
+  right: 0;
+  top: 0;
 }
 
 #directory-tree .tree-row {
@@ -161,17 +166,6 @@ div.splitter {
   text-overflow: ellipsis;
 }
 
-#directory-tree .tree-item > .tree-row.ejectable {
-  position: inherit;
-}
-
-#directory-tree .tree-row.ejectable > .label {
-  left: 48px;
-  position: absolute;
-  right: 48px;
-  width: calc(100% - 48px - 40px - 3px);
-}
-
 #directory-tree .tree-row .rename-placeholder {
   display: block;
   flex: auto;
@@ -223,9 +217,7 @@ div.splitter {
   cursor: pointer;
   flex: none;
   height: 40px;
-  left: calc(100% - 40px - 3px);
-  position: absolute;
-  right: calc(100% - 40px - 3px);
+  position: relative;
   width: 40px;
   z-index: 1;  /* Make sure the icon is on upper layer than paper-ripple. */
 }

ui/file_manager/file_manager/foreground/js/ui/directory_tree.js

@@ -676,15 +676,6 @@ class DirectoryItem extends cr.ui.TreeItem {
     // Add the eject button as the last element of the tree row content.
     const parent = rowElement.querySelector('.label').parentElement;
     assert(parent).appendChild(ejectButton);
-
-    // Mark the tree row element with CSS class .ejectable.
-    rowElement.classList.add('ejectable');
-
-    // Disable paper-ripple on this rowElement, crbug.com/965382.
-    const rowRipple = rowElement.querySelector('paper-ripple');
-    if (rowRipple) {
-      rowRipple.setAttribute('style', 'visibility:hidden');
-    }
   }
 
   /**

ui/file_manager/integration_tests/file_manager/navigation_list.js → ui/file_manager/integration_tests/file_manager/directory_tree.js

@@ -5,10 +5,10 @@
 'use strict';
 (() => {
   /**
-   * Tests that the directory tree area can be vertically scrolled.
+   * Tests that the directory tree can be vertically scrolled.
    */
-  testcase.navigationListVerticalScroll = async () => {
-    // Add directory entries to overflow the navigation list container.
+  testcase.directoryTreeVerticalScroll = async () => {
+    // Add directory entries to overflow the directory tree container.
     let folders = [ENTRIES.photos];
     for (let i = 0; i < 30; i++) {
       folders.push(new TestEntryInfo({
@@ -21,66 +21,53 @@
       }));
     }
 
-    // Open FilesApp on Downloads. Expand the navigation list view of Downloads.
+    // Open FilesApp on Downloads. Expand the directory tree view of Downloads.
     const appId = await setupAndWaitUntilReady(RootPath.DOWNLOADS, folders, []);
     await expandRoot(appId, TREEITEM_DOWNLOADS);
 
-    // Get the navigationList and verify it is not scrolled.
-    const navigationList = '.dialog-navigation-list';
-    const originalList = await remoteCall.waitForElementStyles(
-        appId, navigationList, ['scrollTop']);
-    chrome.test.assertTrue(originalList.scrollTop === 0);
+    // Get the directory tree and verify it is not scrolled.
+    const directoryTree = '#directory-tree';
+    const original = await remoteCall.waitForElementStyles(
+        appId, directoryTree, ['scrollTop']);
+    chrome.test.assertTrue(original.scrollTop === 0);
 
-    // Scroll the navigation list down (vertical scroll).
+    // Scroll the directory tree down (vertical scroll).
     await remoteCall.callRemoteTestUtil(
-        'setScrollTop', appId, [navigationList, 100]);
+        'setScrollTop', appId, [directoryTree, 100]);
 
-    // Check: the navigation list should be scrolled.
-    const newList = await remoteCall.waitForElementStyles(
-        appId, navigationList, ['scrollTop']);
-    chrome.test.assertTrue(newList.scrollTop > 0);
+    // Check: the directory tree should be scrolled.
+    const scrolled = await remoteCall.waitForElementStyles(
+        appId, directoryTree, ['scrollTop']);
+    chrome.test.assertTrue(scrolled.scrollTop === 100, 'Tree should scroll');
   };
 
   /**
-   * Tests that the directory tree area can be horizontally scrolled.
-   * TODO(crbug.com/966807): Clean up test case to match the comment style and
-   * organization of navigationListVerticalScroll.
-   * Remove unnecessary check that originalWidth > 0.
+   * Tests that the directory tree does not horizontally scroll.
    */
-  testcase.navigationListHorizontalScroll = async () => {
+  testcase.directoryTreeHorizontalScroll = async () => {
     // Open FilesApp on Downloads. Expand the navigation list view of Downloads.
     const appId = await setupAndWaitUntilReady(
         RootPath.DOWNLOADS, BASIC_LOCAL_ENTRY_SET, []);
     await expandRoot(appId, TREEITEM_DOWNLOADS);
 
-    // Get the navigationList width property and make sure it has non-zero size.
-    const navigationList = '.dialog-navigation-list';
-    const originalList =
-        await remoteCall.waitForElementStyles(appId, navigationList, ['width']);
-    const originalWidth = Number(originalList.styles['width'].match(/[0-9]*/));
-    chrome.test.assertTrue(originalWidth > 0);
+    // Get the directory tree and verify it is not scrolled.
+    const directoryTree = '#directory-tree';
+    const original = await remoteCall.waitForElementStyles(
+        appId, directoryTree, ['scrollLeft']);
+    chrome.test.assertTrue(original.scrollLeft === 0);
 
-    // Shrink the navigation list tree area to 50px.
+    // Shrink the navigation list tree to 50px.
+    const navigationList = '.dialog-navigation-list';
     await remoteCall.callRemoteTestUtil(
         'setElementStyles', appId, [navigationList, {width: '50px'}]);
 
-    // Check the navigation list area is scrolled completely to the left side.
-    chrome.test.assertTrue(originalList.scrollLeft === 0);
-
-    // Scroll the navigation list down (horizontal scroll).
+    // Scroll the directory tree left (horizontal scroll).
     await remoteCall.callRemoteTestUtil(
-        'setScrollLeft', appId, [navigationList, 100]);
-
-    // Get the current state of the NavigationWidth width and left scroll
-    // offset.
-    const newList =
-        await remoteCall.waitForElementStyles(appId, navigationList, ['width']);
-
-    // Check that the width has been reduced.
-    const newWidth = Number(newList.styles['width'].match(/[0-9]*/));
-    chrome.test.assertTrue(newWidth < originalWidth);
+        'setScrollLeft', appId, [directoryTree, 100]);
 
-    // Check: the navigation list should be scrolled.
-    chrome.test.assertTrue(newList.scrollLeft > 0);
+    // Check; the directory tree should not be scrolled.
+    const scrolled = await remoteCall.waitForElementStyles(
+        appId, directoryTree, ['scrollLeft']);
+    chrome.test.assertTrue(scrolled.scrollLeft === 0, 'Tree should not scroll');
   };
 })();

ui/file_manager/integration_tests/file_manager_test_manifest.json

@@ -18,6 +18,7 @@
       "file_manager/copy_between_windows.js",
       "file_manager/create_new_folder.js",
       "file_manager/crostini.js",
+      "file_manager/directory_tree.js",
       "file_manager/directory_tree_context_menu.js",
       "file_manager/drive_specific.js",
       "file_manager/file_dialog.js",
@@ -34,7 +35,6 @@
       "file_manager/metadata.js",
       "file_manager/metrics.js",
       "file_manager/my_files.js",
-      "file_manager/navigation_list.js",
       "file_manager/open_audio_files.js",
       "file_manager/open_image_files.js",
       "file_manager/open_sniffed_files.js",