Fix the tooltip for thumbnail view option in gallery on tabbing

Fix the tooltip for thumbnail view option in gallery on tabbing

Previously, when tab button was clicked up to thumbnail view option in
gallery, the tooltip for the option did not work. This is also true of
slide view option in thumbnail mode. Both issues are resolved with this
patch.

BUG=549547
TEST=manually tested in the issue
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2626973004
Cr-Commit-Position: refs/heads/master@{#443156}
Committed: https://chromium.googlesource.com/chromium/src/+/0c13a0b61a5ca9950be7796af8c480429b03e87a

[takise, 2017-01-12 01:28:25]

Description was changed from

==========
Fix the tooltip for thumbnail view option in gallery on tabbing

Previously, when tab button was clicked up to thumbnail view option in
gallery, the tooltip for the option did not work. This is also true of
slide view option in thumbnail mode. Both issues are resolved with this
patch.

BUG=549547
TEST=manually tested in the issue
==========

to

==========
Fix the tooltip for thumbnail view option in gallery on tabbing

Previously, when tab button was clicked up to thumbnail view option in
gallery, the tooltip for the option did not work. This is also true of
slide view option in thumbnail mode. Both issues are resolved with this
patch.

BUG=549547
TEST=manually tested in the issue
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[takise, 2017-01-12 01:32:17]

takise@google.com changed reviewers:
+ oka@chromium.org

[takise, 2017-01-12 01:32:18]

PTAL

[oka, 2017-01-12 01:59:25]

https://codereview.chromium.org/2626973004/diff/1/ui/file_manager/gallery/js/...
File ui/file_manager/gallery/js/gallery.js (right):

https://codereview.chromium.org/2626973004/diff/1/ui/file_manager/gallery/js/...
ui/file_manager/gallery/js/gallery.js:545: */
Add a space before *.

https://codereview.chromium.org/2626973004/diff/1/ui/file_manager/gallery/js/...
ui/file_manager/gallery/js/gallery.js:546: 
Remove this empty line.

https://codereview.chromium.org/2626973004/diff/1/ui/file_manager/gallery/js/...
ui/file_manager/gallery/js/gallery.js:548: var button = event.currentTarget;
Are you sure that event is not undefined? If event is undefined, this line
causes error.
I guess you can use this.modeSwitchButton_ here instead.

https://codereview.chromium.org/2626973004/diff/1/ui/file_manager/gallery/js/...
ui/file_manager/gallery/js/gallery.js:584:
this.updateModeButtonAttribute_(opt_event);
I think this line should be moved to setCurrentMode_.
The comment of setCurrentMode_ reads "Sets the current mode, update the UI.". It
looks the appropriate place to update the button attribute.

[oka, 2017-01-12 02:15:25]

Description was changed from

==========
Fix the tooltip for thumbnail view option in gallery on tabbing

Previously, when tab button was clicked up to thumbnail view option in
gallery, the tooltip for the option did not work. This is also true of
slide view option in thumbnail mode. Both issues are resolved with this
patch.

BUG=549547
TEST=manually tested in the issue
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Fix the tooltip for thumbnail view option in gallery on tabbing

Previously, when tab button was clicked up to thumbnail view option in
gallery, the tooltip for the option did not work. This is also true of
slide view option in thumbnail mode. Both issues are resolved with this
patch.

BUG=549547
TEST=manually tested in the issue
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[takise, 2017-01-12 04:08:05]

PTAL

https://codereview.chromium.org/2626973004/diff/1/ui/file_manager/gallery/js/...
File ui/file_manager/gallery/js/gallery.js (right):

https://codereview.chromium.org/2626973004/diff/1/ui/file_manager/gallery/js/...
ui/file_manager/gallery/js/gallery.js:545: */
On 2017/01/12 01:59:24, oka wrote:
> Add a space before *.

Done.

https://codereview.chromium.org/2626973004/diff/1/ui/file_manager/gallery/js/...
ui/file_manager/gallery/js/gallery.js:546: 
On 2017/01/12 01:59:24, oka wrote:
> Remove this empty line.

Done.

https://codereview.chromium.org/2626973004/diff/1/ui/file_manager/gallery/js/...
ui/file_manager/gallery/js/gallery.js:548: var button = event.currentTarget;
On 2017/01/12 01:59:24, oka wrote:
> Are you sure that event is not undefined? If event is undefined, this line
> causes error.
> I guess you can use this.modeSwitchButton_ here instead.

Done.

https://codereview.chromium.org/2626973004/diff/1/ui/file_manager/gallery/js/...
ui/file_manager/gallery/js/gallery.js:584:
this.updateModeButtonAttribute_(opt_event);
On 2017/01/12 01:59:24, oka wrote:
> I think this line should be moved to setCurrentMode_.
> The comment of setCurrentMode_ reads "Sets the current mode, update the UI.".
It
> looks the appropriate place to update the button attribute.

Done.

[oka, 2017-01-12 04:20:35]

lgtm

[oka, 2017-01-12 04:20:49]

On 2017/01/12 04:20:35, oka wrote:
> lgtm

Thank you!

[takise, 2017-01-12 04:21:26]

The CQ bit was checked by takise@google.com

[commit-bot: I haz the power, 2017-01-12 04:21:40]

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

[oka, 2017-01-12 04:34:27]

The CQ bit was unchecked by oka@chromium.org

[takise, 2017-01-12 04:35:18]

The CQ bit was unchecked by takise@google.com

[takise, 2017-01-12 04:35:46]

The CQ bit was checked by takise@google.com

[commit-bot: I haz the power, 2017-01-12 04:36:05]

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, 2017-01-12 04:52:02]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484195746476200,
"parent_rev": "30b506970f7903f7434dad86568fc90633dadce1", "commit_rev":
"0c13a0b61a5ca9950be7796af8c480429b03e87a"}

[commit-bot: I haz the power, 2017-01-12 04:52:33]

Description was changed from

==========
Fix the tooltip for thumbnail view option in gallery on tabbing

Previously, when tab button was clicked up to thumbnail view option in
gallery, the tooltip for the option did not work. This is also true of
slide view option in thumbnail mode. Both issues are resolved with this
patch.

BUG=549547
TEST=manually tested in the issue
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Fix the tooltip for thumbnail view option in gallery on tabbing

Previously, when tab button was clicked up to thumbnail view option in
gallery, the tooltip for the option did not work. This is also true of
slide view option in thumbnail mode. Both issues are resolved with this
patch.

BUG=549547
TEST=manually tested in the issue
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2626973004
Cr-Commit-Position: refs/heads/master@{#443156}
Committed:
https://chromium.googlesource.com/chromium/src/+/0c13a0b61a5ca9950be7796af8c4...
==========

[commit-bot: I haz the power, 2017-01-12 04:52:34]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/0c13a0b61a5ca9950be7796af8c4...

[oka, 2017-01-12 04:53:33]

Conguratulations for your first patch!

On Thu, Jan 12, 2017 at 1:52 PM commit-bot@chromium.org via
codereview.chromium.org <reply@chromiumcodereview-hr.appspotmail.com> wrote:

> Committed patchset #2 (id:20001) as
>
>
https://chromium.googlesource.com/chromium/src/+/0c13a0b61a5ca9950be7796af8c4...
>
> https://codereview.chromium.org/2626973004/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2017-01-12 04:59:55]

Thank you Oka-san for your kind help!

On Thu, Jan 12, 2017 at 1:53 PM, Keigo Oka <oka@chromium.org> wrote:

> Conguratulations for your first patch!
>
> On Thu, Jan 12, 2017 at 1:52 PM commit-bot@chromium.org via
> codereview.chromium.org <reply@chromiumcodereview-hr.appspotmail.com>
> wrote:
>
>> Committed patchset #2 (id:20001) as
>> https://chromium.googlesource.com/chromium/src/+/
>> 0c13a0b61a5ca9950be7796af8c480429b03e87a
>>
>> https://codereview.chromium.org/2626973004/
>>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.