Sync: Refresh the UI to match Alex's mocks.

Sync: Refresh the UI to match Alex's mocks.

BUG=none
TEST=none

R=csilv@chromium.org

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

[csilv, 2011-04-16 01:03:47]

LGTM

[csilv, 2011-04-16 01:03:55]

http://codereview.chromium.org/6865030/diff/1/chrome/app/generated_resources.grd
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/6865030/diff/1/chrome/app/generated_resources....
chrome/app/generated_resources.grd:8660: <message
name="IDS_SYNC_CONFIRM_SYNC_PREFERENCES" desc="The title of the 'Confirm Sync
Preferences' page of the customize sync dialog.">
Should this be titlecase for Mac/Linux only?

http://codereview.chromium.org/6865030/diff/1/chrome/browser/sync/resources/c...
File chrome/browser/sync/resources/configure.html (right):

http://codereview.chromium.org/6865030/diff/1/chrome/browser/sync/resources/c...
chrome/browser/sync/resources/configure.html:236: <script
src="chrome://resources/js/cr.js"></script>
optional: consider including util.js for the $(id) function.  Maybe a future
TODO item.

http://codereview.chromium.org/6865030/diff/1/chrome/browser/sync/resources/c...
chrome/browser/sync/resources/configure.html:262: if ((keepEverythingSynced !=
undefined && !keepEverythingSynced) ||
optional: this could be simplified to:
if (keepEverythingSynced !== true || usePassphrase)

http://codereview.chromium.org/6865030/diff/1/chrome/browser/sync/resources/c...
chrome/browser/sync/resources/configure.html:363:
document.getElementById("sync-custom-passphrase").style.display = "none";
use '.hidden = true' instead?

http://codereview.chromium.org/6865030/diff/1/chrome/browser/sync/resources/c...
chrome/browser/sync/resources/configure.html:413:
//document.getElementById("aborted_text").className = "sync-error-hide";
Consider removing unused code.

http://codereview.chromium.org/6865030/diff/1/chrome/browser/sync/resources/c...
chrome/browser/sync/resources/configure.html:451: else if (mode =="explicit")
If these are the only two modes, then consider simplifying to:
  document.getElementById("sync-custom-passphrase").hidden = (mode == "google")

http://codereview.chromium.org/6865030/diff/1/chrome/browser/sync/resources/c...
chrome/browser/sync/resources/configure.html:621: <input id="passphrase"
type="password" i18n-values="placeholder:passphraseLabel">
wrap to 80 cols

http://codereview.chromium.org/6865030/diff/1/chrome/browser/sync/resources/c...
chrome/browser/sync/resources/configure.html:624: <input id="confirm-passphrase"
type="password" i18n-values="placeholder:confirmLabel">
wrap to 80 cols

http://codereview.chromium.org/6865030/diff/1/chrome/browser/sync/sync_setup_...
File chrome/browser/sync/sync_setup_wizard.cc (right):

http://codereview.chromium.org/6865030/diff/1/chrome/browser/sync/sync_setup_...
chrome/browser/sync/sync_setup_wizard.cc:136: AddString(
okay, totally picky...  the new lines 136, 164, and 169 each employ a different
wrapping method.  consider choosing one... personally i like 164.

[James Hawkins, 2011-04-17 19:05:05]

http://codereview.chromium.org/6865030/diff/1/chrome/app/generated_resources.grd
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/6865030/diff/1/chrome/app/generated_resources....
chrome/app/generated_resources.grd:8660: <message
name="IDS_SYNC_CONFIRM_SYNC_PREFERENCES" desc="The title of the 'Confirm Sync
Preferences' page of the customize sync dialog.">
On 2011/04/16 01:03:55, csilv wrote:
> Should this be titlecase for Mac/Linux only?

Sure. Done.

http://codereview.chromium.org/6865030/diff/1/chrome/browser/sync/resources/c...
File chrome/browser/sync/resources/configure.html (right):

http://codereview.chromium.org/6865030/diff/1/chrome/browser/sync/resources/c...
chrome/browser/sync/resources/configure.html:236: <script
src="chrome://resources/js/cr.js"></script>
On 2011/04/16 01:03:55, csilv wrote:
> optional: consider including util.js for the $(id) function.  Maybe a future
> TODO item.

Added the TODO.

http://codereview.chromium.org/6865030/diff/1/chrome/browser/sync/resources/c...
chrome/browser/sync/resources/configure.html:262: if ((keepEverythingSynced !=
undefined && !keepEverythingSynced) ||
On 2011/04/16 01:03:55, csilv wrote:
> optional: this could be simplified to:
> if (keepEverythingSynced !== true || usePassphrase)

Done.

http://codereview.chromium.org/6865030/diff/1/chrome/browser/sync/resources/c...
chrome/browser/sync/resources/configure.html:363:
document.getElementById("sync-custom-passphrase").style.display = "none";
On 2011/04/16 01:03:55, csilv wrote:
> use '.hidden = true' instead?

Done.

http://codereview.chromium.org/6865030/diff/1/chrome/browser/sync/resources/c...
chrome/browser/sync/resources/configure.html:413:
//document.getElementById("aborted_text").className = "sync-error-hide";
On 2011/04/16 01:03:55, csilv wrote:
> Consider removing unused code.

Fixed it up so we're using it again.

http://codereview.chromium.org/6865030/diff/1/chrome/browser/sync/resources/c...
chrome/browser/sync/resources/configure.html:451: else if (mode =="explicit")
On 2011/04/16 01:03:55, csilv wrote:
> If these are the only two modes, then consider simplifying to:
>   document.getElementById("sync-custom-passphrase").hidden = (mode ==
"google")

Blah I hate this code. I'm going to leave it as is until the cleanup so I can
get feedback on what was ultimately intended here.

http://codereview.chromium.org/6865030/diff/1/chrome/browser/sync/resources/c...
chrome/browser/sync/resources/configure.html:621: <input id="passphrase"
type="password" i18n-values="placeholder:passphraseLabel">
On 2011/04/16 01:03:55, csilv wrote:
> wrap to 80 cols

Done.

http://codereview.chromium.org/6865030/diff/1/chrome/browser/sync/resources/c...
chrome/browser/sync/resources/configure.html:624: <input id="confirm-passphrase"
type="password" i18n-values="placeholder:confirmLabel">
On 2011/04/16 01:03:55, csilv wrote:
> wrap to 80 cols

Done.

http://codereview.chromium.org/6865030/diff/1/chrome/browser/sync/sync_setup_...
File chrome/browser/sync/sync_setup_wizard.cc (right):

http://codereview.chromium.org/6865030/diff/1/chrome/browser/sync/sync_setup_...
chrome/browser/sync/sync_setup_wizard.cc:136: AddString(
On 2011/04/16 01:03:55, csilv wrote:
> okay, totally picky...  the new lines 136, 164, and 169 each employ a
different
> wrapping method.  consider choosing one... personally i like 164.

Made 164 like 136; 169 can't be wrapped the same because the args don't all fit
one on line at any point.

[James Hawkins, 2011-04-18 03:00:08]

The red builders are all a problem with nacl (unrelated); landing.