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 a requestAnimationFrame polyfill #904

Merged
merged 1 commit into from Sep 6, 2016

Conversation

rreusser
Copy link
Contributor

@rreusser rreusser commented Sep 6, 2016

This PR adds a proper RAF polyfill based on this discussion and the resulting repo. I debated whether to make this Lib.requestAnimationFrame and Lib.cancelAnimationFrame or not. The only downside of this approach is that it would fix libraries using requestAnimationFrame without a polyfill.

As part of this, remove the logic from:

if(window.requestAnimationFrame) {
window.requestAnimationFrame(function() {
Plotly.relayout(gd, 'xaxis.range', [dataMin, dataMax]);
});
} else {
setTimeout(function() {
Plotly.relayout(gd, 'xaxis.range', [dataMin, dataMax]);
}, 16);
}

@rreusser rreusser mentioned this pull request Sep 6, 2016
14 tasks
@@ -13,6 +13,8 @@ var d3 = require('d3');

var lib = module.exports = {};

require('./request_animation_frame');
Copy link
Contributor

@etpinard etpinard Sep 6, 2016

Choose a reason for hiding this comment

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

Since raf is already pretty well supported (ref http://caniuse.com/#feat=requestanimationframe), I'd vote for not including the polyfill in the plotly.js bundle by default.

I would move the request_animation_frame.js to dist/extras and add a line in the dist/ README.md generated here.

@rreusser rreusser force-pushed the request-animation-frame-polyfill branch from d87a468 to 57611e9 Compare September 6, 2016 14:27
@@ -52,6 +52,7 @@ function getInfoContent() {
'',
'```html',
'<script>if(typeof window.Int16Array !== \'function\')document.write("<scri"+"pt src=\'extras/typedarray.min.js\'></scr"+"ipt>");</script>',
'<script>document.write("<scri"+"pt src=\'extras/request_animation_frame.js\'></scr"+"ipt>");</script>',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@etpinard Are two document.write script tag insertions in series valid?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know.

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 guess I can't imagine any reason why it wouldn't be. Just not a pattern I ever really use.

@etpinard
Copy link
Contributor

etpinard commented Sep 6, 2016

Thanks for cleaning this up 💃

@rreusser rreusser merged commit e8e1e7f into master Sep 6, 2016
@rreusser rreusser deleted the request-animation-frame-polyfill branch September 6, 2016 14:55
@etpinard etpinard added this to the v1.17.0 milestone Sep 6, 2016
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.

None yet

2 participants