Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2421433003:
Add ScrollPageToElement to action_runner. (Closed)
|
Created:
4 years, 2 months ago by rnephew (Reviews Here) Modified:
4 years, 2 months ago CC:
catapult-reviews_chromium.org, telemetry-reviews_chromium.org, alexandermont Target Ref:
refs/heads/master Project:
catapult Visibility:
Public. |
DescriptionAdd ScrollPageToElement to action_runner.
This adds the ability scroll an element into view on the page.
BUG=chromium:651909
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/f506b6809e993f7415cc069e9cbcfa587e985aa4
Patch Set 1 #
Total comments: 13
Patch Set 2 : Add Unittest #Patch Set 3 : Add ScrollPageToElement to action_runner. #Patch Set 4 : scroll element to center instead of top #
Total comments: 38
Patch Set 5 : Add ScrollPageToElement to action_runner. #
Total comments: 2
Patch Set 6 : Add ScrollPageToElement to action_runner. #
Messages
Total messages: 23 (6 generated)
rnephew@chromium.org changed reviewers: + nednguyen@google.com
nednguyen@google.com changed reviewers: + charliea@chromium.org
Charlie: can you take the first pass of reviewing?
On 2016/10/13 17:50:25, nednguyen wrote: > Charlie: can you take the first pass of reviewing? Also side note: I am still working on unit tests for this.
> Also side note: I am still working on unit tests for this. Test added.
https://codereview.chromium.org/2421433003/diff/1/telemetry/telemetry/interna... File telemetry/telemetry/internal/actions/action_runner.py (right): https://codereview.chromium.org/2421433003/diff/1/telemetry/telemetry/interna... telemetry/telemetry/internal/actions/action_runner.py:396: def ScrollPageToElement(self, element): I think it's weird that this function takes an element, whereas the other <Action>Element functions have other means of specifying which element is being acted on. See PinchElement: it allows the user to specify the element either through a function (kind of like what you do) and through a selector. I imagine that the selector is usually the more useful of the two. Should we do the same? https://codereview.chromium.org/2421433003/diff/1/telemetry/telemetry/interna... telemetry/telemetry/internal/actions/action_runner.py:397: """Perform scroll gesture on page until element is in view. s/element/an element (similar to PinchElement above) https://codereview.chromium.org/2421433003/diff/1/telemetry/telemetry/interna... telemetry/telemetry/internal/actions/action_runner.py:402: js = ''' I don't see any other functions in this page that have raw Javascript in them, which is probably a bad sign. It looks like all of the javascript is abstracted into these "actions" that live in https://cs.corp.google.com/github/catapult-project/catapult/telemetry/telemet..., and these functions basically just make some assertions and defer to those actions. I think we should probably be doing the same. https://codereview.chromium.org/2421433003/diff/1/telemetry/telemetry/interna... telemetry/telemetry/internal/actions/action_runner.py:403: function getScrollDist(elem) { This actually pollutes the global namespace with a new function called getScrollDist(). I think probably what we want to do is something like: (element) => { // Your function here... }(%s); Basically, we create an anonymous function and then immediately invoke it, which avoids having to create an identifier for the function. https://codereview.chromium.org/2421433003/diff/1/telemetry/telemetry/interna... telemetry/telemetry/internal/actions/action_runner.py:405: var viewHeight = Math.max( I'm not sure I completely understand why the viewHeight is the MAX of these two numbers. It seems like if we're trying to calculate the height of the view, it'd actually be the min of these two numbers. Could you clarify? Also, all of the pages that I've found seem to automatically set document.documentElement.clientHeight to equal window.innerHeight when the document would normally be shorter than the window's height. Under what circumstances is this check useful? (My test page: http://www.brainjar.com/java/host/test.html) https://codereview.chromium.org/2421433003/diff/1/telemetry/telemetry/interna... telemetry/telemetry/internal/actions/action_runner.py:407: if (!(rect.bottom < 0 || rect.top - viewHeight >=0)) { nit: make sure to have spaces around both sides of operators (viewHeight >= 0, not viewHeight >=0) https://codereview.chromium.org/2421433003/diff/1/telemetry/telemetry/interna... telemetry/telemetry/internal/actions/action_runner.py:407: if (!(rect.bottom < 0 || rect.top - viewHeight >=0)) { I think that maybe this could be more clearly expressed by dividing it up more clearly into the possible cases, like: if (rect.bottom < 0) { // The bottom of the element is above the viewport. return rect.bottom; } if (rect.top - viewHeight >= 0) { // rect.top provides the pixel offset of the element from the // top of the page. Because that exceeds the viewport's height, // we know that the element is below the viewport. return rect.top - viewHeight; } // The element is in the viewport. return 0; The "Chrome way" of constructing branches is to check first for the case that you don't have to do anything, and immediately short-circuit out of the function if you see that case. This is how the function is currently written. However, that's a little bit awkward for this function because we're looking at three mutually exclusive cases, and in my opinion: if (!conditionA or !conditionB) return 0; if (conditionA) return foo; if (conditionB) return bar; is less readable than: if (conditionA) return foo; if (conditionB) return bar; return 0; because you don't have to repeat the branching condition in two places in the second approach.
https://codereview.chromium.org/2421433003/diff/1/telemetry/telemetry/interna... File telemetry/telemetry/internal/actions/action_runner.py (right): https://codereview.chromium.org/2421433003/diff/1/telemetry/telemetry/interna... telemetry/telemetry/internal/actions/action_runner.py:396: def ScrollPageToElement(self, element): On 2016/10/13 18:44:53, charliea wrote: > I think it's weird that this function takes an element, whereas the other > <Action>Element functions have other means of specifying which element is being > acted on. See PinchElement: it allows the user to specify the element either > through a function (kind of like what you do) and through a selector. I imagine > that the selector is usually the more useful of the two. Should we do the same? Done. https://codereview.chromium.org/2421433003/diff/1/telemetry/telemetry/interna... telemetry/telemetry/internal/actions/action_runner.py:397: """Perform scroll gesture on page until element is in view. On 2016/10/13 18:44:53, charliea wrote: > s/element/an element (similar to PinchElement above) Done. https://codereview.chromium.org/2421433003/diff/1/telemetry/telemetry/interna... telemetry/telemetry/internal/actions/action_runner.py:402: js = ''' On 2016/10/13 18:44:53, charliea wrote: > I don't see any other functions in this page that have raw Javascript in them, > which is probably a bad sign. It looks like all of the javascript is abstracted > into these "actions" that live in > https://cs.corp.google.com/github/catapult-project/catapult/telemetry/telemet..., > and these functions basically just make some assertions and defer to those > actions. I think we should probably be doing the same. Done. https://codereview.chromium.org/2421433003/diff/1/telemetry/telemetry/interna... telemetry/telemetry/internal/actions/action_runner.py:403: function getScrollDist(elem) { On 2016/10/13 18:44:53, charliea wrote: > This actually pollutes the global namespace with a new function called > getScrollDist(). > > I think probably what we want to do is something like: > > (element) => { > // Your function here... > }(%s); > > Basically, we create an anonymous function and then immediately invoke it, which > avoids having to create an identifier for the function. Done. https://codereview.chromium.org/2421433003/diff/1/telemetry/telemetry/interna... telemetry/telemetry/internal/actions/action_runner.py:405: var viewHeight = Math.max( On 2016/10/13 18:44:53, charliea wrote: > I'm not sure I completely understand why the viewHeight is the MAX of these two > numbers. It seems like if we're trying to calculate the height of the view, it'd > actually be the min of these two numbers. Could you clarify? > > Also, all of the pages that I've found seem to automatically set > document.documentElement.clientHeight to equal window.innerHeight when the > document would normally be shorter than the window's height. Under what > circumstances is this check useful? > > (My test page: http://www.brainjar.com/java/host/test.html) Done. https://codereview.chromium.org/2421433003/diff/1/telemetry/telemetry/interna... telemetry/telemetry/internal/actions/action_runner.py:407: if (!(rect.bottom < 0 || rect.top - viewHeight >=0)) { On 2016/10/13 18:44:53, charliea wrote: > nit: make sure to have spaces around both sides of operators > > (viewHeight >= 0, not viewHeight >=0) Done.
Patchset #3 (id:40001) has been deleted
https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... File telemetry/telemetry/internal/testing/page_with_swipeables.html (right): https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/testing/page_with_swipeables.html:51: <div id="off-screen"></div> How can we be sure that this is off screen?
https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... File telemetry/telemetry/internal/actions/scroll_to_element.py (right): https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/scroll_to_element.py:47: f(%s); If we can compute the distance here, why not also call the js to scroll directly (like in scroll.py). This would help avoid the extra round trip between telemetry & the browser.
https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... File telemetry/telemetry/internal/actions/scroll_to_element.py (right): https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/scroll_to_element.py:47: f(%s); On 2016/10/13 23:16:12, nednguyen wrote: > If we can compute the distance here, why not also call the js to scroll directly > (like in scroll.py). This would help avoid the extra round trip between > telemetry & the browser. I was trying to reuse code to limit the complexity as much as possible. Does the extra round trip cause problems that we should avoid? https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... File telemetry/telemetry/internal/testing/page_with_swipeables.html (right): https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/testing/page_with_swipeables.html:51: <div id="off-screen"></div> On 2016/10/13 23:13:48, nednguyen wrote: > How can we be sure that this is off screen? The div above it is 5000x5000 pixels in size, so thats not a guarantee it'll be off screen I guess, but its a good indicator that it should be at first.
https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... File telemetry/telemetry/internal/actions/action_runner_unittest.py (right): https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/action_runner_unittest.py:288: off_screen_element = 'document.querySelectorAll("#off-screen")[0]' Can you move this to a separate test case, s.t like ScrollPageToElement ? https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... File telemetry/telemetry/internal/actions/scroll_to_element.py (right): https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/scroll_to_element.py:47: f(%s); On 2016/10/14 00:13:18, rnephew (Reviews Here) wrote: > On 2016/10/13 23:16:12, nednguyen wrote: > > If we can compute the distance here, why not also call the js to scroll > directly > > (like in scroll.py). This would help avoid the extra round trip between > > telemetry & the browser. > > I was trying to reuse code to limit the complexity as much as possible. Does the > extra round trip cause problems that we should avoid? The round trip mostly will cause more delay & extra js issued on the page. Though in this case, your js is not that much, so I think this could be fine. https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... File telemetry/telemetry/internal/testing/page_with_swipeables.html (right): https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/testing/page_with_swipeables.html:51: <div id="off-screen"></div> On 2016/10/14 00:13:18, rnephew (Reviews Here) wrote: > On 2016/10/13 23:13:48, nednguyen wrote: > > How can we be sure that this is off screen? > > The div above it is 5000x5000 pixels in size, so thats not a guarantee it'll be > off screen I guess, but its a good indicator that it should be at first. Ah I see. In your test, can you first assert that it's off screen? https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/testing/page_with_swipeables.html:52: </body> can you s.t like another tall-and-wide, then another off screen element? Would be nice to check scrolling down to get into the element, then scroll up to get to another element.
Looking a lot closer! I'll cede final approval to Ned, as I'll be OOO tomorrow https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... File telemetry/telemetry/internal/actions/action_runner.py (right): https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/action_runner.py:402: text: The element must contains this exact text. |text| isn't an arg here. bad c/p? https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... File telemetry/telemetry/internal/actions/action_runner_unittest.py (right): https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/action_runner_unittest.py:288: off_screen_element = 'document.querySelectorAll("#off-screen")[0]' On 2016/10/14 00:19:01, nednguyen wrote: > Can you move this to a separate test case, s.t like ScrollPageToElement ? +1 https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/action_runner_unittest.py:289: determine_if_on_screen = ''' I think this variable is a little misleadingly named. I'd expect a "determine_if_on_screen" variable to be a function that returns True or False depending on whether the element is on the screen. This one instead returns 0, 1, or 2. Maybe something like "viewport_comparator" would be better? Or, because it's not a python function but rather a js one, viewport_comparator_js? https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/action_runner_unittest.py:290: function getOnScreen(elem) { nit: similar comment to before, where I think it makes sense to make this an anonymous function to avoid polluting the global namespace: (elem) => { var rect = elem.getBoundingClientRect(); ... }(%s) https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/action_runner_unittest.py:295: return 2 nit: all JS lines should end in semicolons https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/action_runner_unittest.py:295: return 2 I might go with -1 (above the viewport), 0 (in the viewport), and 1 (below the viewport) to mirror the typical interface for compare functions (see "compare function" here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Obje...) https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/action_runner_unittest.py:301: return 1 nit: need semicolon https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/action_runner_unittest.py:303: return 0 nit: need semicolon https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... File telemetry/telemetry/internal/actions/scroll_to_element.py (right): https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/scroll_to_element.py:1: # Copyright 2015 The Chromium Authors. All rights reserved. 2016 https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/scroll_to_element.py:13: speed=None): What is |speed| in this case? The number of pixels scrolled per second? Probably worth documenting. (similarly, not sure what |timeout| means in this context) https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/scroll_to_element.py:30: js = ''' Maybe get_distance_js? https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/scroll_to_element.py:31: f = ( can you just do: (elem) => { // Your function code here... }(%s); ? In this case, you're still polluting the global namespace with a function named "f" https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/scroll_to_element.py:45: return 0 nit: needs a semicolon https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/scroll_to_element.py:59: scroller.WillRunAction(tab) I'm not 100% sure, but it seems like we might want to create the scroller in WillRunAction so that all of the same exceptions that would get raised for a ScrollAction will also get raised at the same time for our ScrollToElementAction https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... File telemetry/telemetry/internal/testing/page_with_swipeables.html (right): https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/testing/page_with_swipeables.html:52: </body> On 2016/10/14 00:19:02, nednguyen wrote: > can you s.t like another tall-and-wide, then another off screen element? Would > be nice to check scrolling down to get into the element, then scroll up to get > to another element. +1 Without this, we can't really have any confidence that scrolling up works.
https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... File telemetry/telemetry/internal/actions/action_runner.py (right): https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/action_runner.py:402: text: The element must contains this exact text. On 2016/10/14 03:34:28, charliea (OOO until 10-17) wrote: > |text| isn't an arg here. bad c/p? Done. https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... File telemetry/telemetry/internal/actions/action_runner_unittest.py (right): https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/action_runner_unittest.py:288: off_screen_element = 'document.querySelectorAll("#off-screen")[0]' On 2016/10/14 00:19:01, nednguyen wrote: > Can you move this to a separate test case, s.t like ScrollPageToElement ? Done. https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/action_runner_unittest.py:289: determine_if_on_screen = ''' On 2016/10/14 03:34:28, charliea (OOO until 10-17) wrote: > I think this variable is a little misleadingly named. I'd expect a > "determine_if_on_screen" variable to be a function that returns True or False > depending on whether the element is on the screen. This one instead returns 0, > 1, or 2. > > Maybe something like "viewport_comparator" would be better? Or, because it's not > a python function but rather a js one, viewport_comparator_js? Done. https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/action_runner_unittest.py:290: function getOnScreen(elem) { On 2016/10/14 03:34:28, charliea (OOO until 10-17) wrote: > nit: similar comment to before, where I think it makes sense to make this an > anonymous function to avoid polluting the global namespace: > > (elem) => { > var rect = elem.getBoundingClientRect(); > ... > }(%s) Done. https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/action_runner_unittest.py:295: return 2 On 2016/10/14 03:34:28, charliea (OOO until 10-17) wrote: > I might go with -1 (above the viewport), 0 (in the viewport), and 1 (below the > viewport) to mirror the typical interface for compare functions (see "compare > function" here: > https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Obje...) Done. https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/action_runner_unittest.py:295: return 2 On 2016/10/14 03:34:28, charliea (OOO until 10-17) wrote: > nit: all JS lines should end in semicolons Done. https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/action_runner_unittest.py:301: return 1 On 2016/10/14 03:34:28, charliea (OOO until 10-17) wrote: > nit: need semicolon Done. https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/action_runner_unittest.py:303: return 0 On 2016/10/14 03:34:28, charliea (OOO until 10-17) wrote: > nit: need semicolon Done. https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... File telemetry/telemetry/internal/actions/scroll_to_element.py (right): https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/scroll_to_element.py:1: # Copyright 2015 The Chromium Authors. All rights reserved. On 2016/10/14 03:34:28, charliea (OOO until 10-17) wrote: > 2016 Done. https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/scroll_to_element.py:13: speed=None): On 2016/10/14 03:34:28, charliea (OOO until 10-17) wrote: > What is |speed| in this case? The number of pixels scrolled per second? Probably > worth documenting. (similarly, not sure what |timeout| means in this context) Done. https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/scroll_to_element.py:30: js = ''' On 2016/10/14 03:34:28, charliea (OOO until 10-17) wrote: > Maybe get_distance_js? Done. https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/scroll_to_element.py:31: f = ( On 2016/10/14 03:34:28, charliea (OOO until 10-17) wrote: > can you just do: > > (elem) => { > // Your function code here... > }(%s); > > ? In this case, you're still polluting the global namespace with a function > named "f" Done. https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/scroll_to_element.py:37: return rect.top + (window.innerHeight / 2); rect.top is a negative number here, so need to subtract not add. https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/scroll_to_element.py:45: return 0 On 2016/10/14 03:34:28, charliea (OOO until 10-17) wrote: > nit: needs a semicolon Done. https://codereview.chromium.org/2421433003/diff/80001/telemetry/telemetry/int... telemetry/telemetry/internal/actions/scroll_to_element.py:59: scroller.WillRunAction(tab) On 2016/10/14 03:34:28, charliea (OOO until 10-17) wrote: > I'm not 100% sure, but it seems like we might want to create the scroller in > WillRunAction so that all of the same exceptions that would get raised for a > ScrollAction will also get raised at the same time for our ScrollToElementAction Done.
lgtm https://codereview.chromium.org/2421433003/diff/100001/telemetry/telemetry/in... File telemetry/telemetry/internal/actions/action_runner_unittest.py (right): https://codereview.chromium.org/2421433003/diff/100001/telemetry/telemetry/in... telemetry/telemetry/internal/actions/action_runner_unittest.py:265: if not page_action.IsGestureSourceTypeSupported( I think we don't need this. When I patch your CL & run the test on my Mac, it did not run because of this check. Removing this test make it runs the actual action just fine.
https://codereview.chromium.org/2421433003/diff/100001/telemetry/telemetry/in... File telemetry/telemetry/internal/actions/action_runner_unittest.py (right): https://codereview.chromium.org/2421433003/diff/100001/telemetry/telemetry/in... telemetry/telemetry/internal/actions/action_runner_unittest.py:265: if not page_action.IsGestureSourceTypeSupported( On 2016/10/14 18:05:59, nednguyen wrote: > I think we don't need this. When I patch your CL & run the test on my Mac, it > did not run because of this check. Removing this test make it runs the actual > action just fine. Done.
The CQ bit was checked by rnephew@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from nednguyen@google.com Link to the patchset: https://codereview.chromium.org/2421433003/#ps120001 (title: "Add ScrollPageToElement to action_runner.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Add ScrollPageToElement to action_runner. This adds the ability scroll an element into view on the page. BUG=chromium:651909 ========== to ========== Add ScrollPageToElement to action_runner. This adds the ability scroll an element into view on the page. BUG=chromium:651909 Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapu... ==========
Message was sent while issue was closed.
Committed patchset #6 (id:120001) as https://chromium.googlesource.com/external/github.com/catapult-project/catapu... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
