[filebrowser] Enable sharing in gallery. For now, show all image-related extensions as 'Sharing'.

[filebrowser] Enable sharing in gallery. For now, show all image-related extensions as 'Sharing'.
Also, a little refactoring in file tasks execution.

BUG=chromium-os:19534
TEST=See bug.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=102937

[dgozman, 2011-09-26 13:03:44]

May you have a look, please?

[Vladislav Kaznacheev, 2011-09-27 08:31:07]

http://codereview.chromium.org/8036019/diff/2001/chrome/browser/resources/fil...
File chrome/browser/resources/file_manager/js/image_editor/gallery.css (right):

http://codereview.chromium.org/8036019/diff/2001/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/js/image_editor/gallery.css:311: opacity:
0.0;
I suspect that a transparent div will consume mouse events potentially blocking
part of the image (if the image is really wide). Add pointer-events: none?

http://codereview.chromium.org/8036019/diff/2001/chrome/browser/resources/fil...
File chrome/browser/resources/file_manager/js/image_editor/gallery.js (right):

http://codereview.chromium.org/8036019/diff/2001/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/js/image_editor/gallery.js:95: if
(shareActions.length > 0) {
Why not hide everything the above 4 lines behind the condition?

http://codereview.chromium.org/8036019/diff/2001/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/js/image_editor/gallery.js:109:
this.shareMenu_ = doc.createElement('div');
Why create a menu if shareActions is empty?

http://codereview.chromium.org/8036019/diff/2001/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/js/image_editor/gallery.js:214:
action.execute([item.getUrl()]);
This would not work if the item has been edited but not saved. 
getUrl will give you you the old (pre-edit) url. You need to call saveChanges_
and invoke the action from the completion callback.

[dgozman, 2011-09-27 12:36:28]

http://codereview.chromium.org/8036019/diff/2001/chrome/browser/resources/fil...
File chrome/browser/resources/file_manager/js/image_editor/gallery.css (right):

http://codereview.chromium.org/8036019/diff/2001/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/js/image_editor/gallery.css:311: opacity:
0.0;
On 2011/09/27 08:31:07, Vladislav Kaznacheev wrote:
> I suspect that a transparent div will consume mouse events potentially
blocking
> part of the image (if the image is really wide). Add pointer-events: none?

Done.

http://codereview.chromium.org/8036019/diff/2001/chrome/browser/resources/fil...
File chrome/browser/resources/file_manager/js/image_editor/gallery.js (right):

http://codereview.chromium.org/8036019/diff/2001/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/js/image_editor/gallery.js:95: if
(shareActions.length > 0) {
On 2011/09/27 08:31:07, Vladislav Kaznacheev wrote:
> Why not hide everything the above 4 lines behind the condition?

this.shareButton_ is referenced through the code, and I don't want to add checks
there. So, it's created here, but not shown.

http://codereview.chromium.org/8036019/diff/2001/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/js/image_editor/gallery.js:109:
this.shareMenu_ = doc.createElement('div');
On 2011/09/27 08:31:07, Vladislav Kaznacheev wrote:
> Why create a menu if shareActions is empty?

Same as for shareButton. However, I guard appending it to container with the
same array length check.

http://codereview.chromium.org/8036019/diff/2001/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/js/image_editor/gallery.js:214:
action.execute([item.getUrl()]);
On 2011/09/27 08:31:07, Vladislav Kaznacheev wrote:
> This would not work if the item has been edited but not saved. 
> getUrl will give you you the old (pre-edit) url. You need to call saveChanges_
> and invoke the action from the completion callback.

Done.

[Vladislav Kaznacheev, 2011-09-27 14:10:52]

LGTM

On 2011/09/27 12:36:28, dgozman wrote:
>
http://codereview.chromium.org/8036019/diff/2001/chrome/browser/resources/fil...
> File chrome/browser/resources/file_manager/js/image_editor/gallery.css
(right):
> 
>
http://codereview.chromium.org/8036019/diff/2001/chrome/browser/resources/fil...
> chrome/browser/resources/file_manager/js/image_editor/gallery.css:311:
opacity:
> 0.0;
> On 2011/09/27 08:31:07, Vladislav Kaznacheev wrote:
> > I suspect that a transparent div will consume mouse events potentially
> blocking
> > part of the image (if the image is really wide). Add pointer-events: none?
> 
> Done.
> 
>
http://codereview.chromium.org/8036019/diff/2001/chrome/browser/resources/fil...
> File chrome/browser/resources/file_manager/js/image_editor/gallery.js (right):
> 
>
http://codereview.chromium.org/8036019/diff/2001/chrome/browser/resources/fil...
> chrome/browser/resources/file_manager/js/image_editor/gallery.js:95: if
> (shareActions.length > 0) {
> On 2011/09/27 08:31:07, Vladislav Kaznacheev wrote:
> > Why not hide everything the above 4 lines behind the condition?
> 
> this.shareButton_ is referenced through the code, and I don't want to add
checks
> there. So, it's created here, but not shown.
> 
>
http://codereview.chromium.org/8036019/diff/2001/chrome/browser/resources/fil...
> chrome/browser/resources/file_manager/js/image_editor/gallery.js:109:
> this.shareMenu_ = doc.createElement('div');
> On 2011/09/27 08:31:07, Vladislav Kaznacheev wrote:
> > Why create a menu if shareActions is empty?
> 
> Same as for shareButton. However, I guard appending it to container with the
> same array length check.
> 
>
http://codereview.chromium.org/8036019/diff/2001/chrome/browser/resources/fil...
> chrome/browser/resources/file_manager/js/image_editor/gallery.js:214:
> action.execute([item.getUrl()]);
> On 2011/09/27 08:31:07, Vladislav Kaznacheev wrote:
> > This would not work if the item has been edited but not saved. 
> > getUrl will give you you the old (pre-edit) url. You need to call
saveChanges_
> > and invoke the action from the completion callback.
> 
> Done.

[commit-bot: I haz the power, 2011-09-27 14:34:32]

CQ is trying tha patch. Follow status at
https://chromium-status.appspot.com/cq/dgozman@chromium.org/8036019/7001

[commit-bot: I haz the power, 2011-09-27 15:53:55]

Change committed as 102937