New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 846587 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 16
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug


Sign in to add a comment

Add "My Files" root in new Files app navigation

Project Member Reported by sashab@chromium.org, May 25 2018

Issue description

Add support for nested roots, and create the "My Files" root in the new Files app navigation.

When the user clicks on the root, it opens the child items in the RHS panel.

This is part of a project to refresh the Files App navigation UI (mocks - go/filesapp-2018-mocks).
 

Comment 1 by sashab@chromium.org, May 25 2018

Blocking: 846588

Comment 2 by sashab@chromium.org, May 25 2018

Blocking: 846589

Comment 3 by sashab@chromium.org, May 25 2018

Blocking: 846590
Blocking: 841640
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d0bca74166ccd0d8534ffaf3ead5beaf779a8350

commit d0bca74166ccd0d8534ffaf3ead5beaf779a8350
Author: Luciano Pacheco <lucmult@chromium.org>
Date: Wed Jul 04 06:41:00 2018

Implement base interface and types for new navigation

Add interface FilesAppEntry which is the base interface that moving
forward the app UI will converge as base type that can be displayed on
different UI components such as: navigation tree, file list and
breadcrumbs, eventually superseding Entry type.

Add VolumeEntry which implements interface FilesAppEntry to represent a
Volume, this will allow to display Volumes on file list/Right Hand Side
(RHS).

Add EntryList which implements interface FilesAppEntry to represent a
list of entries.  This will be used to implement "My Files" which will
contain a list of VolumeEntry for the volumes: Downloads, Linux Files
(Crostini) and Play Files (ARC++).

Design doc: https://docs.google.com/document/d/1X5XSLKJd0yerL-qFhpb2z9ibUVb_W3gG_tfIV7T_Qt0

Bug:  846587 , 835203
Test: Unit-test for the new types.
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: Ia2fce233338f8b8e0969b77daf4c77139852c441
Reviewed-on: https://chromium-review.googlesource.com/1086680
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Reviewed-by: Sasha Morrissey <sashab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572496}
[modify] https://crrev.com/d0bca74166ccd0d8534ffaf3ead5beaf779a8350/chrome/browser/chromeos/file_manager/file_manager_jstest.cc
[modify] https://crrev.com/d0bca74166ccd0d8534ffaf3ead5beaf779a8350/ui/file_manager/file_manager/common/js/BUILD.gn
[add] https://crrev.com/d0bca74166ccd0d8534ffaf3ead5beaf779a8350/ui/file_manager/file_manager/common/js/files_app_entry_types.js
[add] https://crrev.com/d0bca74166ccd0d8534ffaf3ead5beaf779a8350/ui/file_manager/file_manager/common/js/files_app_entry_types_unittest.html
[add] https://crrev.com/d0bca74166ccd0d8534ffaf3ead5beaf779a8350/ui/file_manager/file_manager/common/js/files_app_entry_types_unittest.js

Project Member

Comment 6 by bugdroid1@chromium.org, Jul 4

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e408b270a1e523054c950595ad139558979e8282

commit e408b270a1e523054c950595ad139558979e8282
Author: Luciano Pacheco <lucmult@chromium.org>
Date: Wed Jul 04 07:15:45 2018

Add closure markup to use FilesAppEntry

This is a continuation of crrev.com/c/1086680 and crrev.com/c/1116400.

isFakeEntry changed the logic to not use operator "in" since Closure
compiler was failing for FilesAppEntry types:
"ERROR - Cannot use the IN operator with structs".

Bug:  846587 
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: I34914893d01bba43fdbcfc9e4b197031e50c662a
Reviewed-on: https://chromium-review.googlesource.com/1116517
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Reviewed-by: Sasha Morrissey <sashab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572503}
[modify] https://crrev.com/e408b270a1e523054c950595ad139558979e8282/ui/file_manager/externs/command_handler_deps.js
[modify] https://crrev.com/e408b270a1e523054c950595ad139558979e8282/ui/file_manager/externs/volume_info_list.js
[modify] https://crrev.com/e408b270a1e523054c950595ad139558979e8282/ui/file_manager/externs/volume_manager.js
[modify] https://crrev.com/e408b270a1e523054c950595ad139558979e8282/ui/file_manager/file_manager/background/js/mock_volume_manager.js
[modify] https://crrev.com/e408b270a1e523054c950595ad139558979e8282/ui/file_manager/file_manager/background/js/volume_manager_impl.js
[modify] https://crrev.com/e408b270a1e523054c950595ad139558979e8282/ui/file_manager/file_manager/common/js/importer_common.js
[modify] https://crrev.com/e408b270a1e523054c950595ad139558979e8282/ui/file_manager/file_manager/common/js/util.js
[modify] https://crrev.com/e408b270a1e523054c950595ad139558979e8282/ui/file_manager/file_manager/common/js/volume_manager_common.js
[modify] https://crrev.com/e408b270a1e523054c950595ad139558979e8282/ui/file_manager/file_manager/foreground/js/directory_contents.js
[modify] https://crrev.com/e408b270a1e523054c950595ad139558979e8282/ui/file_manager/file_manager/foreground/js/directory_model.js
[modify] https://crrev.com/e408b270a1e523054c950595ad139558979e8282/ui/file_manager/file_manager/foreground/js/file_manager.js
[modify] https://crrev.com/e408b270a1e523054c950595ad139558979e8282/ui/file_manager/file_manager/foreground/js/file_manager_commands.js
[modify] https://crrev.com/e408b270a1e523054c950595ad139558979e8282/ui/file_manager/file_manager/foreground/js/file_transfer_controller.js
[modify] https://crrev.com/e408b270a1e523054c950595ad139558979e8282/ui/file_manager/file_manager/foreground/js/import_controller.js
[modify] https://crrev.com/e408b270a1e523054c950595ad139558979e8282/ui/file_manager/file_manager/foreground/js/volume_manager_wrapper.js

Project Member

Comment 8 by bugdroid1@chromium.org, Jul 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/10195bc69820c4cc24e332655aa939e9a6cf374c

commit 10195bc69820c4cc24e332655aa939e9a6cf374c
Author: Luciano Pacheco <lucmult@chromium.org>
Date: Wed Jul 04 07:38:01 2018

Add My Files parent for Downloads (behind flag)

Add My Files parent on navigation tree, navigation_list_model.js
creates the "My Files" entry as a EntryList and EntryListItem on
directory_tree.js is the UI element to display it.

This is a continuation of:
 * crrev.com/c/1086680
 * crrev.com/c/1116400
 * crrev.com/c/1116517

Some changes required to display EntryList:
 * isDescendantEntry: Inspect on each of EntryList's children.
 * FileManager.initializeUI: Fetch asynchronously the flag to be able
to use before displaying the navigation tree.
 * extract the switch (modelItem.type) to its own function, to be able
to reuse it.

Design doc: https://docs.google.com/document/d/1X5XSLKJd0yerL-qFhpb2z9ibUVb_W3gG_tfIV7T_Qt0

Bug:  846587 ,  846592 ,  846591 ,  846590 ,  846589 ,  846588 ,  846586 
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: Id8f27f09763d4478b93d52dd32e4ed851c7e448d
Reviewed-on: https://chromium-review.googlesource.com/1113258
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Reviewed-by: Sasha Morrissey <sashab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572507}
[modify] https://crrev.com/10195bc69820c4cc24e332655aa939e9a6cf374c/chrome/app/chromeos_strings.grdp
[add] https://crrev.com/10195bc69820c4cc24e332655aa939e9a6cf374c/chrome/app/chromeos_strings_grdp/IDS_FILE_BROWSER_MY_FILES_ROOT_LABEL.png.sha1
[modify] https://crrev.com/10195bc69820c4cc24e332655aa939e9a6cf374c/chrome/browser/chromeos/extensions/file_manager/private_api_strings.cc
[modify] https://crrev.com/10195bc69820c4cc24e332655aa939e9a6cf374c/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc
[modify] https://crrev.com/10195bc69820c4cc24e332655aa939e9a6cf374c/ui/file_manager/file_manager/background/js/volume_manager_impl.js
[modify] https://crrev.com/10195bc69820c4cc24e332655aa939e9a6cf374c/ui/file_manager/file_manager/common/js/util.js
[modify] https://crrev.com/10195bc69820c4cc24e332655aa939e9a6cf374c/ui/file_manager/file_manager/common/js/volume_manager_common.js
[modify] https://crrev.com/10195bc69820c4cc24e332655aa939e9a6cf374c/ui/file_manager/file_manager/foreground/js/directory_model.js
[modify] https://crrev.com/10195bc69820c4cc24e332655aa939e9a6cf374c/ui/file_manager/file_manager/foreground/js/file_manager.js
[modify] https://crrev.com/10195bc69820c4cc24e332655aa939e9a6cf374c/ui/file_manager/file_manager/foreground/js/navigation_list_model.js
[modify] https://crrev.com/10195bc69820c4cc24e332655aa939e9a6cf374c/ui/file_manager/file_manager/foreground/js/navigation_list_model_unittest.js
[modify] https://crrev.com/10195bc69820c4cc24e332655aa939e9a6cf374c/ui/file_manager/file_manager/foreground/js/ui/directory_tree.js
[modify] https://crrev.com/10195bc69820c4cc24e332655aa939e9a6cf374c/ui/file_manager/file_manager/foreground/js/ui/location_line.js
[add] https://crrev.com/10195bc69820c4cc24e332655aa939e9a6cf374c/ui/file_manager/integration_tests/file_manager/my_files.js
[modify] https://crrev.com/10195bc69820c4cc24e332655aa939e9a6cf374c/ui/file_manager/integration_tests/file_manager_test_manifest.json

Project Member

Comment 9 by bugdroid1@chromium.org, Jul 6

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bc4f4fff5a133785a9709a3982ad0aff6897f281

commit bc4f4fff5a133785a9709a3982ad0aff6897f281
Author: Luciano Pacheco <lucmult@chromium.org>
Date: Fri Jul 06 03:52:27 2018

Add My Files to breadcrumbs

Change VolumeInfo to allow a prefix entry to be used as a prefix on
breadcrumbs.

Change EntryList to inject itself as prefix on VolumeEntry and its
VolumeInfo.

Add test for checking that My Files is diplayed on breadcrumbs.

Bug:  846587 
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: If53a02a44e83b0b50f9416ea6387a1ae9c585f41
Reviewed-on: https://chromium-review.googlesource.com/1126588
Reviewed-by: Sasha Morrissey <sashab@chromium.org>
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572888}
[modify] https://crrev.com/bc4f4fff5a133785a9709a3982ad0aff6897f281/ui/file_manager/externs/volume_info.js
[modify] https://crrev.com/bc4f4fff5a133785a9709a3982ad0aff6897f281/ui/file_manager/file_manager/background/js/volume_info_impl.js
[modify] https://crrev.com/bc4f4fff5a133785a9709a3982ad0aff6897f281/ui/file_manager/file_manager/common/js/files_app_entry_types.js
[modify] https://crrev.com/bc4f4fff5a133785a9709a3982ad0aff6897f281/ui/file_manager/file_manager/common/js/files_app_entry_types_unittest.js
[modify] https://crrev.com/bc4f4fff5a133785a9709a3982ad0aff6897f281/ui/file_manager/file_manager/foreground/js/ui/location_line.js
[modify] https://crrev.com/bc4f4fff5a133785a9709a3982ad0aff6897f281/ui/file_manager/integration_tests/file_manager/my_files.js

Project Member

Comment 10 by bugdroid1@chromium.org, Jul 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2f6a359563677fdf5f87552a1fc06283ca8e32ed

commit 2f6a359563677fdf5f87552a1fc06283ca8e32ed
Author: Luciano Pacheco <lucmult@chromium.org>
Date: Wed Jul 11 02:45:37 2018

Add icon to My Files

Add both icons for My Files active and inactive.

Bug:  846593 ,  846587 
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: I718f3374868686ed0aa9480deb83ebf96e9717e4
Reviewed-on: https://chromium-review.googlesource.com/1130557
Reviewed-by: Sasha Morrissey <sashab@chromium.org>
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574047}
[modify] https://crrev.com/2f6a359563677fdf5f87552a1fc06283ca8e32ed/ui/file_manager/file_manager/foreground/css/file_types.css
[add] https://crrev.com/2f6a359563677fdf5f87552a1fc06283ca8e32ed/ui/file_manager/file_manager/foreground/images/volumes/2x/my_files.png
[add] https://crrev.com/2f6a359563677fdf5f87552a1fc06283ca8e32ed/ui/file_manager/file_manager/foreground/images/volumes/2x/my_files_active.png
[add] https://crrev.com/2f6a359563677fdf5f87552a1fc06283ca8e32ed/ui/file_manager/file_manager/foreground/images/volumes/my_files.png
[add] https://crrev.com/2f6a359563677fdf5f87552a1fc06283ca8e32ed/ui/file_manager/file_manager/foreground/images/volumes/my_files_active.png
[modify] https://crrev.com/2f6a359563677fdf5f87552a1fc06283ca8e32ed/ui/file_manager/file_manager/foreground/js/ui/directory_tree.js

Project Member

Comment 11 by bugdroid1@chromium.org, Jul 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f79c17b68e41c6a125b8798709431fd5363980fa

commit f79c17b68e41c6a125b8798709431fd5363980fa
Author: Luciano Pacheco <lucmult@chromium.org>
Date: Wed Jul 11 03:41:18 2018

Fix My Files to display its children on right hand side

Also fix to be able to open Downloads from RHS.

Add |name| attribute for crostini FakeEntry so it can display it on
RHS.

Add |file-name| attribute on FileTable rows to make it easier to select
a single row on tests.

Fix crash due to DCHECK on
getFileTasksInternal::FileManagerPrivateInternalGetFileTasksFunction
where it would fail with entries being empty when we send an array with
only FilesAppEntry instances, because the API binding discards objects
that aren't native Entry. So we remove non-native entries before
calling |FileManagerPrivate.getFileTasks|.

Change DirectoryContentScanner to not check for FakeEntry but instead
check for the availability of the desired method |createReader| since
FilesAppEntry also implements it even though they're non-native (aka
FakeEntry). This fixes My Files to display its children on RHS.

Change FileFilter to check for attributes existence (filesystem and
fullPath) before operating on it. This fixes crostini FakeEntry to be
able to be displayed on RHS.

Change DirectoryModel to check for non-null VolumeInfo. This fixes
this function running for My Files and crostini entries.

Bug:  846587 ,  857335 
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: Id3cfd1e957db57114c97de1d34a10494dcaaeedb
Reviewed-on: https://chromium-review.googlesource.com/1130559
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Reviewed-by: Sasha Morrissey <sashab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574060}
[modify] https://crrev.com/f79c17b68e41c6a125b8798709431fd5363980fa/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc
[modify] https://crrev.com/f79c17b68e41c6a125b8798709431fd5363980fa/ui/file_manager/file_manager/background/js/launcher_search.js
[modify] https://crrev.com/f79c17b68e41c6a125b8798709431fd5363980fa/ui/file_manager/file_manager/common/js/util.js
[modify] https://crrev.com/f79c17b68e41c6a125b8798709431fd5363980fa/ui/file_manager/file_manager/foreground/js/directory_contents.js
[modify] https://crrev.com/f79c17b68e41c6a125b8798709431fd5363980fa/ui/file_manager/file_manager/foreground/js/directory_model.js
[modify] https://crrev.com/f79c17b68e41c6a125b8798709431fd5363980fa/ui/file_manager/file_manager/foreground/js/file_manager.js
[modify] https://crrev.com/f79c17b68e41c6a125b8798709431fd5363980fa/ui/file_manager/file_manager/foreground/js/file_tasks.js
[modify] https://crrev.com/f79c17b68e41c6a125b8798709431fd5363980fa/ui/file_manager/file_manager/foreground/js/ui/file_table.js
[modify] https://crrev.com/f79c17b68e41c6a125b8798709431fd5363980fa/ui/file_manager/integration_tests/file_manager/my_files.js

Project Member

Comment 12 by bugdroid1@chromium.org, Jul 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/29b0747b710b65c27814e0b7af308cbbab28d00e

commit 29b0747b710b65c27814e0b7af308cbbab28d00e
Author: Luciano Pacheco <lucmult@chromium.org>
Date: Mon Jul 16 04:29:02 2018

Move media view volumes above shortcuts

This change is following: go/filesapp-2018-mocks

Bug:  846587 
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: I066b3670ef0c06bc2b170ecd8991b6940fdac637
Reviewed-on: https://chromium-review.googlesource.com/1137804
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575199}
[modify] https://crrev.com/29b0747b710b65c27814e0b7af308cbbab28d00e/ui/file_manager/file_manager/foreground/js/navigation_list_model.js
[modify] https://crrev.com/29b0747b710b65c27814e0b7af308cbbab28d00e/ui/file_manager/file_manager/foreground/js/navigation_list_model_unittest.js

Status: Fixed (was: Assigned)
Project Member

Comment 14 by bugdroid1@chromium.org, Jul 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a0e2753f75c926313e183b912584a7f15790825d

commit a0e2753f75c926313e183b912584a7f15790825d
Author: Luciano Pacheco <lucmult@chromium.org>
Date: Mon Jul 16 07:41:10 2018

Rename the "My Files" flag to disable-my-files-navigation

"My Files" is on by default after crrev.com/c/1131025. Rename
the "My Files" flag to disable-my-files-navigation so can turn
it off.

Bug:  846587 ,  846585 
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: I987230566c383fd8385f65916704cd0e9b32fd27
Reviewed-on: https://chromium-review.googlesource.com/1136456
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575203}
[modify] https://crrev.com/a0e2753f75c926313e183b912584a7f15790825d/chrome/browser/about_flags.cc
[modify] https://crrev.com/a0e2753f75c926313e183b912584a7f15790825d/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/a0e2753f75c926313e183b912584a7f15790825d/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/a0e2753f75c926313e183b912584a7f15790825d/chromeos/chromeos_switches.cc
[modify] https://crrev.com/a0e2753f75c926313e183b912584a7f15790825d/chromeos/chromeos_switches.h
[modify] https://crrev.com/a0e2753f75c926313e183b912584a7f15790825d/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/a0e2753f75c926313e183b912584a7f15790825d/ui/file_manager/file_manager/common/js/util.js
[modify] https://crrev.com/a0e2753f75c926313e183b912584a7f15790825d/ui/file_manager/file_manager/foreground/js/file_manager.js
[modify] https://crrev.com/a0e2753f75c926313e183b912584a7f15790825d/ui/file_manager/file_manager/foreground/js/navigation_list_model.js

Project Member

Comment 15 by bugdroid1@chromium.org, Jul 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5a35dd521e51fd02698d6718848e75534e346317

commit 5a35dd521e51fd02698d6718848e75534e346317
Author: Luciano Pacheco <lucmult@chromium.org>
Date: Tue Jul 17 02:34:47 2018

Add entry icons on file list

Add iconName property on VolumeEntry and on crostini FakeItem.

Change FileType.getIcon to prefer iconName property if available.

Change CSS to add icon to file-type-icon which is used on file list and
file grid.

The changes above fix crostini (a FakeEntry), downloads and android
(VolumeEntry) to be displayed with proper icons on RHS.

Change directory tree (LHS) icon for shortcut, from "drive" to a new
dedicated icon for shortcuts.

Bug:  846587 
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: I44e08f9f6d711c45a4e698acbb203e9cb3a87d9d
Reviewed-on: https://chromium-review.googlesource.com/1132258
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575524}
[modify] https://crrev.com/5a35dd521e51fd02698d6718848e75534e346317/ui/file_manager/file_manager/common/js/file_type.js
[modify] https://crrev.com/5a35dd521e51fd02698d6718848e75534e346317/ui/file_manager/file_manager/common/js/files_app_entry_types.js
[modify] https://crrev.com/5a35dd521e51fd02698d6718848e75534e346317/ui/file_manager/file_manager/common/js/files_app_entry_types_unittest.js
[modify] https://crrev.com/5a35dd521e51fd02698d6718848e75534e346317/ui/file_manager/file_manager/foreground/css/file_types.css
[add] https://crrev.com/5a35dd521e51fd02698d6718848e75534e346317/ui/file_manager/file_manager/foreground/images/volumes/2x/shortcut.png
[add] https://crrev.com/5a35dd521e51fd02698d6718848e75534e346317/ui/file_manager/file_manager/foreground/images/volumes/2x/shortcut_active.png
[add] https://crrev.com/5a35dd521e51fd02698d6718848e75534e346317/ui/file_manager/file_manager/foreground/images/volumes/shortcut.png
[add] https://crrev.com/5a35dd521e51fd02698d6718848e75534e346317/ui/file_manager/file_manager/foreground/images/volumes/shortcut_active.png
[modify] https://crrev.com/5a35dd521e51fd02698d6718848e75534e346317/ui/file_manager/file_manager/foreground/js/file_manager.js
[modify] https://crrev.com/5a35dd521e51fd02698d6718848e75534e346317/ui/file_manager/file_manager/foreground/js/ui/directory_tree.js

Sign in to add a comment