Files app: Add a label to drag image when attempting to move files to write-protected volume. |
||||||||
Issue descriptionWhat steps will reproduce the problem? (1) Insert a SD card which is write protected. (2) Open FIles app (3) Drag a file or a folder to the inserted drive. As seen in the attached screenshot, we see an icon (red circle with slash) representing that moving the file is not allowed. It'd be better to have a text label along with the icon to tell the user more explicitly.
,
Oct 12 2016
mitsuji@, is this that you mentioned last week? If it's correct, please assign me back.
,
Oct 12 2016
Yes that's it. weifangsun@ could you get the approved copy for this? I can help you.
,
Oct 20 2016
Is this a proposal to make the indicator more visible, or to give "why" the operation is not permitted (i.e. device write protect or forbidden by admin)? If it's the former, my preferred way is to keep current drag icon but show a modal dialog box after a user dropped the item to such destination. In this way, there will be 2 benefits: - Can use larger space to show longer messages. - Messages on a drag icon is not easy to see, especially if user releases the mouse button as soon as the cursor hovers over the destination. - Also, when dragging items using touchscreen, the message might be hidden under the user's finger and hand, or a stylus pen.
,
Oct 20 2016
@yamaguchi - Spoke with @mitsuji about this last week and we are looking to address the latter. We'd like to provide the reason why an operation is not permitted for a specific drive. I'm working to obtain an approved copy for the error cases.
,
Oct 20 2016
Got it. Here is another bug which is potentially relevant to this issue. "Pasting files to read-only folder using keyboard shortcut ignored silently" https://bugs.chromium.org/p/chromium/issues/detail?id=657821 It is not necessarily a bug, but might be another place we want to provide the reason why an operation is permitted.
,
Oct 24 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/eaaad9842eef857da9e5e14b4ee5e6ffe6fe37ef commit eaaad9842eef857da9e5e14b4ee5e6ffe6fe37ef Author: yamaguchi <yamaguchi@chromium.org> Date: Mon Oct 24 09:46:17 2016 Preserve the hardware read-only flag in Disk object. The hardware read-only flag is obtained from device enumeration and stored to Disk objects. Existing code overwrites that field when applying read-only policy. However, that information will be needed when we support remounting devices to switch between read-only and read-write mode upon policy update. After this change, Disk objects can distinguish whether a device is mounted read-only because of a read-only hardware or because of the mount option passed to Mount (for ExternalStorageReadOnly policy). TEST=Run chromeos_unittest, unit_tests, and browser_tests. BUG= 642247 , 655003 Review-Url: https://codereview.chromium.org/2440443003 Cr-Commit-Position: refs/heads/master@{#427034} [modify] https://crrev.com/eaaad9842eef857da9e5e14b4ee5e6ffe6fe37ef/chrome/browser/chromeos/extensions/file_manager/device_event_router_unittest.cc [modify] https://crrev.com/eaaad9842eef857da9e5e14b4ee5e6ffe6fe37ef/chrome/browser/chromeos/extensions/file_manager/file_manager_private_apitest.cc [modify] https://crrev.com/eaaad9842eef857da9e5e14b4ee5e6ffe6fe37ef/chrome/browser/chromeos/file_manager/volume_manager_unittest.cc [modify] https://crrev.com/eaaad9842eef857da9e5e14b4ee5e6ffe6fe37ef/chromeos/disks/disk_mount_manager.cc [modify] https://crrev.com/eaaad9842eef857da9e5e14b4ee5e6ffe6fe37ef/chromeos/disks/disk_mount_manager.h [modify] https://crrev.com/eaaad9842eef857da9e5e14b4ee5e6ffe6fe37ef/chromeos/disks/disk_mount_manager_unittest.cc [modify] https://crrev.com/eaaad9842eef857da9e5e14b4ee5e6ffe6fe37ef/chromeos/disks/mock_disk_mount_manager.cc
,
Oct 28 2016
weifangsun wrote in #6: > I'm working to obtain an approved copy for the error cases. I will start to implement the feature with a provisional UI in the meantime.
,
Nov 2 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dd19a48cddc73cdfea90e644ad05084aeb75ffb0 commit dd19a48cddc73cdfea90e644ad05084aeb75ffb0 Author: yamaguchi <yamaguchi@chromium.org> Date: Wed Nov 02 06:47:01 2016 Propagate the read_only_hardware flag of volumes to js. Only for removable disk partitions. The property will be used to distinguish between write-protect devices and restricted write access by the policy. UI code will be able to provide better messages based on that. BUG=643693, 655003 TEST=manual test. Deploy to a device and set breakpoint in backgroun_script.js at the beginning of DeviceHandler.prototype.onMountCompleted(). See event.volumeMetadata has isReadoOnlyHardware value reflecting the attached device. Review-Url: https://codereview.chromium.org/2451713002 Cr-Commit-Position: refs/heads/master@{#429223} [modify] https://crrev.com/dd19a48cddc73cdfea90e644ad05084aeb75ffb0/chrome/browser/chromeos/extensions/file_manager/file_manager_private_apitest.cc [modify] https://crrev.com/dd19a48cddc73cdfea90e644ad05084aeb75ffb0/chrome/browser/chromeos/extensions/file_manager/private_api_util.cc [modify] https://crrev.com/dd19a48cddc73cdfea90e644ad05084aeb75ffb0/chrome/browser/chromeos/file_manager/volume_manager.cc [modify] https://crrev.com/dd19a48cddc73cdfea90e644ad05084aeb75ffb0/chrome/browser/chromeos/file_manager/volume_manager.h [modify] https://crrev.com/dd19a48cddc73cdfea90e644ad05084aeb75ffb0/chrome/common/extensions/api/file_manager_private.idl [modify] https://crrev.com/dd19a48cddc73cdfea90e644ad05084aeb75ffb0/chrome/test/data/extensions/api_test/file_browser/mount_test/test.js [modify] https://crrev.com/dd19a48cddc73cdfea90e644ad05084aeb75ffb0/third_party/closure_compiler/externs/file_manager_private.js
,
Nov 15 2016
Note: A drag image attached by dataTransfer.setDragImage at start cannot be altered while dragging. Therefore we'd need a separate mechanism to do this. https://bugs.chromium.org/p/chromium/issues/detail?id=379250
,
Nov 16 2016
Here is a demo movie clip. (change: crrev.com/2503513003 ) Note that the code for the demo was tweaked by these points: - Dummy text on the label. - Always shows the label for "shared", "recent", and "available offline" folders. Also, the style of the label (or tooltip) is TBD.
,
Nov 17 2016
LGTM! Thank you for sharing the video. mitsuji@, are you ok with this?
,
Nov 18 2016
Yes thank you for this! weifangsun@ please get the copy from llevin@ and run it by UI review.
,
Nov 28 2016
We obtained the following copy from llevin@ for potential messages: - "Device is read-only" - "Access restricted" @sgabriel, would you be able to provide some mocks with the layout/style of displaying this messaging?
,
Nov 30 2016
Attaching a screenshot for code review. https://codereview.chromium.org/2503513003/diff2/150001:170001/ui/file_manager/file_manager/foreground/css/file_manager.css
,
Nov 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/39b15cae8da27648a916ef320c4e0eb2e6ca7637 commit 39b15cae8da27648a916ef320c4e0eb2e6ca7637 Author: yamaguchi <yamaguchi@chromium.org> Date: Wed Nov 30 13:25:14 2016 Show a label to describe some invalid drag-drop operations in Files app. 1. If the destination (volume or folder) is in a write-protected external storage, "Device is write-protected". 2. If the destination is in an external storage device and writing access to there is restricted, "Access restricted". 3. Otherwise, no message. <<< The design and the copy are still TBD. >>> TEST=manual test BUG= 655003 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2503513003 Cr-Commit-Position: refs/heads/master@{#435239} [modify] https://crrev.com/39b15cae8da27648a916ef320c4e0eb2e6ca7637/chrome/app/chromeos_strings.grdp [modify] https://crrev.com/39b15cae8da27648a916ef320c4e0eb2e6ca7637/chrome/browser/chromeos/extensions/file_manager/private_api_strings.cc [modify] https://crrev.com/39b15cae8da27648a916ef320c4e0eb2e6ca7637/ui/file_manager/file_manager/background/js/mock_volume_manager.js [modify] https://crrev.com/39b15cae8da27648a916ef320c4e0eb2e6ca7637/ui/file_manager/file_manager/background/js/volume_info_impl.js [modify] https://crrev.com/39b15cae8da27648a916ef320c4e0eb2e6ca7637/ui/file_manager/file_manager/background/js/volume_manager_unittest.js [modify] https://crrev.com/39b15cae8da27648a916ef320c4e0eb2e6ca7637/ui/file_manager/file_manager/background/js/volume_manager_util.js [modify] https://crrev.com/39b15cae8da27648a916ef320c4e0eb2e6ca7637/ui/file_manager/file_manager/foreground/css/file_manager.css [modify] https://crrev.com/39b15cae8da27648a916ef320c4e0eb2e6ca7637/ui/file_manager/file_manager/foreground/js/compiled_resources.gyp [add] https://crrev.com/39b15cae8da27648a916ef320c4e0eb2e6ca7637/ui/file_manager/file_manager/foreground/js/drop_effect_and_label.js [modify] https://crrev.com/39b15cae8da27648a916ef320c4e0eb2e6ca7637/ui/file_manager/file_manager/foreground/js/file_transfer_controller.js [modify] https://crrev.com/39b15cae8da27648a916ef320c4e0eb2e6ca7637/ui/file_manager/file_manager/foreground/js/main_scripts.js [modify] https://crrev.com/39b15cae8da27648a916ef320c4e0eb2e6ca7637/ui/file_manager/file_manager/foreground/js/providers_model_unittest.js [modify] https://crrev.com/39b15cae8da27648a916ef320c4e0eb2e6ca7637/ui/file_manager/file_manager/main.html
,
Nov 30 2016
,
Nov 30 2016
,
Feb 10 2017
Chrome OS version 57.0.2987.32/9202.18.0 |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by fukino@chromium.org
, Oct 12 2016