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

Issue 875109 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 14
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: ----

Blocked on:
issue 796815



Sign in to add a comment

Add "Paste" keyboard shortcut in overflow menu

Project Member Reported by mcirimele@chromium.org, Aug 16

Issue description

Description of the issue:
The shortcut for "Paste" is only shown in the overflow menu when a file is selected.

Steps to reproduce:
1. Copy something in Files app.
2. Click the overflow (three dot) menu when no file is selected

Expected:
"Paste" and "Ctrl+V" is shown on one line

Observed:
"Paste" is shown without the shortcut.

Solution:
Add "Ctrl+V" to the "Paste" option whenever it is visible. Note: it should also be there when the action is grayed out.


 
Components: Platform>Apps>FileManager
Labels: OS-Chrome
Owner: noel@chromium.org
Status: Assigned (was: Available)
Labels: -M-70 M-71
Labels: Files-Fixit-2018
Labels: -M-71 M-72
Blockedon: 796815
This paste menu item was added in Chrome M65  issue 796815  with change log
  https://chromium-review.googlesource.com/c/chromium/src/+/841962

Cc: fukino@chromium.org yamaguchi@chromium.org
Owner: fukino@chromium.org
That change included a Ctrl+V shortcut for this new gear-menu item in patch-set #7. However, during code review, the shortcut was explicitly removed per fukino-san's request:

https://chromium-review.googlesource.com/c/chromium/src/+/841962/7/ui/file_manager/file_manager/main.html#93

  "Ctrl-V should be bound only to paste command."

Re-assigning to fukino-san for comment / reasoning. +cc yamaguchi-san
Actually I don't remember the reason...
Is it OK to assign the same shortcut for multiple commands? (both command are fired or one is prioritized?)
I'm not sure the behavior.

BTW, what should we do when one folder is selected and the gear menu is opened?
If we show "paste-into-folder" in such cases, the behavior of Ctrl-V is consistent and won't need paste-into-current command.
Owner: noel@chromium.org
>Actually I don't remember the reason...

No worries :)

> Is it OK to assign the same shortcut for multiple commands? (both command are fired or one is prioritized?)  I'm not sure the behavior.

Turns out it is ok.  When I press Ctrl+V, only the #paste command is executed, the #paste-into-current-folder is not executed.

For more details, see 
  https://chromium-review.googlesource.com/c/chromium/src/+/1331352

> BTW, what should we do when one folder is selected and the gear menu is opened?

Good question, not sure.  Current behavior ignores the fact that a folder is selected, so gear-menu->paste and does the same action as Ctrl+V (paste).

My plan was to stick to that.

> If we show "paste-into-folder" in such cases, the behavior of Ctrl-V is consistent and won't need paste-into-current command.

Interesting idea...
Project Member

Comment 11 by bugdroid1@chromium.org, Nov 14

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

commit 6b0833d45d8449489762fa4f823fb950bb15658a
Author: Noel Gordon <noel@chromium.org>
Date: Wed Nov 14 07:47:28 2018

Add Ctrl+V shortcut for gear-menu #paste-into-current-folder item

CL:841962 added the paste-into-current-folder gear-menu command, but no
keyboard shortcut was provided (M65).

Add a Ctrl+V shortcut to the #paste-into-current-folder <command> so it
is displayed in the gear-menu.

The #paste <command> has the same shortcut. So if a user clicks Ctrl+V,
would both commands execute? Manual testing showed that Ctrl+V executes
the #paste command only.

Automatic tests cover keyboard (copy) Ctrl+C then (paste) Ctrl+V case:

  KeyboardOperations/FilesApp*keyboardCopyDownloads
  KeyboardOperations/FilesApp*keyboardCopyDrive

Automatic tests cover gear-menu #paste-into-current-folder use:

  GearMenu/FilesApp*showPasteIntoCurrentFolder

Bug:  875109 
Change-Id: Iec2a98ae0b31ccbe1d2518c6d4c4e8edd7e02a2d
Reviewed-on: https://chromium-review.googlesource.com/c/1331352
Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607930}
[modify] https://crrev.com/6b0833d45d8449489762fa4f823fb950bb15658a/ui/file_manager/file_manager/main.html

Status: Fixed (was: Assigned)
> > If we show "paste-into-folder" in such cases, the behavior of Ctrl-V is consistent and won't need paste-into-current command.

> Interesting idea...

Spoke to weifangsun@ about this, who mentioned the reasons for the existing behavior and suggested we stick with them for now.  (Nice idea though).
Labels: -Files-Fixit-2018

Comment 14 Deleted

Found  issue 875405 , exactly the same as the "interesting idea..." above (last paragraph #9.  Given #12, I think we can close  issue 875405 .

Labels: Files-Fixit-2018
(Silly of me to remove that label in #13)

Sign in to add a comment