Files app: Loading detailed information for single file.

Files app: Loading detailed information for single file.

BUG=274045
Committed: https://crrev.com/05bd65e0abd9e1dcf7c2ee6abd24a271dd6f839e
Cr-Commit-Position: refs/heads/master@{#381672}

[ryoh, 2016-03-14 12:46:18]

ryoh@chromium.org changed reviewers:
+ fukino@chromium.org, hirono@chromium.org

[ryoh, 2016-03-14 12:46:19]

Hi, could you take a look?

I added preview panel, that shows
 - an image when user selets an image.
 - video element when user selects a video.
 - a thumbnail and audio element when user selects a audio.

And also, filesize and modification time of the selected file are displayed.

[ryoh, 2016-03-14 12:46:35]

Description was changed from

==========
[WIP] Files app: Loading detailed information for single file.

BUG=274045
==========

to

==========
Files app: Loading detailed information for single file.

BUG=274045
==========

[hirono, 2016-03-15 04:26:37]

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

https://codereview.chromium.org/1789593002/diff/60001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/css/file_manager.css:191: content:"";
nit: indent.
nit: one space between content: and "".

https://codereview.chromium.org/1789593002/diff/60001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/foreground/js/ui/single_file_details.js
(right):

https://codereview.chromium.org/1789593002/diff/60001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/ui/single_file_details.js:58:
this.metadataModel_.get([entry], SingleFileDetailsPanel.LOADING_ITEMS)
Maybe we should handle the case.

1. Selection is changed. Metadata read for file A.
2. Selection is changed. Metadata read for file B.
3. Metadata is loaded for file B.
4. Metadata is loaded for file A.

Now selection refers file B, but the metadata shown in the panel refers file A.

[ryoh, 2016-03-15 05:12:15]

Hi, I update the patch.
Could you take a look?

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

https://codereview.chromium.org/1789593002/diff/60001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/css/file_manager.css:191: content:"";
On 2016/03/15 04:26:36, hirono wrote:
> nit: indent.
> nit: one space between content: and "".

Done.

https://codereview.chromium.org/1789593002/diff/60001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/foreground/js/ui/single_file_details.js
(right):

https://codereview.chromium.org/1789593002/diff/60001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/ui/single_file_details.js:58:
this.metadataModel_.get([entry], SingleFileDetailsPanel.LOADING_ITEMS)
On 2016/03/15 04:26:36, hirono wrote:
> Maybe we should handle the case.
> 
> 1. Selection is changed. Metadata read for file A.
> 2. Selection is changed. Metadata read for file B.
> 3. Metadata is loaded for file B.
> 4. Metadata is loaded for file A.
> 
> Now selection refers file B, but the metadata shown in the panel refers file
A.

I see.
I will introduce new "ticket_" private member field and handle those cases
correctly.

[fukino, 2016-03-15 05:42:35]

https://codereview.chromium.org/1789593002/diff/60001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/foreground/js/ui/single_file_details.js
(right):

https://codereview.chromium.org/1789593002/diff/60001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/ui/single_file_details.js:156:
'contentImageTransform',
IIUC, reading metadata for contentXXXX results in downloading file from Drive
(correct?), which sounds unexpected as metadata preview pane.

[ryoh, 2016-03-15 06:24:07]

https://codereview.chromium.org/1789593002/diff/60001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/foreground/js/ui/single_file_details.js
(right):

https://codereview.chromium.org/1789593002/diff/60001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/ui/single_file_details.js:156:
'contentImageTransform',
On 2016/03/15 05:42:35, fukino wrote:
> IIUC, reading metadata for contentXXXX results in downloading file from Drive
> (correct?), which sounds unexpected as metadata preview pane.

 - Music files:
  - We need contentThumbnail to display thumbnails
  - Anyway we will download from drive for preview.
 - Video files:
  - We will download from drive for preview, but we don't need contentThumbnail.
 - Image files
   - We don't have to download files entirely.

So I will write these conditions.
Thanks!

[ryoh, 2016-03-15 12:11:45]

I update the patch.
Currently, we use thumbnails from Drive if available.
Could you take a look?

[ryoh, 2016-03-17 06:41:50]

ryoh@chromium.org changed reviewers:
- hirono@chromium.org

[fukino, 2016-03-17 07:58:30]

lgtm with a nit. Thanks!

https://codereview.chromium.org/1789593002/diff/140001/ui/file_manager/file_m...
File ui/file_manager/file_manager/foreground/js/ui/single_file_details.js
(right):

https://codereview.chromium.org/1789593002/diff/140001/ui/file_manager/file_m...
ui/file_manager/file_manager/foreground/js/ui/single_file_details.js:30: *
@param {!FileEntry} entry
nit: Add @private for private functions. (for this function and following
functioons)

[ryoh, 2016-03-17 08:10:06]

Thanks!

https://codereview.chromium.org/1789593002/diff/140001/ui/file_manager/file_m...
File ui/file_manager/file_manager/foreground/js/ui/single_file_details.js
(right):

https://codereview.chromium.org/1789593002/diff/140001/ui/file_manager/file_m...
ui/file_manager/file_manager/foreground/js/ui/single_file_details.js:30: *
@param {!FileEntry} entry
On 2016/03/17 07:58:30, fukino wrote:
> nit: Add @private for private functions. (for this function and following
> functioons)

Done.

[ryoh, 2016-03-17 08:10:11]

The CQ bit was checked by ryoh@chromium.org

[ryoh, 2016-03-17 08:10:11]

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/1789593002/#ps160001
(title: ".")

[commit-bot: I haz the power, 2016-03-17 08:10:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1789593002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1789593002/160001

[commit-bot: I haz the power, 2016-03-17 08:43:47]

Description was changed from

==========
Files app: Loading detailed information for single file.

BUG=274045
==========

to

==========
Files app: Loading detailed information for single file.

BUG=274045
==========

[commit-bot: I haz the power, 2016-03-17 08:43:48]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-03-17 08:45:24]

Description was changed from

==========
Files app: Loading detailed information for single file.

BUG=274045
==========

to

==========
Files app: Loading detailed information for single file.

BUG=274045

Committed: https://crrev.com/05bd65e0abd9e1dcf7c2ee6abd24a271dd6f839e
Cr-Commit-Position: refs/heads/master@{#381672}
==========

[commit-bot: I haz the power, 2016-03-17 08:45:25]

Patchset 9 (id:??) landed as
https://crrev.com/05bd65e0abd9e1dcf7c2ee6abd24a271dd6f839e
Cr-Commit-Position: refs/heads/master@{#381672}