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 test for ES6 Promises #1169

Merged
merged 1 commit into from
Jan 3, 2014
Merged

Conversation

kristerkari
Copy link
Contributor

This feature test is from Jake Archibald's ES6 Promises polyfill.

Quick browser test:
Firefox 26 returns false
Chrome 31.0.1650.63 returns false
Chrome 34.0.1766.0 canary returns true

@patrickkettner
Copy link
Member

I don't really dig the repetition of of window.Promise, but thats not a big deal.

could you add the polyfill to @jakearchibald's test that you linked to in the PR link?

Also, I know that @paulirish had spoken with @stucox about adding a promise based syntax to the async tests. Are you guys cool with Modernizr.promises being taken up by a detect? I can't imagine using that for the actual API, but worth making sure

@kristerkari
Copy link
Contributor Author

could you add the polyfill to @jakearchibald's test that you linked to in the PR link?

Sorry, I did not quite understand this, add what where?

@patrickkettner
Copy link
Member

Sorry, upon rereading that that was really unclear.

Could you add the ES6-Promises polyfill that you linked to to the polyfills.json and document it in this feature-detect

@kristerkari
Copy link
Contributor Author

Alright, sure thing.

@kristerkari
Copy link
Contributor Author

Ok, PR updated. Added "polyfills" and "caniuse" name.

@patrickkettner
Copy link
Member

beautiful work, as always. 👍 from me, @stucox, you good with about the above API question?

patrickkettner added a commit that referenced this pull request Jan 3, 2014
@patrickkettner patrickkettner merged commit 3408014 into Modernizr:master Jan 3, 2014
@stucox
Copy link
Member

stucox commented Jan 8, 2014

Sorry for the delay on weighing in on this. promises is definitely the most obvious name for this detect.

I can’t picture us using Modernizr.promises for a promise-compatible API syntax, but the very similar Modernizr.promise could be a vague possibility.

That said, my current thinking is that we’d just extend Modernizr.on() to make it promise-compliant, e.g.:

Modernizr.on('webp').then(function (result) {
    // WebP supported
}, function (error) {
    // WebP not supported
});

@patrickkettner
Copy link
Member

beauty, we were on the same train afterall.

patrickkettner added a commit to patrickkettner/Modernizr that referenced this pull request Feb 22, 2015
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.

None yet

3 participants