Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add scrollTo selector #774

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Add scrollTo selector #774

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b9f926f
Update clickAndHoverHelper.js
aliu145 May 15, 2018
b9fdcd3
Update clickAndHoverHelper.js
aliu145 May 15, 2018
0712adf
Merge pull request #2 from aliu145/anna.liu/add-scrollTo-selector-1
aliu145 May 15, 2018
7d0b902
Update README.md
aliu145 May 15, 2018
a394746
Merge pull request #3 from aliu145/anna.liu/add-scrollTo-selector-2
aliu145 May 15, 2018
bd77957
Adding test for scrollToSelector
May 17, 2018
019e1d9
Update index.html
aliu145 May 17, 2018
2a019aa
Update backstop_features.js
aliu145 May 17, 2018
24a52dd
Merge pull request #4 from aliu145/master
aliu145 May 17, 2018
7d9c350
Merge branch 'anna.liu/add-scrollTo-selector' of https://github.com/a…
Jun 25, 2018
e4c2770
Adding tests for scrollToSelector
Jun 26, 2018
c0640ee
update test label
Jul 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ removeSelectors // Array of selectors set to display: none
onReadyScript // After the above conditions are met -- use this script to modify UI state prior to screen shots e.g. hovers, clicks etc.
hoverSelector // Move the pointer over the specified DOM element prior to screen shot (available with default onReadyScript)
clickSelector // Click the specified DOM element prior to screen shot (available with default onReadyScript)
scrollToSelector // Scrolls the specified DOM element into view prior to screen shot (available with default onReadyScript)
postInteractionWait // Wait for a selector after interacting with hoverSelector or clickSelector (optionally accepts wait time in ms. Idea for use with a click or hover element transition. available with default onReadyScript)
selectors // Array of selectors to capture. Defaults to document if omitted. Use "viewport" to capture the viewport size. See Targeting elements in the next section for more info...
selectorExpansion // See Targeting elements in the next section for more info...
Expand Down
10 changes: 10 additions & 0 deletions capture/engine_scripts/chromy/clickAndHoverHelper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
module.exports = function (chromy, scenario) {
var hoverSelector = scenario.hoverSelector;
var clickSelector = scenario.clickSelector;
var scrollToSelector = scenario.scrollToSelector;
var postInteractionWait = scenario.postInteractionWait; // selector [str] | ms [int]

if (hoverSelector) {
Expand All @@ -17,6 +18,15 @@ module.exports = function (chromy, scenario) {
.wait(clickSelector)
.click(clickSelector);
}

if (scrollToSelector) {
chromy
.wait()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does this line do?

.evaluate(`_scrollToSelector = '${scrollToSelector}'`)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont think you need this line.

.evaluate(function() {
document.querySelector(_scrollToSelector).scrollIntoView();
}, scrollToSelector);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you want this...

.evaluate(function(scrollToSelector){
  document.querySelector(scrollToSelector).scrollIntoView();
}, scrollToSelector);

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably also need to guard against scrollToSelector not being found.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @garris, I made these updates but it's still not working (ie the screenshot shows that it did not properly scroll the container)

}

if (postInteractionWait) {
chromy.wait(postInteractionWait);
Expand Down
10 changes: 10 additions & 0 deletions capture/engine_scripts/puppet/clickAndHoverHelper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
module.exports = async (page, scenario) => {
var hoverSelector = scenario.hoverSelectors || scenario.hoverSelector;
var clickSelector = scenario.clickSelectors || scenario.clickSelector;
var scrollToSelector = scenario.scrollToSelectors || scenario.scrollToSelector;
var postInteractionWait = scenario.postInteractionWait; // selector [str] | ms [int]

if (hoverSelector) {
Expand All @@ -16,6 +17,15 @@ module.exports = async (page, scenario) => {
await page.click(clickSelectorIndex);
}
}

if (scrollToSelector) {
for (const scrollToSelectorIndex of [].concat(scrollToSelector)) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you intend scrollToSelector to take an array value? or just a string? Array feels like it would produce some noisy results.

await page.waitFor(scrollToSelectorIndex);
await page.evaluate(scrollToSelectorIndex => {
document.querySelector(scrollToSelectorIndex).scrollIntoView();
}, scrollToSelectorIndex);
}
}

if (postInteractionWait) {
await page.waitFor(postInteractionWait);
Expand Down