-
-
Notifications
You must be signed in to change notification settings - Fork 257
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
add waitFor msg #356
Conversation
@@ -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' } = {}) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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...
tests/unit/dom/wait-for-test.js
Outdated
@@ -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' }); |
There was a problem hiding this comment.
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' } = {}) { |
There was a problem hiding this comment.
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...
@snewcomer - 👋 looks like some linting failures here still but if you fix those and rebase against master, CI should be green again... |
There was a problem hiding this 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?
Thank you! |
Opens up
waitFor
to have atimeoutMessage
.