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

Issue 846589 link

Starred by 3 users

Issue metadata

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

Blocked on:
issue 846587



Sign in to add a comment

Add "Play Apps" to new My Files root

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

Issue description

Add "Play Apps" to the new My Files root in the left hand nav, and point it to the users ARC++ files.

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

 

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

Cc: -fukino@chromium.org lucmult@chromium.org
Owner: fukino@chromium.org
I've already added a root for Android files on left-hand nav behind a flag. (--show-android-files-in-files-app)
https://chromium-review.googlesource.com/c/chromium/src/+/1067378

Let me take this bug.
Once the root folder for local files (i.e. "Pixelbool" in the mock) is introduced, I'll move the root for Android under the local root.

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

Cc: weifangsun@chromium.org
BTW, the experimental implementation uses a label "Android Files" instead of "Play Apps" because:
1) In the latest plan, we don't show folders created by Play apps by default.
2) The default folders like Pictures, Music, etc... are defined and provided by OS, not by apps.

Weifang, let's include this point to the UI discussion! 
Sorry for the confusion - After working with Sebastien on the latest mock updates, we are going to name the ARC++ root "Play Files" to be consistent with "Linux Files".

Comment 4 by sashab@chromium.org, May 28 2018

Fukino, just to clarify --

In the "old" tree (the current one in File Manager), we should display the media views (Pictures, Music, etc). There are no plans to change this.

In the "new" tree (the one that will say "Play Files"), we should display the Android folder tree (whichever folders are visible from the root). The media views are not visible in this new tree.

Does this make sense? :)

Sasha

Comment 5 by fukino@chromium.org, May 28 2018

Re:#3
To me, "Android Files" sounds more consistent with "Linux Files", as they are both in format "{OS} Files".
(As I mentioned in c#2, the default folders under "Play Files" are defined and provided by Android OS, not by Google Play service.)

Re:#4
In the "old" tree (~M68), we don't have any plan to change the tree. We are still showing media views on the tree.
IIUC, in the "new" tree (M69~), we change the tree structure (e.g. introducing "Pixelbook", the root for local files), and along with it, "Play Apps" will be added under the local root.
The detailed spec about the new tree (e.g. how we expose files from media provider) is still under discussion.
fukino@ - Definitely understand your point. However, the reason for using "Play" vs. "Android" is that this is the approach we've taken from a marketing perspective for ARC++.
Re:#6 Thank you for explaining the background!
I'll rename the volume name and change its icon.
Project Member

Comment 8 by bugdroid1@chromium.org, Jun 6 2018

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

commit 75622996bb6cbae10d346f0ecd43e4b0b0508899
Author: Naoki Fukino <fukino@chromium.org>
Date: Wed Jun 06 05:50:50 2018

Rename the volume label for Android files.

The volume label for Android files should be "Android Files", so this CL renames
the label to the appropriate one and internationalize the label.
This CL changes the icon for the "Play Files" volume, too.

Bug:  846589 
Test: Manually tested.
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Id5ad9194094c14985d20a1055c153bab6c4d6b95
Reviewed-on: https://chromium-review.googlesource.com/1086899
Reviewed-by: Sasha Morrissey <sashab@chromium.org>
Commit-Queue: Naoki Fukino <fukino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564800}
[modify] https://crrev.com/75622996bb6cbae10d346f0ecd43e4b0b0508899/chrome/app/chromeos_strings.grdp
[add] https://crrev.com/75622996bb6cbae10d346f0ecd43e4b0b0508899/chrome/app/chromeos_strings_grdp/IDS_FILE_BROWSER_ANDROID_FILES_ROOT_LABEL.png.sha1
[modify] https://crrev.com/75622996bb6cbae10d346f0ecd43e4b0b0508899/chrome/browser/chromeos/extensions/file_manager/private_api_strings.cc
[modify] https://crrev.com/75622996bb6cbae10d346f0ecd43e4b0b0508899/ui/file_manager/file_manager/background/js/volume_manager_util.js
[modify] https://crrev.com/75622996bb6cbae10d346f0ecd43e4b0b0508899/ui/file_manager/file_manager/foreground/images/volumes/2x/android.png
[modify] https://crrev.com/75622996bb6cbae10d346f0ecd43e4b0b0508899/ui/file_manager/file_manager/foreground/images/volumes/2x/android_active.png
[modify] https://crrev.com/75622996bb6cbae10d346f0ecd43e4b0b0508899/ui/file_manager/file_manager/foreground/images/volumes/android.png
[modify] https://crrev.com/75622996bb6cbae10d346f0ecd43e4b0b0508899/ui/file_manager/file_manager/foreground/images/volumes/android_active.png

Comment 9 by fukino@chromium.org, Jun 14 2018

Cc: fukino@chromium.org
Owner: lucmult@chromium.org
lucmult@ is going to move "Play Files" root under "My Files" as a part of his CL.
Let me assign this bug to lucmult@.
Project Member

Comment 10 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

Status: Fixed (was: Assigned)

Sign in to add a comment