Clean up mac translate bars.

Clean up mac translate bars.

Switch buttons to use the metal style, and fix up spacing a bit.

BUG=40425
TEST=none

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

[feldstein, 2010-07-02 00:18:00]

Jeremy can you take a look at this?

[jeremy, 2010-07-05 08:02:07]

http://codereview.chromium.org/2804035/diff/3001/4003
File chrome/browser/cocoa/translate/translate_infobar_base.mm (right):

http://codereview.chromium.org/2804035/diff/3001/4003#newcode420
chrome/browser/cocoa/translate/translate_infobar_base.mm:420: NSArray*
metalPopups = [NSArray arrayWithObjects:optionsPopUp_.get(),
Can we add a [- allControls ] method and iterate over it, combining these loops
into one look which just toggles on the class of each object?

A common function will allow you to reuse the list in [- verifyLayout bellow.]

Also, can you add a block comment here to match the rest of this fuction.

// Tweak style for popup and button controls, or somesuch ?

[feldstein, 2010-07-07 22:30:38]

http://codereview.chromium.org/2804035/diff/3001/4003
File chrome/browser/cocoa/translate/translate_infobar_base.mm (right):

http://codereview.chromium.org/2804035/diff/3001/4003#newcode420
chrome/browser/cocoa/translate/translate_infobar_base.mm:420: NSArray*
metalPopups = [NSArray arrayWithObjects:optionsPopUp_.get(),
On 2010/07/05 08:02:07, jeremy wrote:
> Can we add a [- allControls ] method and iterate over it, combining these
loops
> into one look which just toggles on the class of each object?
> 
> A common function will allow you to reuse the list in [- verifyLayout bellow.]
> 
> Also, can you add a block comment here to match the rest of this fuction.
> 
> // Tweak style for popup and button controls, or somesuch ?

Done.

[feldstein, 2010-07-09 02:05:42]

Hey Jeremy,
I added another fix into this.  There was an issue where if the window was
really small the options button would overlap the other buttons and it would
look bad.  I added code to shrink the option button if possible, and hide it if
not so it doesn't overlap anymore.

Jay, I think we will need to do this for windows and linux too.

[jeremy, 2010-07-11 08:35:12]

AFAIK we don't generally handle the case of the infobars behaving well when
shrinking the Window.

But kudos for taking care of this case!

http://codereview.chromium.org/2804035/diff/22001/23003
File chrome/browser/cocoa/translate/translate_infobar_base.h (right):

http://codereview.chromium.org/2804035/diff/22001/23003#newcode90
chrome/browser/cocoa/translate/translate_infobar_base.h:90: -
(void)adjustOptionsButtonToView:(NSView*)lastView;
The method name here isn't really clear, how about:
adjustOptionsButtonSizeAndVisibilityForView:... ?

http://codereview.chromium.org/2804035/diff/22001/23003#newcode127
chrome/browser/cocoa/translate/translate_infobar_base.h:127: // Teardown and
rebuild the options menu.
Could you add a comment about the hideTitle parameter.

http://codereview.chromium.org/2804035/diff/22001/23004
File chrome/browser/cocoa/translate/translate_infobar_base.mm (right):

http://codereview.chromium.org/2804035/diff/22001/23004#newcode301
chrome/browser/cocoa/translate/translate_infobar_base.mm:301: NSString*
optionsLabel = hideTitle ? @"" :
Can you use [optionsPopupUp isHidden] instead of passing in a paramter?

http://codereview.chromium.org/2804035/diff/22001/23004#newcode476
chrome/browser/cocoa/translate/translate_infobar_base.mm:476: -
(void)didChangeFrame:(NSNotification*)notification {
Can you add this to a category at the top of the file with a comment?

[feldstein, 2010-07-13 00:02:39]

ptal. I wasn't particularly sure about the decision for putting the
didChangeFrame: method in a category, was that done correctly, or was i supposed
to put an interface somewhere first.

http://codereview.chromium.org/2804035/diff/22001/23003
File chrome/browser/cocoa/translate/translate_infobar_base.h (right):

http://codereview.chromium.org/2804035/diff/22001/23003#newcode90
chrome/browser/cocoa/translate/translate_infobar_base.h:90: -
(void)adjustOptionsButtonToView:(NSView*)lastView;
On 2010/07/11 08:35:12, jeremy wrote:
> The method name here isn't really clear, how about:
> adjustOptionsButtonSizeAndVisibilityForView:... ?

Done.

http://codereview.chromium.org/2804035/diff/22001/23003#newcode127
chrome/browser/cocoa/translate/translate_infobar_base.h:127: // Teardown and
rebuild the options menu.
On 2010/07/11 08:35:12, jeremy wrote:
> Could you add a comment about the hideTitle parameter.

Done.

http://codereview.chromium.org/2804035/diff/22001/23004
File chrome/browser/cocoa/translate/translate_infobar_base.mm (right):

http://codereview.chromium.org/2804035/diff/22001/23004#newcode301
chrome/browser/cocoa/translate/translate_infobar_base.mm:301: NSString*
optionsLabel = hideTitle ? @"" :
On 2010/07/11 08:35:12, jeremy wrote:
> Can you use [optionsPopupUp isHidden] instead of passing in a paramter?

No, the popup isn't hidden, it's just made smaller so we need to hide the title.

http://codereview.chromium.org/2804035/diff/22001/23004#newcode476
chrome/browser/cocoa/translate/translate_infobar_base.mm:476: -
(void)didChangeFrame:(NSNotification*)notification {
On 2010/07/11 08:35:12, jeremy wrote:
> Can you add this to a category at the top of the file with a comment?

Done.

[jeremy, 2010-07-13 06:06:03]

LGTM