MD Settings: make iron list offset so items render onscreen

MD Settings: make iron list offset so items render onscreen

R=dbeam@chromium.org
BUG=689356
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2805213003
Cr-Commit-Position: refs/heads/master@{#463312}
Committed: https://chromium.googlesource.com/chromium/src/+/2aa395986fe5e88a8c6d867c5e6e663697c89854

[hcarmona, 2017-04-07 23:09:56]

Description was changed from

==========
MD Settings: make iron list offset so items render onscreen

R=dbeam@chromium.org
BUG=689356
==========

to

==========
MD Settings: make iron list offset so items render onscreen

R=dbeam@chromium.org
BUG=689356
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[hcarmona, 2017-04-07 23:09:58]

The CQ bit was checked by hcarmona@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-07 23:10:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hcarmona, 2017-04-07 23:11:56]

PTAL

[Dan Beam, 2017-04-07 23:32:53]

https://codereview.chromium.org/2805213003/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/search_engines_page/search_engines_page.js
(right):

https://codereview.chromium.org/2805213003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_engines_page/search_engines_page.js:67:
this.$.otherEngines.scrollOffset = 184 +
this looks brittle, where does the 184 come from?  are you sure it's the same on
all platforms?  what happens if this changes?

[hcarmona, 2017-04-08 00:02:01]

https://codereview.chromium.org/2805213003/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/search_engines_page/search_engines_page.js
(right):

https://codereview.chromium.org/2805213003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_engines_page/search_engines_page.js:67:
this.$.otherEngines.scrollOffset = 184 +
On 2017/04/07 23:32:53, Dan Beam wrote:
> this looks brittle, where does the 184 come from?  are you sure it's the same
on
> all platforms?  what happens if this changes?

Agreed, for parts that are in this file, I can add an ID and get the height
using the client rect...
Maybe putting them in a div and getting the height of that?
But, the header on the top of the page is outside the shadow dom.

Suggestions?

[Dan Beam, 2017-04-08 00:09:44]

https://codereview.chromium.org/2805213003/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/search_engines_page/search_engines_page.js
(right):

https://codereview.chromium.org/2805213003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_engines_page/search_engines_page.js:67:
this.$.otherEngines.scrollOffset = 184 +
On 2017/04/08 00:02:01, hcarmona wrote:
> On 2017/04/07 23:32:53, Dan Beam wrote:
> > this looks brittle, where does the 184 come from?  are you sure it's the
same
> on
> > all platforms?  what happens if this changes?
> 
> Agreed, for parts that are in this file, I can add an ID and get the height
> using the client rect...
> Maybe putting them in a div and getting the height of that?
> But, the header on the top of the page is outside the shadow dom.
> 
> Suggestions?

offsetTop

[commit-bot: I haz the power, 2017-04-08 00:18:44]

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

[commit-bot: I haz the power, 2017-04-08 00:18:45]

Dry run: This issue passed the CQ dry run.

[hcarmona, 2017-04-08 00:24:40]

https://codereview.chromium.org/2805213003/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/search_engines_page/search_engines_page.js
(right):

https://codereview.chromium.org/2805213003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_engines_page/search_engines_page.js:67:
this.$.otherEngines.scrollOffset = 184 +
On 2017/04/08 00:09:44, Dan Beam wrote:
> On 2017/04/08 00:02:01, hcarmona wrote:
> > On 2017/04/07 23:32:53, Dan Beam wrote:
> > > this looks brittle, where does the 184 come from?  are you sure it's the
> same
> > on
> > > all platforms?  what happens if this changes?
> > 
> > Agreed, for parts that are in this file, I can add an ID and get the height
> > using the client rect...
> > Maybe putting them in a div and getting the height of that?
> > But, the header on the top of the page is outside the shadow dom.
> > 
> > Suggestions?
> 
> offsetTop

Sweet. Done.

[Dan Beam, 2017-04-10 05:34:50]

lgtm if offsetTop is the exact right amount at all times (you tested locally
right?)

[hcarmona, 2017-04-10 16:36:51]

The CQ bit was checked by hcarmona@chromium.org

[commit-bot: I haz the power, 2017-04-10 16:37:17]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-10 17:38:03]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1491842211160650,
"parent_rev": "8b7a7716bedcc5120cafc3df7ecb61aa542a1089", "commit_rev":
"2aa395986fe5e88a8c6d867c5e6e663697c89854"}

[commit-bot: I haz the power, 2017-04-10 17:38:50]

Description was changed from

==========
MD Settings: make iron list offset so items render onscreen

R=dbeam@chromium.org
BUG=689356
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: make iron list offset so items render onscreen

R=dbeam@chromium.org
BUG=689356
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2805213003
Cr-Commit-Position: refs/heads/master@{#463312}
Committed:
https://chromium.googlesource.com/chromium/src/+/2aa395986fe5e88a8c6d867c5e6e...
==========

[commit-bot: I haz the power, 2017-04-10 17:38:51]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/2aa395986fe5e88a8c6d867c5e6e...