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

Issue 682356 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 8
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: ----

Blocked on:
issue 658456
issue 869252
issue 869282

Blocking:
issue 831945



Sign in to add a comment

Move "Add new services" from left nav to overflow menu

Project Member Reported by weifangsun@chromium.org, Jan 18 2017

Issue description

Following up from UI review for crbug.com/669966 -

We should move the "Add new services" action item, currently located in the left navigation of the Files App as an option under the overflow menu in the upper right hand corner of the app.

 
Description: Show this description
Blockedon: 658456
Linking a bug that we should review when we revisit this update.
Cc: mtomasz@chromium.org
What's the rationale for moving "Add new services" to the overflow menu? Will it be more discoverable and/or less distracting for users?
I think the main feedback is that "Add new services" is an action rather than directory and so should be in a menu.
That's true, but it's an action related to volumes, not to files. The overflow menu is naturally less discoverable and may reduce number of installs of Files app extensions. It will also make it difficult to mount new volumes exposed by these extensions (Such as Windows shares / samba).

I'm not saying we shouldn't do that, but I wanted to make sure that both pros and cons were carefully taken into account.

Comment 6 by sashab@chromium.org, Feb 24 2018

Cc: -weifangsun@chromium.org fukino@chromium.org
Labels: CrOS-FilesApp
Owner: weifangsun@chromium.org
Assigning to weifang to investigate whether we want to do this or not.

Comment 7 by sashab@chromium.org, Feb 28 2018

Labels: -CrOS-FilesApp
Cc: weifangsun@chromium.org
Labels: M-69
Owner: ----
Status: Available (was: Assigned)
We should do this as part of the UI refresh in M69.
Blocking: 831945

Comment 10 by loyso@chromium.org, May 30 2018

IMZHO we need a new action (overflow?) menu for volumes/drives/FSPs etc...
Owner: lucmult@chromium.org
Status: Started (was: Available)
Labels: -Pri-3 Pri-2
Cc: mcirimele@chromium.org
Following up on our conversation yesterday we made a few changes. 

When no services are installed:
- Show "Install new services" above storage in the context menu (no separator).

When there are services installed:
- Show "Add new services" above storage in the context menu (no separator)
- Clicking "Add new services" brings up a second menu.
- Second menu shows previously installed services first, then a separator, then "Install new service". 

Post-M69 we would like to look into implementing the submenu pattern across Files app. 

Let me know if you have any questions!
add-services-no-services.png
103 KB View Download
add-services-installed-services.png
103 KB View Download
Sorry! It should be singular - 

"Install new service"
"Add new service"

Got it right in the mock at least :)
Blockedon: 869252
Blockedon: 869282
Project Member

Comment 19 by bugdroid1@chromium.org, Aug 1

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

commit 65de167b0a621f73d4bb14b596af67b892afe33b
Author: Luciano Pacheco <lucmult@chromium.org>
Date: Wed Aug 01 00:32:13 2018

Move "Add new services" button from directory tree

Add "Add new services" menu item to Files app "gear menu" and change
the behavior to:

1. Go directly to Webstore in case there isn't any provider/FSP
installed.
2. Open "providers menu" when there is at least one provider/FSP
installed, which is the old behaviour.

Add a new command "new-service" to display the providers menu to be
able open it from the gear menu.

Change the gear menu item label to be "Add new service".

Fix My files tests because "Add new services" isn't displayed on
directory tree anymore.

Bug:  682356 ,  869240 
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: Ie9bf211c42a18b3717a72f40e57ceaade5958281
Reviewed-on: https://chromium-review.googlesource.com/1139944
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579639}
[modify] https://crrev.com/65de167b0a621f73d4bb14b596af67b892afe33b/chrome/app/chromeos_strings.grdp
[modify] https://crrev.com/65de167b0a621f73d4bb14b596af67b892afe33b/ui/file_manager/file_manager/foreground/js/file_manager.js
[modify] https://crrev.com/65de167b0a621f73d4bb14b596af67b892afe33b/ui/file_manager/file_manager/foreground/js/file_manager_commands.js
[modify] https://crrev.com/65de167b0a621f73d4bb14b596af67b892afe33b/ui/file_manager/file_manager/foreground/js/gear_menu_controller.js
[modify] https://crrev.com/65de167b0a621f73d4bb14b596af67b892afe33b/ui/file_manager/file_manager/foreground/js/ui/file_manager_ui.js
[modify] https://crrev.com/65de167b0a621f73d4bb14b596af67b892afe33b/ui/file_manager/file_manager/foreground/js/ui/gear_menu.js
[modify] https://crrev.com/65de167b0a621f73d4bb14b596af67b892afe33b/ui/file_manager/file_manager/main.html
[modify] https://crrev.com/65de167b0a621f73d4bb14b596af67b892afe33b/ui/file_manager/file_manager/test/js/strings.js
[modify] https://crrev.com/65de167b0a621f73d4bb14b596af67b892afe33b/ui/file_manager/integration_tests/file_manager/my_files.js
[modify] https://crrev.com/65de167b0a621f73d4bb14b596af67b892afe33b/ui/file_manager/integration_tests/file_manager/providers.js

mcirimele@

Can you check the attachments and let me know if they're the way we intend?

After you confirm I'll request to merge on M-69 branch.
add-new-service
2.0 MB View Download
navigate-between-folders-install-new-service-share-line-div.webm
4.5 MB View Download
Looks good to me, thanks for the screencasts.
Labels: Merge-Request-69
I'm requesting to merge the CL below on M-69.
crrev.com/c/1139944

The screencasts from #20 were recorded on branch 69 (branch-heads/3497) with the patch merged there.

Since there isn't a simple way to have other users to test it on M-69, I resorted to record a screencast and our UX mcirimele@ agreed that it's working as intended on #21.

The patch includes integration tests too. 
Project Member

Comment 23 by sheriffbot@chromium.org, Aug 8

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: There is .grd file changes and we are only 26 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-69 Merge-Approved-69
This merge includes a .grd change that includes text change, with could require translations. The .grd file will be removed from the merge to M69. Approving M69 merge to move and not change the text. Per conversation with lucmult@.
Project Member

Comment 25 by bugdroid1@chromium.org, Aug 8

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a0b7e14a19bfb170a3acceb29fc0ea3157eb6cd2

commit a0b7e14a19bfb170a3acceb29fc0ea3157eb6cd2
Author: Luciano Pacheco <lucmult@chromium.org>
Date: Wed Aug 08 02:40:02 2018

Move "Add new services" button from directory tree

Add "Add new services" menu item to Files app "gear menu" and change
the behavior to:

1. Go directly to Webstore in case there isn't any provider/FSP
installed.
2. Open "providers menu" when there is at least one provider/FSP
installed, which is the old behaviour.

Add a new command "new-service" to display the providers menu to be
able open it from the gear menu.

Change the gear menu item label to be "Add new service".

Fix My files tests because "Add new services" isn't displayed on
directory tree anymore.

Bug:  682356 ,  869240 
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: Ie9bf211c42a18b3717a72f40e57ceaade5958281
Reviewed-on: https://chromium-review.googlesource.com/1139944
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#579639}(cherry picked from commit 65de167b0a621f73d4bb14b596af67b892afe33b)
Reviewed-on: https://chromium-review.googlesource.com/1166482
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#485}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/a0b7e14a19bfb170a3acceb29fc0ea3157eb6cd2/ui/file_manager/file_manager/foreground/js/file_manager.js
[modify] https://crrev.com/a0b7e14a19bfb170a3acceb29fc0ea3157eb6cd2/ui/file_manager/file_manager/foreground/js/file_manager_commands.js
[modify] https://crrev.com/a0b7e14a19bfb170a3acceb29fc0ea3157eb6cd2/ui/file_manager/file_manager/foreground/js/gear_menu_controller.js
[modify] https://crrev.com/a0b7e14a19bfb170a3acceb29fc0ea3157eb6cd2/ui/file_manager/file_manager/foreground/js/ui/file_manager_ui.js
[modify] https://crrev.com/a0b7e14a19bfb170a3acceb29fc0ea3157eb6cd2/ui/file_manager/file_manager/foreground/js/ui/gear_menu.js
[modify] https://crrev.com/a0b7e14a19bfb170a3acceb29fc0ea3157eb6cd2/ui/file_manager/file_manager/main.html
[modify] https://crrev.com/a0b7e14a19bfb170a3acceb29fc0ea3157eb6cd2/ui/file_manager/file_manager/test/js/strings.js
[modify] https://crrev.com/a0b7e14a19bfb170a3acceb29fc0ea3157eb6cd2/ui/file_manager/integration_tests/file_manager/my_files.js
[modify] https://crrev.com/a0b7e14a19bfb170a3acceb29fc0ea3157eb6cd2/ui/file_manager/integration_tests/file_manager/providers.js

Status: Fixed (was: Started)
Merged on M-69 branch without .grd file as can be seen on comment #25.

To merge without the translation I have:

1. Created the cherry-pick patch using Gerrit.
2. Created a local branch from branch_heads/3497
3. Ran git cl patch #patch id created on step1
4. Reverted .grd file
5. Uploaded a new patchset.

Sign in to add a comment