Implement autofill Address and Credit Card lists.

Implement autofill Address and Credit Card lists.

This CL includes tests to verify UI.
Regenerates the private API in order to ensure closure compile.
API is not called to populate UI w/ real data, will be followup CL.

BUG=607348, 607347
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/83216ec07f04aa199a9de9a6e4a66c45696df84c
Cr-Commit-Position: refs/heads/master@{#394032}

[hcarmona, 2016-05-04 23:52:25]

Description was changed from

==========
Implement autofill Address and Credit Card lists.

This CL includes tests to verify UI.
Regenerates the private API in order to ensure closure compile.
API is not called to populate UI w/ real data, will be followup CL.

BUG=607348,607347
==========

to

==========
Implement autofill Address and Credit Card lists.

This CL includes tests to verify UI.
Regenerates the private API in order to ensure closure compile.
API is not called to populate UI w/ real data, will be followup CL.

BUG=607348,607347
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[hcarmona, 2016-05-05 19:44:14]

Patchset #1 (id:1) has been deleted

[hcarmona, 2016-05-05 19:54:43]

Description was changed from

==========
Implement autofill Address and Credit Card lists.

This CL includes tests to verify UI.
Regenerates the private API in order to ensure closure compile.
API is not called to populate UI w/ real data, will be followup CL.

BUG=607348,607347
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Implement autofill Address and Credit Card lists.

This CL includes tests to verify UI.
Regenerates the private API in order to ensure closure compile.
API is not called to populate UI w/ real data, will be followup CL.

BUG=607348,607347
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[hcarmona, 2016-05-05 19:54:43]

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

[hcarmona, 2016-05-05 19:55:57]

PTAL, thanks!

[Dan Beam, 2016-05-06 22:01:46]

lgtm

https://codereview.chromium.org/1947293002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.html
(right):

https://codereview.chromium.org/1947293002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.html:11:
<style>
fun fact: these 3 lines could be:

<style include="settings-shared passwords-shared">
  ...
</style>

https://codereview.chromium.org/1947293002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.html:13:
border-top: 1px solid #e0e0e0;
can we use a --paper or --google var instead?

https://codereview.chromium.org/1947293002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.html:54:
<iron-list id="creditCardList" items="[[creditCards]]"
indent off

https://codereview.chromium.org/1947293002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.js
(right):

https://codereview.chromium.org/1947293002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.js:48:
return item.expirationMonth + '/' + item.expirationYear;
is this international?

https://codereview.chromium.org/1947293002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html
(right):

https://codereview.chromium.org/1947293002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html:13:
<style>
same fun fact :P

https://codereview.chromium.org/1947293002/diff/20001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/settings_autofill_section_browsertest.js
(right):

https://codereview.chromium.org/1947293002/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/settings/settings_autofill_section_browsertest.js:62: *
@return {!string}
string?

https://codereview.chromium.org/1947293002/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/settings/settings_autofill_section_browsertest.js:78:
ret.metadata = { isLocal: true, };
{isLocal: true},
^^           ^^ no spaces

[hcarmona, 2016-05-07 01:01:25]

Rebase + fix issues.

https://codereview.chromium.org/1947293002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.html
(right):

https://codereview.chromium.org/1947293002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.html:11:
<style>
On 2016/05/06 22:01:46, Dan Beam wrote:
> fun fact: these 3 lines could be:
> 
> <style include="settings-shared passwords-shared">
>   ...
> </style>

Nice! Done.

https://codereview.chromium.org/1947293002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.html:13:
border-top: 1px solid #e0e0e0;
On 2016/05/06 22:01:45, Dan Beam wrote:
> can we use a --paper or --google var instead?

Done.

https://codereview.chromium.org/1947293002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.html:54:
<iron-list id="creditCardList" items="[[creditCards]]"
On 2016/05/06 22:01:45, Dan Beam wrote:
> indent off

Done.

https://codereview.chromium.org/1947293002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.js
(right):

https://codereview.chromium.org/1947293002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.js:48:
return item.expirationMonth + '/' + item.expirationYear;
On 2016/05/06 22:01:46, Dan Beam wrote:
> is this international?

Old settings uses MM/YYYY format also. There's no i18n logic.

https://code.google.com/p/chromium/codesearch#chromium/src/components/autofil...

https://codereview.chromium.org/1947293002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html
(right):

https://codereview.chromium.org/1947293002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html:13:
<style>
On 2016/05/06 22:01:46, Dan Beam wrote:
> same fun fact :P

Done.

https://codereview.chromium.org/1947293002/diff/20001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/settings_autofill_section_browsertest.js
(right):

https://codereview.chromium.org/1947293002/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/settings/settings_autofill_section_browsertest.js:62: *
@return {!string}
On 2016/05/06 22:01:46, Dan Beam wrote:
> string?

Done.

https://codereview.chromium.org/1947293002/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/settings/settings_autofill_section_browsertest.js:78:
ret.metadata = { isLocal: true, };
On 2016/05/06 22:01:46, Dan Beam wrote:
> {isLocal: true},
> ^^           ^^ no spaces

Done.

[Dan Beam, 2016-05-09 17:48:46]

https://codereview.chromium.org/1947293002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.js
(right):

https://codereview.chromium.org/1947293002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.js:48:
return item.expirationMonth + '/' + item.expirationYear;
On 2016/05/07 01:01:25, Hector Carmona wrote:
> On 2016/05/06 22:01:46, Dan Beam wrote:
> > is this international?
> 
> Old settings uses MM/YYYY format also. There's no i18n logic.
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/components/autofil...

that's fine, there's just a ton of variations in Autofill itself:
https://code.google.com/p/chromium/codesearch#chromium/src/components/autofil...

let's just port what was there first

https://codereview.chromium.org/1947293002/diff/40001/chrome/browser/resource...
File
chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.html
(right):

https://codereview.chromium.org/1947293002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.html:43:
<a is="action-link" on-tap="onAddAddressTap_">$i18n{addAddress}</a>
where is action_link.js included?

https://codereview.chromium.org/1947293002/diff/40001/chrome/browser/resource...
File
chrome/browser/resources/settings/passwords_and_forms_page/passwords_shared_css.html
(right):

https://codereview.chromium.org/1947293002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/passwords_and_forms_page/passwords_shared_css.html:31:
-webkit-margin-start: 56px;
this looks like a duplication of some ui control's dimensions. can that control
just be in the flow of the render tree instead?

https://codereview.chromium.org/1947293002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/passwords_and_forms_page/passwords_shared_css.html:46:
}
you should use action_link.css and/or settings-shared

[hcarmona, 2016-05-12 00:07:06]

https://codereview.chromium.org/1947293002/diff/40001/chrome/browser/resource...
File
chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.html
(right):

https://codereview.chromium.org/1947293002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/passwords_and_forms_page/autofill_section.html:43:
<a is="action-link" on-tap="onAddAddressTap_">$i18n{addAddress}</a>
On 2016/05/09 17:48:46, Dan Beam wrote:
> where is action_link.js included?

Included.

https://codereview.chromium.org/1947293002/diff/40001/chrome/browser/resource...
File
chrome/browser/resources/settings/passwords_and_forms_page/passwords_shared_css.html
(right):

https://codereview.chromium.org/1947293002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/passwords_and_forms_page/passwords_shared_css.html:31:
-webkit-margin-start: 56px;
On 2016/05/09 17:48:46, Dan Beam wrote:
> this looks like a duplication of some ui control's dimensions. can that
control
> just be in the flow of the render tree instead?

:host now has the 20px margin which updates this to 36px.
I've also moved the list and headings into a div to take advantage of
this .item-list so that there's less layout of individual elements.

https://codereview.chromium.org/1947293002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/passwords_and_forms_page/passwords_shared_css.html:46:
}
On 2016/05/09 17:48:46, Dan Beam wrote:
> you should use action_link.css and/or settings-shared

Done.

[hcarmona, 2016-05-16 20:15:58]

Friendly ping on this

[Dan Beam, 2016-05-16 22:57:25]

still lgtm

[hcarmona, 2016-05-17 01:37:49]

The CQ bit was checked by hcarmona@chromium.org

[hcarmona, 2016-05-17 01:37:50]

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

[commit-bot: I haz the power, 2016-05-17 01:38:12]

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

[commit-bot: I haz the power, 2016-05-17 02:44:49]

Description was changed from

==========
Implement autofill Address and Credit Card lists.

This CL includes tests to verify UI.
Regenerates the private API in order to ensure closure compile.
API is not called to populate UI w/ real data, will be followup CL.

BUG=607348,607347
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Implement autofill Address and Credit Card lists.

This CL includes tests to verify UI.
Regenerates the private API in order to ensure closure compile.
API is not called to populate UI w/ real data, will be followup CL.

BUG=607348,607347
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2016-05-17 02:44:51]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-05-17 02:47:29]

Description was changed from

==========
Implement autofill Address and Credit Card lists.

This CL includes tests to verify UI.
Regenerates the private API in order to ensure closure compile.
API is not called to populate UI w/ real data, will be followup CL.

BUG=607348,607347
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Implement autofill Address and Credit Card lists.

This CL includes tests to verify UI.
Regenerates the private API in order to ensure closure compile.
API is not called to populate UI w/ real data, will be followup CL.

BUG=607348,607347
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/83216ec07f04aa199a9de9a6e4a66c45696df84c
Cr-Commit-Position: refs/heads/master@{#394032}
==========

[commit-bot: I haz the power, 2016-05-17 02:47:30]

Patchset 4 (id:??) landed as
https://crrev.com/83216ec07f04aa199a9de9a6e4a66c45696df84c
Cr-Commit-Position: refs/heads/master@{#394032}

[Dan Beam, 2016-05-26 23:40:36]

https://codereview.chromium.org/1947293002/diff/80001/chrome/browser/resource...
File
chrome/browser/resources/settings/passwords_and_forms_page/passwords_shared_css.html
(right):

https://codereview.chromium.org/1947293002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/passwords_and_forms_page/passwords_shared_css.html:26:
color: var(--google-grey-600);
google-grey-600 doesn't exist, did you mean paper-grey-600?