Settings Rewrite: Add Sign-in (Chrome) section

Settings Rewrite: Add Sign-in (Chrome) section

Also moves Advanced Sync Settings as a subpage of Sign-in.

This is just a shell that contains a shell. The implementation is not anywhere near done.

BUG=537405
Committed: https://crrev.com/a2f4781e19c9456f878710c6d5b95a384ba12459
Cr-Commit-Position: refs/heads/master@{#352363}

[tommycli, 2015-09-29 22:24:31]

tommycli@chromium.org changed reviewers:
+ dbeam@chromium.org

[tommycli, 2015-09-29 22:24:32]

dbeam: ptal

[tommycli, 2015-09-30 22:08:01]

On 2015/09/29 22:24:32, tommycli wrote:
> dbeam: ptal

ping

[Dan Beam, 2015-09-30 22:43:16]

https://chromiumcodereview.appspot.com/1381553003/diff/60001/chrome/browser/r...
File chrome/browser/resources/settings/settings_page/settings_router.js (right):

https://chromiumcodereview.appspot.com/1381553003/diff/60001/chrome/browser/r...
chrome/browser/resources/settings/settings_page/settings_router.js:115: url:
'/syncSetup',
is there an order to these?  can we alphabetize by URL?

https://chromiumcodereview.appspot.com/1381553003/diff/60001/chrome/browser/r...
File chrome/browser/resources/settings/signin_page/signin_page.css (right):

https://chromiumcodereview.appspot.com/1381553003/diff/60001/chrome/browser/r...
chrome/browser/resources/settings/signin_page/signin_page.css:3: * found in the
LICENSE file. */
what's the point of this crap? this is like the 10th useless CSS file. stop this

https://chromiumcodereview.appspot.com/1381553003/diff/60001/chrome/browser/r...
File chrome/browser/resources/settings/signin_page/signin_page.html (right):

https://chromiumcodereview.appspot.com/1381553003/diff/60001/chrome/browser/r...
chrome/browser/resources/settings/signin_page/signin_page.html:15:
<neon-animatable id="">
why can't we just put <neon-animatable id>?

https://chromiumcodereview.appspot.com/1381553003/diff/60001/chrome/browser/r...
File chrome/browser/resources/settings/signin_page/signin_page.js (right):

https://chromiumcodereview.appspot.com/1381553003/diff/60001/chrome/browser/r...
chrome/browser/resources/settings/signin_page/signin_page.js:20: is:
'cr-settings-signin-page',
add new pages without the cr-

[tommycli, 2015-10-01 16:44:59]

dbeam: thanks. I had to merge origin/master, so the inter-patch delta might be
confusing, btw.

https://codereview.chromium.org/1381553003/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_page/settings_router.js (right):

https://codereview.chromium.org/1381553003/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/settings_page/settings_router.js:115: url:
'/syncSetup',
On 2015/09/30 22:43:16, Dan Beam wrote:
> is there an order to these?  can we alphabetize by URL?

No current order. This delta doesn't break the ordering any further, since it's
still behind /searchEngines.

How about I make a re-ordering patch right after this?

https://codereview.chromium.org/1381553003/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/signin_page/signin_page.css (right):

https://codereview.chromium.org/1381553003/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/signin_page/signin_page.css:3: * found in the
LICENSE file. */
On 2015/09/30 22:43:16, Dan Beam wrote:
> what's the point of this crap? this is like the 10th useless CSS file. stop
this

Done.

https://codereview.chromium.org/1381553003/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/signin_page/signin_page.html (right):

https://codereview.chromium.org/1381553003/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/signin_page/signin_page.html:15:
<neon-animatable id="">
On 2015/09/30 22:43:16, Dan Beam wrote:
> why can't we just put <neon-animatable id>?

It technically works, however, I think it's less confusing to write id="", as
then we are explicitly giving it an empty string id, whereas the other way makes
it seem like it 'has a' id.

Alternatively I can change it back to id="main" for the default page in a
separate patch if the above seems confusing.

https://codereview.chromium.org/1381553003/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/signin_page/signin_page.js (right):

https://codereview.chromium.org/1381553003/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/signin_page/signin_page.js:20: is:
'cr-settings-signin-page',
On 2015/09/30 22:43:16, Dan Beam wrote:
> add new pages without the cr-

Done.

[tommycli, 2015-10-02 17:51:30]

tommycli@chromium.org changed reviewers:
+ michaelpg@chromium.org
- dbeam@chromium.org

[tommycli, 2015-10-02 17:51:30]

michaelpg: Do you mind taking over this review? Thanks!

Tommy

[michaelpg, 2015-10-03 05:23:32]

lgtm

https://codereview.chromium.org/1381553003/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/signin_page/signin_page.html (right):

https://codereview.chromium.org/1381553003/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/signin_page/signin_page.html:15:
<neon-animatable id="">
On 2015/10/01 16:44:59, tommycli wrote:
> On 2015/09/30 22:43:16, Dan Beam wrote:
> > why can't we just put <neon-animatable id>?
> 
> It technically works, however, I think it's less confusing to write id="", as
> then we are explicitly giving it an empty string id, whereas the other way
makes
> it seem like it 'has a' id.
> 
> Alternatively I can change it back to id="main" for the default page in a
> separate patch if the above seems confusing.

^ that

https://codereview.chromium.org/1381553003/diff/100001/chrome/app/settings_st...
File chrome/app/settings_strings.grdp (right):

https://codereview.chromium.org/1381553003/diff/100001/chrome/app/settings_st...
chrome/app/settings_strings.grdp:451: <!-- Sign in Page -->
ubernit: Sign-in (hyphenate prepositional verbs used as modifiers, but not
prepositional verbs used as verbs)

rationale: using correct English in comments makes incorrect English less likely
to slip into user-visible strings

[tommycli, 2015-10-05 17:15:46]

https://codereview.chromium.org/1381553003/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/signin_page/signin_page.html (right):

https://codereview.chromium.org/1381553003/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/signin_page/signin_page.html:15:
<neon-animatable id="">
On 2015/10/03 05:23:31, michaelpg (OOO til 19th) wrote:
> On 2015/10/01 16:44:59, tommycli wrote:
> > On 2015/09/30 22:43:16, Dan Beam wrote:
> > > why can't we just put <neon-animatable id>?
> > 
> > It technically works, however, I think it's less confusing to write id="",
as
> > then we are explicitly giving it an empty string id, whereas the other way
> makes
> > it seem like it 'has a' id.
> > 
> > Alternatively I can change it back to id="main" for the default page in a
> > separate patch if the above seems confusing.
> 
> ^ that

Sounds good I'll change it back to id="main" for all of them

https://codereview.chromium.org/1381553003/diff/100001/chrome/app/settings_st...
File chrome/app/settings_strings.grdp (right):

https://codereview.chromium.org/1381553003/diff/100001/chrome/app/settings_st...
chrome/app/settings_strings.grdp:451: <!-- Sign in Page -->
On 2015/10/03 05:23:32, michaelpg (OOO til 19th) wrote:
> ubernit: Sign-in (hyphenate prepositional verbs used as modifiers, but not
> prepositional verbs used as verbs)
> 
> rationale: using correct English in comments makes incorrect English less
likely
> to slip into user-visible strings

good call  :D

[tommycli, 2015-10-05 17:15:51]

The CQ bit was checked by tommycli@chromium.org

[tommycli, 2015-10-05 17:15:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/1381553003/#ps120001
(title: " ")

[commit-bot: I haz the power, 2015-10-05 17:19:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1381553003/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1381553003/120001

[commit-bot: I haz the power, 2015-10-05 18:01:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-05 18:01:22]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Dan Beam, 2015-10-05 18:02:32]

dbeam@chromium.org changed reviewers:
+ dbeam@chromium.org

[Dan Beam, 2015-10-05 18:02:33]

lgtm

[tommycli, 2015-10-05 18:03:06]

dbeam: PTAL

Looks like I still need your OWNER approval for
chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc

[tommycli, 2015-10-05 18:05:57]

The CQ bit was checked by tommycli@chromium.org

[tommycli, 2015-10-05 18:06:06]

dbeam / michaelpg: thank you all

[commit-bot: I haz the power, 2015-10-05 18:11:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1381553003/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1381553003/120001

[commit-bot: I haz the power, 2015-10-05 18:45:03]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-10-05 18:45:57]

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