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

add waitFor msg #356

Merged
merged 4 commits into from
Apr 20, 2018
Merged

add waitFor msg #356

merged 4 commits into from
Apr 20, 2018

Conversation

snewcomer
Copy link
Contributor

Opens up waitFor to have a timeoutMessage.

@@ -15,7 +15,7 @@ import { nextTickPromise } from '../-utils';
@param {number} [options.count=null] the number of elements that should match the provided selector (null means one or more)
@returns {Element|Array<Element>} the element (or array of elements) that were being waited upon
*/
export default function waitFor(selector, { timeout = 1000, count = null } = {}) {
export default function waitFor(selector, { timeout = 1000, count = null, timeoutMessage = 'waitFor timed out' } = {}) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can revert the default and keep waitUntil timed out if you would like.

Copy link
Member

Choose a reason for hiding this comment

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

Nah, I think this is fine...

@@ -71,11 +71,11 @@ module('DOM Helper: waitFor', function(hooks) {

let start = Date.now();
try {
await waitFor('.something', { timeout: 100 });
await waitFor('.something', { timeout: 100, timeoutMessage: '.something timed out' });
Copy link
Member

Choose a reason for hiding this comment

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

Lets leave this test alone, and add another that is specifically for modifying the timeout message...

@@ -15,7 +15,7 @@ import { nextTickPromise } from '../-utils';
@param {number} [options.count=null] the number of elements that should match the provided selector (null means one or more)
@returns {Element|Array<Element>} the element (or array of elements) that were being waited upon
*/
export default function waitFor(selector, { timeout = 1000, count = null } = {}) {
export default function waitFor(selector, { timeout = 1000, count = null, timeoutMessage = 'waitFor timed out' } = {}) {
Copy link
Member

Choose a reason for hiding this comment

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

Nah, I think this is fine...

@rwjblue
Copy link
Member

rwjblue commented Apr 20, 2018

@snewcomer - 👋 looks like some linting failures here still but if you fix those and rebase against master, CI should be green again...

Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

Sorry @snewcomer, I just noticed that we also need to update the docs for the new option, would you mind adding that?

@rwjblue rwjblue merged commit 89cf4ac into emberjs:master Apr 20, 2018
@rwjblue
Copy link
Member

rwjblue commented Apr 20, 2018

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants