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.

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

Added waitForPageToLoad function #57

Merged
merged 45 commits into from
Mar 25, 2018
Merged

Conversation

Marketionist
Copy link
Member

@Marketionist Marketionist commented Mar 11, 2018

This pull request adds waitForPageToLoad function

NOTE: WARNING: the "doDoubleClick" command will be deprecated soon. This command is not part of the W3C WebDriver spec and won't be supported in future versions of the driver. It is recommended to just call the click command on the same element twice in the row.

WARNING: the "moveTo" command will be deprecated soon. Note: This command is not part of the W3C WebDriver spec and won't be supported in future versions of the driver. It is recommended to use the actions command to emulate pointer events.

@Marketionist Marketionist changed the title Added waitForLoaded function Added waitForPageToLoad function Mar 13, 2018
@@ -1,4 +1,4 @@
let e = {};
const e = {};
Copy link
Member Author

Choose a reason for hiding this comment

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

This object is being changed in the next line, so it looks like it should be let instead of const

@@ -1,4 +1,4 @@
let e = {};
const e = {};
Copy link
Member Author

Choose a reason for hiding this comment

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

This object is being changed (extended with additional properties) in the next 2 lines, so it looks like it should be let instead of const

@@ -64,7 +64,7 @@ function pageObjectGetter (str) {
function getPageObject (str) {
const pageObjectGetterFunc = stepsConfig.objectsProcessor.pageObjectGetter || pageObjectGetter;
const value = pageObjectGetterFunc(str);
const idValue = value.replace(_r(regDynamicId, 'g'), stepsConfig.id.getId());
const idValue = value.replace(_r(regDynamicId, 'g'), stepsConfig.id.id);
Copy link
Member Author

Choose a reason for hiding this comment

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

stepsConfig.id.id - sounds a bit strange - maybe it will be better to name it stepsConfig.id.idValue?

@@ -99,7 +99,7 @@ function dictionaryGetter (str) {
function getDictionaryObject (str) {
const dictionaryGetterFunc = stepsConfig.objectsProcessor.dictionaryGetter || dictionaryGetter;
const value = dictionaryGetterFunc(str);
const idValue = value.replace(_r(regDynamicId, 'g'), stepsConfig.id.getId());
const idValue = value.replace(_r(regDynamicId, 'g'), stepsConfig.id.id);
Copy link
Member Author

Choose a reason for hiding this comment

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

stepsConfig.id.id - sounds a bit strange - maybe it will be better to name it stepsConfig.id.idValue?

constructor () {
this.value = defaultIdValue || idGenerator.call(this);
}
get id () {
Copy link
Member Author

Choose a reason for hiding this comment

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

stepsConfig.id.id - sounds a bit strange - maybe it will be better to name it stepsConfig.id.idValue?

@@ -1,4 +1,4 @@
let e = {};
const e = {};
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe it is still better to use let here, because this object is being changed in the next line (adding property)?

@@ -1,4 +1,4 @@
let e = {};
const e = {};
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe it is still better to use let here, because this object is being changed in the next line (adding property)?

@@ -0,0 +1,36 @@
const defaultFinishedLoadingConditions = (loaderSelectors) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe we should rename this file to something like steps.helper.functions.js (because it looks more like a file with logic then config and we also have a second file with the same name in test/steps.conf.js - and it looks like a real config file more)?

}

// Check if any loaders are still present on the page
return !loaderSelectors.some((selector) => browser.execute((s) => document.querySelector(s), selector));
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe it will be better to use http://webdriver.io/api/protocol/executeAsync.html here as we are going to use async functions everywhere

@Marketionist Marketionist merged commit 3b83ae3 into master Mar 25, 2018
@Marketionist Marketionist deleted the feature/wait-for-loaded branch March 25, 2018 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants