Add tooltip to close button in app info dialog.

Add tooltip to close button in app info dialog.

BUG=468726
Committed: https://crrev.com/b0cdf973e32ded315880cce7e35260df542185d9
Cr-Commit-Position: refs/heads/master@{#322453}

[benwells, 2015-03-23 06:30:32]

benwells@chromium.org changed reviewers:
+ tsergeant@chromium.org

[benwells, 2015-03-23 06:30:33]

Tim, can you take a look before I send to owners?

[tsergeant, 2015-03-24 00:32:43]

https://codereview.chromium.org/1028933002/diff/1/chrome/browser/ui/views/app...
File chrome/browser/ui/views/app_list/app_list_dialog_container.cc (right):

https://codereview.chromium.org/1028933002/diff/1/chrome/browser/ui/views/app...
chrome/browser/ui/views/app_list/app_list_dialog_container.cc:211:
frame->close_button()->SetTooltipText(l10n_util::GetStringUTF16(IDS_CLOSE));
Can this be moved into BubbleFrameView directly (or even better, into
BubbleFrameView::CreateCloseButton)? I'm not sure if there's any reason why we
would want a close button which does not have a tooltip.

[benwells, 2015-03-24 01:22:52]

https://codereview.chromium.org/1028933002/diff/1/chrome/browser/ui/views/app...
File chrome/browser/ui/views/app_list/app_list_dialog_container.cc (right):

https://codereview.chromium.org/1028933002/diff/1/chrome/browser/ui/views/app...
chrome/browser/ui/views/app_list/app_list_dialog_container.cc:211:
frame->close_button()->SetTooltipText(l10n_util::GetStringUTF16(IDS_CLOSE));
On 2015/03/24 00:32:43, tsergeant wrote:
> Can this be moved into BubbleFrameView directly (or even better, into
> BubbleFrameView::CreateCloseButton)? I'm not sure if there's any reason why we
> would want a close button which does not have a tooltip.

Yeah, I'll ask the owner about that. I initially wanted to do that but the ui
layer doesn't have any strings in its resources.

[tsergeant, 2015-03-24 01:39:57]

lgtm, let's see what owners think.

[benwells, 2015-03-24 03:22:02]

benwells@chromium.org changed reviewers:
+ msw@chromium.org

[benwells, 2015-03-24 03:22:02]

msw for owners review. Please see Tim's comment about just adding the tooltip in
the bubble view.

That would be great but would need a string resource added to ui, which would
mean translation etc. Any thoughts?

[msw, 2015-03-24 17:21:57]

https://codereview.chromium.org/1028933002/diff/1/chrome/browser/ui/views/app...
File chrome/browser/ui/views/app_list/app_list_dialog_container.cc (right):

https://codereview.chromium.org/1028933002/diff/1/chrome/browser/ui/views/app...
chrome/browser/ui/views/app_list/app_list_dialog_container.cc:211:
frame->close_button()->SetTooltipText(l10n_util::GetStringUTF16(IDS_CLOSE));
On 2015/03/24 01:22:52, benwells wrote:
> On 2015/03/24 00:32:43, tsergeant wrote:
> > Can this be moved into BubbleFrameView directly (or even better, into
> > BubbleFrameView::CreateCloseButton)? I'm not sure if there's any reason why
we
> > would want a close button which does not have a tooltip.
> 
> Yeah, I'll ask the owner about that. I initially wanted to do that but the ui
> layer doesn't have any strings in its resources.

You can use IDS_APP_CLOSE from ui/strings/ui_strings.grd
Moving this to BubbleFrameView is okay with me.

[benwells, 2015-03-26 04:02:30]

https://codereview.chromium.org/1028933002/diff/1/chrome/browser/ui/views/app...
File chrome/browser/ui/views/app_list/app_list_dialog_container.cc (right):

https://codereview.chromium.org/1028933002/diff/1/chrome/browser/ui/views/app...
chrome/browser/ui/views/app_list/app_list_dialog_container.cc:211:
frame->close_button()->SetTooltipText(l10n_util::GetStringUTF16(IDS_CLOSE));
On 2015/03/24 17:21:57, msw wrote:
> On 2015/03/24 01:22:52, benwells wrote:
> > On 2015/03/24 00:32:43, tsergeant wrote:
> > > Can this be moved into BubbleFrameView directly (or even better, into
> > > BubbleFrameView::CreateCloseButton)? I'm not sure if there's any reason
why
> we
> > > would want a close button which does not have a tooltip.
> > 
> > Yeah, I'll ask the owner about that. I initially wanted to do that but the
ui
> > layer doesn't have any strings in its resources.
> 
> You can use IDS_APP_CLOSE from ui/strings/ui_strings.grd
> Moving this to BubbleFrameView is okay with me.

Awesome, somehow I didn't find the ui/strings directory.

[msw, 2015-03-26 17:35:04]

lgtm

[benwells, 2015-03-26 20:02:07]

The CQ bit was checked by benwells@chromium.org

[benwells, 2015-03-26 20:02:08]

The patchset sent to the CQ was uploaded after l-g-t-m from
tsergeant@chromium.org
Link to the patchset: https://codereview.chromium.org/1028933002/#ps20001
(title: "Do it in the bubble")

[commit-bot: I haz the power, 2015-03-26 20:02:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1028933002/20001

[commit-bot: I haz the power, 2015-03-26 20:05:52]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-03-26 20:06:36]

Patchset 2 (id:??) landed as
https://crrev.com/b0cdf973e32ded315880cce7e35260df542185d9
Cr-Commit-Position: refs/heads/master@{#322453}