Add a DOM UI version of ibus-hangul configuration dialog.

Add a DOM UI version of ibus-hangul configuration dialog.

This dialog cannot be opend from UI yet.
The execution path will be added once "Languages and Input" dialog is ready.

TEST=manually check on the netbook by uncommenting TODO lines.
BUG=chromium-os:4806
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=52279

[mazda, 2010-07-07 10:11:19]

Hi Zel,

Can I ask you to review a DOM UI version of configuration dialog?

[arv (Not doing code reviews), 2010-07-07 18:15:10]

http://codereview.chromium.org/2853032/diff/6001/7001
File chrome/browser/chromeos/dom_ui/language_hangul_options_handler.cc (right):

http://codereview.chromium.org/2853032/diff/6001/7001#newcode34
chrome/browser/chromeos/dom_ui/language_hangul_options_handler.cc:34: value +=
delimiter;
Why not send down an array instead of a character delimited string?

http://codereview.chromium.org/2853032/diff/6001/7005
File chrome/browser/resources/options.html (right):

http://codereview.chromium.org/2853032/diff/6001/7005#newcode153
chrome/browser/resources/options.html:153: <include
src="options/chromeos_language_hangul_options.html">
This will do the include

http://codereview.chromium.org/2853032/diff/6001/7007
File chrome/browser/resources/options/chromeos_language_hangul_options.js
(right):

http://codereview.chromium.org/2853032/diff/6001/7007#newcode13
chrome/browser/resources/options/chromeos_language_hangul_options.js:13:
OptionsPage.call(this, 'languageHangul', templateData.languageHangulPage,
'languageHangulPage');
> 80

[mazda, 2010-07-08 05:28:29]

Thanks for the review.

http://codereview.chromium.org/2853032/diff/6001/7001
File chrome/browser/chromeos/dom_ui/language_hangul_options_handler.cc (right):

http://codereview.chromium.org/2853032/diff/6001/7001#newcode34
chrome/browser/chromeos/dom_ui/language_hangul_options_handler.cc:34: value +=
delimiter;
On 2010/07/07 18:15:10, arv wrote:
> Why not send down an array instead of a character delimited string?

I'd like to pass the returned value to initializeValues in PrefSelect class.
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/resources/opti...
It expects each string in the array contains options value and display text
split with '|' character.

http://codereview.chromium.org/2853032/diff/6001/7005
File chrome/browser/resources/options.html (right):

http://codereview.chromium.org/2853032/diff/6001/7005#newcode153
chrome/browser/resources/options.html:153: <include
src="options/chromeos_language_hangul_options.html">
On 2010/07/07 18:15:10, arv wrote:
> This will do the include

I changed the comment not to use the tag.

http://codereview.chromium.org/2853032/diff/6001/7007
File chrome/browser/resources/options/chromeos_language_hangul_options.js
(right):

http://codereview.chromium.org/2853032/diff/6001/7007#newcode13
chrome/browser/resources/options/chromeos_language_hangul_options.js:13:
OptionsPage.call(this, 'languageHangul', templateData.languageHangulPage,
'languageHangulPage');
On 2010/07/07 18:15:10, arv wrote:
> > 80

Done.

[arv (Not doing code reviews), 2010-07-08 16:37:59]

On Wed, Jul 7, 2010 at 22:28,  <mazda@chromium.org> wrote:
> Thanks for the review.
>
>
> http://codereview.chromium.org/2853032/diff/6001/7001
> File chrome/browser/chromeos/dom_ui/language_hangul_options_handler.cc
> (right):
>
> http://codereview.chromium.org/2853032/diff/6001/7001#newcode34
> chrome/browser/chromeos/dom_ui/language_hangul_options_handler.cc:34:
> value += delimiter;
> On 2010/07/07 18:15:10, arv wrote:
>>
>> Why not send down an array instead of a character delimited string?
>
> I'd like to pass the returned value to initializeValues in PrefSelect
> class.
>
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/resources/opti...
> It expects each string in the array contains options value and display
> text split with '|' character.

Do you mind fixing that?

The options UI is pretty much work in progress and we should not fall
back on current patterns in there when we have better solutions.

> http://codereview.chromium.org/2853032/diff/6001/7005
> File chrome/browser/resources/options.html (right):
>
> http://codereview.chromium.org/2853032/diff/6001/7005#newcode153
> chrome/browser/resources/options.html:153: <include
> src="options/chromeos_language_hangul_options.html">
> On 2010/07/07 18:15:10, arv wrote:
>>
>> This will do the include
>
> I changed the comment not to use the tag.
>
> http://codereview.chromium.org/2853032/diff/6001/7007
> File
> chrome/browser/resources/options/chromeos_language_hangul_options.js
> (right):
>
> http://codereview.chromium.org/2853032/diff/6001/7007#newcode13
> chrome/browser/resources/options/chromeos_language_hangul_options.js:13:
> OptionsPage.call(this, 'languageHangul',
> templateData.languageHangulPage, 'languageHangulPage');
> On 2010/07/07 18:15:10, arv wrote:
>>
>> > 80
>
> Done.
>
> http://codereview.chromium.org/2853032/show
>

[mazda, 2010-07-13 06:01:54]

Fixed language_hangul_options_handler.cc to use an array instead of a character
delimited string.
Please take another look.

Thanks,

[arv (Not doing code reviews), 2010-07-13 16:51:07]

http://codereview.chromium.org/2853032/diff/14002/33006
File chrome/browser/resources/options/chromeos_language_hangul_options.html
(right):

http://codereview.chromium.org/2853032/diff/14002/33006#newcode2
chrome/browser/resources/options/chromeos_language_hangul_options.html:2: <div
class="section">
This was changed to <section>

http://codereview.chromium.org/2853032/diff/14002/33007
File chrome/browser/resources/options/chromeos_language_hangul_options.js
(right):

http://codereview.chromium.org/2853032/diff/14002/33007#newcode35
chrome/browser/resources/options/chromeos_language_hangul_options.js:35:
templateData.keyboardLayoutList.forEach(function (values) {
This is already handled by PrefSelect. You should update PrefSelect to take an
array of arrays instead of splitting the strings.

http://codereview.chromium.org/2853032/diff/14002/33007#newcode37
chrome/browser/resources/options/chromeos_language_hangul_options.js:37: false,
false));
You only need 2 arguments to new Option here.

[mazda, 2010-07-14 01:44:51]

Thanks for the comments.
Please take another look.

http://codereview.chromium.org/2853032/diff/14002/33006
File chrome/browser/resources/options/chromeos_language_hangul_options.html
(right):

http://codereview.chromium.org/2853032/diff/14002/33006#newcode2
chrome/browser/resources/options/chromeos_language_hangul_options.html:2: <div
class="section">
On 2010/07/13 16:51:07, arv wrote:
> This was changed to <section>

Done.

http://codereview.chromium.org/2853032/diff/14002/33007
File chrome/browser/resources/options/chromeos_language_hangul_options.js
(right):

http://codereview.chromium.org/2853032/diff/14002/33007#newcode35
chrome/browser/resources/options/chromeos_language_hangul_options.js:35:
templateData.keyboardLayoutList.forEach(function (values) {
On 2010/07/13 16:51:07, arv wrote:
> This is already handled by PrefSelect. You should update PrefSelect to take an
> array of arrays instead of splitting the strings.

Done.

http://codereview.chromium.org/2853032/diff/14002/33007#newcode37
chrome/browser/resources/options/chromeos_language_hangul_options.js:37: false,
false));
On 2010/07/13 16:51:07, arv wrote:
> You only need 2 arguments to new Option here.

Done.

[arv (Not doing code reviews), 2010-07-14 03:30:59]

LGTM

[mazda, 2010-07-14 04:01:26]

On 2010/07/14 03:30:59, arv wrote:
> LGTM

Submitted.
Thanks!