First round UI touch up of DOMUI based network menu.

First round UI touch up of DOMUI based network menu.

- Update network UI to make it closer to mocks;
- Update menu.css to use default cursor;
- Allow derived menu UI pass localized strings;

BUG=chromium-os:7343
TEST=none.

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

[xiyuan, 2010-10-14 01:03:33]

Oshima, could you look at the menu base code changes? Steven could do the rest.

Thanks.

[oshima, 2010-10-14 01:40:23]

lgtm on c++ part

http://codereview.chromium.org/3771003/diff/1/5
File chrome/browser/chromeos/dom_ui/menu_ui.h (right):

http://codereview.chromium.org/3771003/diff/1/5#newcode43
chrome/browser/chromeos/dom_ui/menu_ui.h:43: virtual void
GetLocalizedValues(DictionaryValue* localized_strings) const {};
AddCustomConfigValues also handles localized string.
This seems to be a way to generate localized string in JS from template, so can
you come up with better name, and
add comment.

http://codereview.chromium.org/3771003/diff/1/6
File chrome/browser/chromeos/dom_ui/network_menu_ui.cc (right):

http://codereview.chromium.org/3771003/diff/1/6#newcode98
chrome/browser/chromeos/dom_ui/network_menu_ui.cc:98:
IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_PASSWORD));
I personally like to use the same name for both languages,
that is "IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_PASSWORD", instead of
pass_prompt. It's very clear that it came from grd even in JS and also I can use
code search to find where particular resource is used. Just a comment and no
need to change now, but I think we should discuss as this will be more important
as we move UI to domui.

[stevenjb, 2010-10-14 20:45:04]

On 2010/10/14 01:03:33, xiyuan wrote:
> Oshima, could you look at the menu base code changes? Steven could do the
rest.
> 
> Thanks.

LGTM. I'm not a good judge of JS style, but I did go through it and it looks
fine as far as I can tell.

I'm pretty sure there is at least one logic bug on my end in
NetworkMenu::GetNetworkAt() (for networks with certificates, e.g. GoogleA,
show_password should be false). I will fix that as soon as this is pushed.

[xiyuan, 2010-10-14 21:38:59]

Synced and running try jobs now.

http://codereview.chromium.org/3771003/diff/1/5
File chrome/browser/chromeos/dom_ui/menu_ui.h (right):

http://codereview.chromium.org/3771003/diff/1/5#newcode43
chrome/browser/chromeos/dom_ui/menu_ui.h:43: virtual void
GetLocalizedValues(DictionaryValue* localized_strings) const {};
On 2010/10/14 01:40:24, oshima wrote:
> AddCustomConfigValues also handles localized string.
> This seems to be a way to generate localized string in JS from template, so
can
> you come up with better name, and
> add comment.

Renamed to AddLocalizedStrings and update comment to mention this is for js
template builder and use with JS class LocalStrings.

http://codereview.chromium.org/3771003/diff/1/6
File chrome/browser/chromeos/dom_ui/network_menu_ui.cc (right):

http://codereview.chromium.org/3771003/diff/1/6#newcode98
chrome/browser/chromeos/dom_ui/network_menu_ui.cc:98:
IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_PASSWORD));
On 2010/10/14 01:40:24, oshima wrote:
> I personally like to use the same name for both languages,
> that is "IDS_OPTIONS_SETTINGS_INTERNET_OPTIONS_PASSWORD", instead of
> pass_prompt. It's very clear that it came from grd even in JS and also I can
use
> code search to find where particular resource is used. Just a comment and no
> need to change now, but I think we should discuss as this will be more
important
> as we move UI to domui.

This is a good question and I am not sure what is the right thing to do here.
From what I know, there seems to be no code style guidance on this. The short
arbitrary names are used for all other DOMUI files. It would be good that we
could come up with some agreement and fix the style. I think your suggestion is
good to avoid collisions although the IDSxxx names are a bit long.

For now, I'll just leave it like this.