MD Settings: On Startup, moving remove URL option under a dots menu.

MD Settings: On Startup, moving remove URL option under a dots menu.

 - Extracting new element <settings-startup-url-entry>
 - Implementing dots menu, that currently only has the "Remove" option.
 - Adding test.

This is in preparation of adding the "Edit" option under the same menu.

BUG=601514
Committed: https://crrev.com/9035bfdadfeece36231732a0dd21fbd0ae200730
Cr-Commit-Position: refs/heads/master@{#387158}

[dpapad, 2016-04-12 01:28:45]

Description was changed from

==========
MD Settings: On Startup, move remove URL option under a dots menu.

BUG=601514
==========

to

==========
MD Settings: On Startup, moving remove URL option under a dots menu.

BUG=601514
==========

[dpapad, 2016-04-12 01:28:53]

Patchset #1 (id:1) has been deleted

[dpapad, 2016-04-12 01:28:57]

Patchset #1 (id:20001) has been deleted

[dpapad, 2016-04-12 01:29:01]

Patchset #1 (id:40001) has been deleted

[dpapad, 2016-04-12 01:29:06]

Patchset #1 (id:60001) has been deleted

[dpapad, 2016-04-12 01:29:10]

Patchset #1 (id:80001) has been deleted

[dpapad, 2016-04-12 01:29:14]

Patchset #1 (id:100001) has been deleted

[dpapad, 2016-04-12 01:30:51]

Description was changed from

==========
MD Settings: On Startup, moving remove URL option under a dots menu.

BUG=601514
==========

to

==========
MD Settings: On Startup, moving remove URL option under a dots menu.

 - Extracting new element <settings-startup-url-entry>
 - Implementing dots menu, that currently only has the "Remove" option.

This is in preparation of adding the "Edit" option under the same menu.

BUG=601514
==========

[dpapad, 2016-04-12 01:31:11]

Description was changed from

==========
MD Settings: On Startup, moving remove URL option under a dots menu.

 - Extracting new element <settings-startup-url-entry>
 - Implementing dots menu, that currently only has the "Remove" option.

This is in preparation of adding the "Edit" option under the same menu.

BUG=601514
==========

to

==========
MD Settings: On Startup, moving remove URL option under a dots menu.

 - Extracting new element <settings-startup-url-entry>
 - Implementing dots menu, that currently only has the "Remove" option.
 - Adding test.

This is in preparation of adding the "Edit" option under the same menu.

BUG=601514
==========

[dpapad, 2016-04-12 01:35:05]

dpapad@chromium.org changed reviewers:
+ dschuyler@chromium.org

[dpapad, 2016-04-12 01:35:06]

Before after screenshots at http://imgur.com/a/58uuX.

[dschuyler, 2016-04-12 18:18:46]

On 2016/04/12 01:35:06, dpapad wrote:
> Before after screenshots at http://imgur.com/a/58uuX.

In the screen shots, it looks like the grey separator lines
were lost in the change.

[Dan Beam, 2016-04-12 18:23:26]

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

[Dan Beam, 2016-04-12 18:23:27]

https://codereview.chromium.org/1876283002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/settings/on_startup_page/startup_url_entry.html
(right):

https://codereview.chromium.org/1876283002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/settings/on_startup_page/startup_url_entry.html:29:
<div class="list-item">
to fix the grey lines, move class="list-item" onto the container element (i.e.
outside of this file)

and also apply the last hunk in this diff:
https://codereview.chromium.org/1875413002/diff/1/chrome/browser/resources/se...

[Dan Beam, 2016-04-12 18:23:36]

Description was changed from

==========
MD Settings: On Startup, moving remove URL option under a dots menu.

 - Extracting new element <settings-startup-url-entry>
 - Implementing dots menu, that currently only has the "Remove" option.
 - Adding test.

This is in preparation of adding the "Edit" option under the same menu.

BUG=601514
==========

to

==========
MD Settings: On Startup, moving remove URL option under a dots menu.

 - Extracting new element <settings-startup-url-entry>
 - Implementing dots menu, that currently only has the "Remove" option.
 - Adding test.

This is in preparation of adding the "Edit" option under the same menu.

BUG=601514
==========

[Dan Beam, 2016-04-12 18:23:36]

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

[Dan Beam, 2016-04-12 18:23:57]

https://codereview.chromium.org/1876283002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/settings/on_startup_page/startup_urls_page.html
(right):

https://codereview.chromium.org/1876283002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/settings/on_startup_page/startup_urls_page.html:14:
<settings-startup-url-entry model="[[item]]">
class="list-item"

[dpapad, 2016-04-12 18:24:40]

On 2016/04/12 at 18:18:46, dschuyler wrote:
> On 2016/04/12 01:35:06, dpapad wrote:
> > Before after screenshots at http://imgur.com/a/58uuX.
> 
> In the screen shots, it looks like the grey separator lines
> were lost in the change.

Thanks for catching this. Fixed in latest patch, I had accidentally removed the
"underbar" CSS class.

[Dan Beam, 2016-04-12 18:25:44]

On 2016/04/12 18:24:40, dpapad wrote:
> On 2016/04/12 at 18:18:46, dschuyler wrote:
> > On 2016/04/12 01:35:06, dpapad wrote:
> > > Before after screenshots at http://imgur.com/a/58uuX.
> > 
> > In the screen shots, it looks like the grey separator lines
> > were lost in the change.
> 
> Thanks for catching this. Fixed in latest patch, I had accidentally removed
the
> "underbar" CSS class.

or that

[dpapad, 2016-04-13 17:24:48]

On 2016/04/12 at 18:24:40, dpapad wrote:
> On 2016/04/12 at 18:18:46, dschuyler wrote:
> > On 2016/04/12 01:35:06, dpapad wrote:
> > > Before after screenshots at http://imgur.com/a/58uuX.
> > 
> > In the screen shots, it looks like the grey separator lines
> > were lost in the change.
> 
> Thanks for catching this. Fixed in latest patch, I had accidentally removed
the "underbar" CSS class.

Friendly ping.

[dschuyler, 2016-04-13 17:58:54]

On the whole it looks good, but I'd like to ask
about whether there is a performance
difference when creating a list with more shadow
doms.  I think an overly large test would be, say
50 items in that list (I don't think it's reasonable
to expect much more than that).  If the difference
is tiny (like < 1ms) on that scale, I'm on board.
This is a font page, unbounded list, so I'm concerned.
WDYT?

https://codereview.chromium.org/1876283002/diff/140001/chrome/browser/resourc...
File chrome/browser/resources/settings/on_startup_page/startup_url_entry.js
(right):

https://codereview.chromium.org/1876283002/diff/140001/chrome/browser/resourc...
chrome/browser/resources/settings/on_startup_page/startup_url_entry.js:19: *
@param {string} url Location of an image to get a set of icons fors.
s/fors/for/

[dpapad, 2016-04-13 18:38:30]

I just tried locally loading the page with 1 vs 50 vs 500 startup URLs, for both
before and after this CL. For 1 and 50 I did not perceive any notice-able
difference in timing before and after. The timeline in DevTools is kind of
useful,
but not to the point where I would be able to tell a 1ms difference. For the 500
URLs case it did feel a bit slower than before. 

The unbound list is a concern of me too, regardless of the extra Shadow DOM
layer or not, and also makes the UI a bit odd for >20 URLs. As discussed a few
days ago, I thought that we would

1) address UX concerns by putting a fixed height and turning on "overflow: auto"
   (Maybe this requires input from Alan?)
2) address performance concerns by converting the dom-repeat to an iron-list (if
   we decide this is necessary).

I am still on board with that plan, but as far as crbug.com/601514, I am focused
on adding the missing functionality.

https://codereview.chromium.org/1876283002/diff/140001/chrome/browser/resourc...
File chrome/browser/resources/settings/on_startup_page/startup_url_entry.js
(right):

https://codereview.chromium.org/1876283002/diff/140001/chrome/browser/resourc...
chrome/browser/resources/settings/on_startup_page/startup_url_entry.js:19: *
@param {string} url Location of an image to get a set of icons fors.
On 2016/04/13 at 17:58:54, dschuyler wrote:
> s/fors/for/

Done.

[dschuyler, 2016-04-13 22:33:00]

Thanks for checking on that.
I entered bug 603305 to track the topic
of switching to a scrolling list when
there are many elements.
LGTM

[dpapad, 2016-04-13 22:40:01]

On 2016/04/13 at 22:33:00, dschuyler wrote:
> Thanks for checking on that.
> I entered bug 603305 to track the topic
> of switching to a scrolling list when
> there are many elements.
> LGTM

Thanks for filing a bug to keep track of this.

[dpapad, 2016-04-13 22:40:04]

The CQ bit was checked by dpapad@chromium.org

[commit-bot: I haz the power, 2016-04-13 22:40:34]

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

[commit-bot: I haz the power, 2016-04-13 23:53:17]

Description was changed from

==========
MD Settings: On Startup, moving remove URL option under a dots menu.

 - Extracting new element <settings-startup-url-entry>
 - Implementing dots menu, that currently only has the "Remove" option.
 - Adding test.

This is in preparation of adding the "Edit" option under the same menu.

BUG=601514
==========

to

==========
MD Settings: On Startup, moving remove URL option under a dots menu.

 - Extracting new element <settings-startup-url-entry>
 - Implementing dots menu, that currently only has the "Remove" option.
 - Adding test.

This is in preparation of adding the "Edit" option under the same menu.

BUG=601514
==========

[commit-bot: I haz the power, 2016-04-13 23:53:18]

Committed patchset #3 (id:160001)

[commit-bot: I haz the power, 2016-04-13 23:54:39]

Description was changed from

==========
MD Settings: On Startup, moving remove URL option under a dots menu.

 - Extracting new element <settings-startup-url-entry>
 - Implementing dots menu, that currently only has the "Remove" option.
 - Adding test.

This is in preparation of adding the "Edit" option under the same menu.

BUG=601514
==========

to

==========
MD Settings: On Startup, moving remove URL option under a dots menu.

 - Extracting new element <settings-startup-url-entry>
 - Implementing dots menu, that currently only has the "Remove" option.
 - Adding test.

This is in preparation of adding the "Edit" option under the same menu.

BUG=601514

Committed: https://crrev.com/9035bfdadfeece36231732a0dd21fbd0ae200730
Cr-Commit-Position: refs/heads/master@{#387158}
==========

[commit-bot: I haz the power, 2016-04-13 23:54:40]

Patchset 3 (id:??) landed as
https://crrev.com/9035bfdadfeece36231732a0dd21fbd0ae200730
Cr-Commit-Position: refs/heads/master@{#387158}