Show a label to describe some invalid drag-drop operations in Files app.

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.

TEST=manual test
BUG=655003
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/39b15cae8da27648a916ef320c4e0eb2e6ca7637
Cr-Commit-Position: refs/heads/master@{#435239}

[yamaguchi, 2016-11-15 10:08:47]

yamaguchi@chromium.org changed reviewers:
+ fukino@chromium.org

[yamaguchi, 2016-11-16 07:01:10]

Description was changed from

==========
Add messages to describe some invalid drag-drop operations in Files app.

BUG=655003
==========

to

==========
Add messages to describe some invalid drag-drop operations in Files app.

BUG=655003
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[yamaguchi, 2016-11-16 07:01:18]

The CQ bit was checked by yamaguchi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-16 07:02:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-16 07:13:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-16 07:13:29]

Dry run: Try jobs failed on following builders:
  closure_compilation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/closure_compila...)

[yamaguchi, 2016-11-16 08:36:43]

The CQ bit was checked by yamaguchi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-16 08:37:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-16 08:49:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-16 08:49:38]

Dry run: Try jobs failed on following builders:
  closure_compilation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/closure_compila...)

[yamaguchi, 2016-11-16 09:09:29]

The CQ bit was checked by yamaguchi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-16 09:09:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yamaguchi, 2016-11-16 09:13:42]

Description was changed from

==========
Add messages to describe some invalid drag-drop operations in Files app.

BUG=655003
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
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, "Read-only devide".
2. If the destination is in an external storage device and writing access to
there is restricted, "Access restricted".
3. Otherwise, no message.

TEST=manual test
BUG=655003
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[yamaguchi, 2016-11-16 09:14:18]

Description was changed from

==========
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, "Read-only devide".
2. If the destination is in an external storage device and writing access to
there is restricted, "Access restricted".
3. Otherwise, no message.

TEST=manual test
BUG=655003
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
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.

TEST=manual test
BUG=655003
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[yamaguchi, 2016-11-16 09:29:44]

The CQ bit was checked by yamaguchi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-16 09:30:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yamaguchi, 2016-11-16 09:49:54]

Revise the CL to contain the entire logic to show the message.
ptal.

[yamaguchi, 2016-11-16 09:50:02]

Revised the CL to contain the entire logic to show the message.
ptal.

[fukino, 2016-11-16 11:58:19]

Looking good.
Please mention in the description that the design and copy are TBD.

https://codereview.chromium.org/2503513003/diff/80001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/foreground/js/drop_effect_and_label.js
(right):

https://codereview.chromium.org/2503513003/diff/80001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/drop_effect_and_label.js:26: *
@private {!string}
'!' is redundant. (string is not nullable by default.)

https://codereview.chromium.org/2503513003/diff/80001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/foreground/js/file_transfer_controller.js
(right):

https://codereview.chromium.org/2503513003/diff/80001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/file_transfer_controller.js:1318: *
('none', 'move' or copy') to the current modifiers status and the
needs indent.

https://codereview.chromium.org/2503513003/diff/80001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/file_transfer_controller.js:1335: }
else if (destinationLocationInfo.volumeInfo.isReadOnlyRemovableDevice) {
We usually don't use 'else' after return statements.

https://codereview.chromium.org/2503513003/diff/80001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/file_transfer_controller.js:1338: }
else {
ditto

[commit-bot: I haz the power, 2016-11-16 12:15:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-16 12:15:43]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[yamaguchi, 2016-11-16 12:22:03]

The CQ bit was checked by yamaguchi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-16 12:22:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yamaguchi, 2016-11-16 12:33:44]

https://codereview.chromium.org/2503513003/diff/80001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/foreground/css/file_manager.css (right):

https://codereview.chromium.org/2503513003/diff/80001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/css/file_manager.css:2423: border: black
thin solid;
I think this style should be made consistent with others, and also needs UX
review.

https://codereview.chromium.org/2503513003/diff/80001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/foreground/js/drop_effect_and_label.js
(right):

https://codereview.chromium.org/2503513003/diff/80001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/drop_effect_and_label.js:26: *
@private {!string}
On 2016/11/16 11:58:18, fukino wrote:
> '!' is redundant. (string is not nullable by default.)

Done.

https://codereview.chromium.org/2503513003/diff/80001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/foreground/js/file_transfer_controller.js
(right):

https://codereview.chromium.org/2503513003/diff/80001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/file_transfer_controller.js:1318: *
('none', 'move' or copy') to the current modifiers status and the
On 2016/11/16 11:58:19, fukino wrote:
> needs indent.

Done.

https://codereview.chromium.org/2503513003/diff/80001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/file_transfer_controller.js:1335: }
else if (destinationLocationInfo.volumeInfo.isReadOnlyRemovableDevice) {
On 2016/11/16 11:58:19, fukino wrote:
> We usually don't use 'else' after return statements.

Done.

https://codereview.chromium.org/2503513003/diff/80001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/file_transfer_controller.js:1338: }
else {
On 2016/11/16 11:58:19, fukino wrote:
> ditto

Done.

[yamaguchi, 2016-11-16 12:35:06]

Description was changed from

==========
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.

TEST=manual test
BUG=655003
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
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
==========

[commit-bot: I haz the power, 2016-11-16 13:29:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-16 13:29:13]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[yamaguchi, 2016-11-21 11:51:22]

The CQ bit was checked by yamaguchi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-21 11:51:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-21 15:52:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-21 15:52:51]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[yamaguchi, 2016-11-24 06:36:08]

The CQ bit was checked by yamaguchi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-24 06:36:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yamaguchi, 2016-11-30 05:29:54]

The CQ bit was checked by yamaguchi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-30 05:30:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yamaguchi, 2016-11-30 05:45:20]

Updated the style reflecting the mock.
ptal.

Here is the screenshot with current patchset.
https://bugs.chromium.org/p/chromium/issues/detail?id=655003#c17

[fukino, 2016-11-30 06:01:14]

https://codereview.chromium.org/2503513003/diff/170001/ui/file_manager/file_m...
File ui/file_manager/file_manager/foreground/css/file_manager.css (right):

https://codereview.chromium.org/2503513003/diff/170001/ui/file_manager/file_m...
ui/file_manager/file_manager/foreground/css/file_manager.css:2419: #drop-label {
Sort alphabetically.

https://codereview.chromium.org/2503513003/diff/170001/ui/file_manager/file_m...
ui/file_manager/file_manager/foreground/css/file_manager.css:2423: margin: 0;
Is this necessary?

https://codereview.chromium.org/2503513003/diff/170001/ui/file_manager/file_m...
ui/file_manager/file_manager/foreground/css/file_manager.css:2424: border: 0;
Is this necessary?

https://codereview.chromium.org/2503513003/diff/170001/ui/file_manager/file_m...
ui/file_manager/file_manager/foreground/css/file_manager.css:2427: max-width:
320px;
Is this necessary? I suppose we use fixed strings.

https://codereview.chromium.org/2503513003/diff/170001/ui/file_manager/file_m...
ui/file_manager/file_manager/foreground/css/file_manager.css:2429: overflow:
hidden;
Is this necessary?

https://codereview.chromium.org/2503513003/diff/170001/ui/file_manager/file_m...
ui/file_manager/file_manager/foreground/css/file_manager.css:2431: font-family:
'Roboto';
Not necessary.

[yamaguchi, 2016-11-30 06:26:11]

https://codereview.chromium.org/2503513003/diff/170001/ui/file_manager/file_m...
File ui/file_manager/file_manager/foreground/css/file_manager.css (right):

https://codereview.chromium.org/2503513003/diff/170001/ui/file_manager/file_m...
ui/file_manager/file_manager/foreground/css/file_manager.css:2419: #drop-label {
On 2016/11/30 06:01:14, fukino wrote:
> Sort alphabetically.

Done.

https://codereview.chromium.org/2503513003/diff/170001/ui/file_manager/file_m...
ui/file_manager/file_manager/foreground/css/file_manager.css:2423: margin: 0;
On 2016/11/30 06:01:14, fukino wrote:
> Is this necessary?

Done.

https://codereview.chromium.org/2503513003/diff/170001/ui/file_manager/file_m...
ui/file_manager/file_manager/foreground/css/file_manager.css:2424: border: 0;
On 2016/11/30 06:01:14, fukino wrote:
> Is this necessary?

Done.

https://codereview.chromium.org/2503513003/diff/170001/ui/file_manager/file_m...
ui/file_manager/file_manager/foreground/css/file_manager.css:2427: max-width:
320px;
On 2016/11/30 06:01:14, fukino wrote:
> Is this necessary? I suppose we use fixed strings.

Done.

https://codereview.chromium.org/2503513003/diff/170001/ui/file_manager/file_m...
ui/file_manager/file_manager/foreground/css/file_manager.css:2429: overflow:
hidden;
On 2016/11/30 06:01:14, fukino wrote:
> Is this necessary?

Done.

https://codereview.chromium.org/2503513003/diff/170001/ui/file_manager/file_m...
ui/file_manager/file_manager/foreground/css/file_manager.css:2431: font-family:
'Roboto';
On 2016/11/30 06:01:14, fukino wrote:
> Not necessary.

Done.

[fukino, 2016-11-30 06:33:37]

lgtm

[yamaguchi, 2016-11-30 06:48:58]

The CQ bit was checked by yamaguchi@chromium.org

[commit-bot: I haz the power, 2016-11-30 06:49:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-30 09:21:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-30 09:21:18]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[yamaguchi, 2016-11-30 12:31:43]

The CQ bit was checked by yamaguchi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-30 12:31:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-30 13:07:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-30 13:07:59]

Dry run: This issue passed the CQ dry run.

[yamaguchi, 2016-11-30 13:21:27]

The CQ bit was checked by yamaguchi@chromium.org

[yamaguchi, 2016-11-30 13:21:28]

The patchset sent to the CQ was uploaded after l-g-t-m from fukino@chromium.org
Link to the patchset: https://codereview.chromium.org/2503513003/#ps230001
(title: "Fix test failure of FileManagerJsTest.DirectoryTreeTest.")

[commit-bot: I haz the power, 2016-11-30 13:21:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-30 13:25:17]

CQ is committing da patch.

Bot data: {"patchset_id": 230001, "attempt_start_ts": 1480512087632860,
"parent_rev": "f417ce6b561f24f93d8a8b54a1970f4eac7bad42", "commit_rev":
"a6afc354ea7e7cf3dd3aadac8b82986b888a1cc0"}

[commit-bot: I haz the power, 2016-11-30 13:25:41]

Description was changed from

==========
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
==========

to

==========
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
==========

[commit-bot: I haz the power, 2016-11-30 13:25:42]

Committed patchset #13 (id:230001)

[commit-bot: I haz the power, 2016-11-30 13:28:16]

Description was changed from

==========
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
==========

to

==========
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

Committed: https://crrev.com/39b15cae8da27648a916ef320c4e0eb2e6ca7637
Cr-Commit-Position: refs/heads/master@{#435239}
==========

[commit-bot: I haz the power, 2016-11-30 13:28:18]

Patchset 13 (id:??) landed as
https://crrev.com/39b15cae8da27648a916ef320c4e0eb2e6ca7637
Cr-Commit-Position: refs/heads/master@{#435239}

[yamaguchi, 2016-11-30 13:46:45]

Description was changed from

==========
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

Committed: https://crrev.com/39b15cae8da27648a916ef320c4e0eb2e6ca7637
Cr-Commit-Position: refs/heads/master@{#435239}
==========

to

==========
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.

TEST=manual test
BUG=655003
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/39b15cae8da27648a916ef320c4e0eb2e6ca7637
Cr-Commit-Position: refs/heads/master@{#435239}
==========

[yamaguchi, 2016-11-30 13:50:59]

On 2016/11/30 13:28:18, commit-bot: I haz the power wrote:
> Patchset 13 (id:??) landed as
> https://crrev.com/39b15cae8da27648a916ef320c4e0eb2e6ca7637
> Cr-Commit-Position: refs/heads/master@{#435239}

Note: I had missed to remove the line which says "<<< The design and the copy
are still TBD. >>>" in the description before this patch landed. However,
actually the patch contains the latest design as noted in Patch Set 11.