Skip to content
This repository has been archived by the owner on Jul 29, 2019. It is now read-only.

Use requestAnimationFrame to throttle redraws #2091

Merged

Conversation

gkubisa
Copy link
Contributor

@gkubisa gkubisa commented Sep 13, 2016

It allows the browser to determine the best rate at which redraws should happen and avoids choppy scrolling when Chrome starts throttling setTimeout (see https://docs.google.com/document/d/163ow-1wjd6L0rAN3V_U6t12eqVkq4mXDDjVaA4OuvCA/edit#)

@mojoaxel
Copy link
Member

@gkubisa Thanks very much! Thats a good idea! It may be even fix #2023.

Could you please remove the changes in the dist folder. We normally no not update this folder in the developing branch. This should work something like this:

git reset HEAD~1 dist
git commit --amend --no-edit
git push --force

@gkubisa gkubisa force-pushed the requestAnimationFrame-for-redraws branch from 3958c45 to ff3948f Compare September 20, 2016 09:47
@gkubisa
Copy link
Contributor Author

gkubisa commented Sep 20, 2016

@mojoaxel, I've just removed those changes to the dist directory

@yotamberk yotamberk merged commit c130f0b into almende:develop Oct 15, 2016
@yotamberk
Copy link
Contributor

@mojoaxel This merge is causing a serious problem with the throttle redraw. I causes asynchronous behavior and causes #2307.
I don;t know how to solve this problem. I think we need to revert this merge.

@gkubisa
Copy link
Contributor Author

gkubisa commented Nov 30, 2016

@yotamberk, reverting this PR would cause another regression (#2023). Would it be possible to just trigger synchronous redraw on initialization? If necessary, certain other actions could also trigger a synchronous redraw, eg updating the chart data programmatically.

@yotamberk
Copy link
Contributor

@gkubisa I understand. I've tried solving the bug it caused without much success... Can you take a look at this?

@yotamberk
Copy link
Contributor

Nvm, I've submitted a PR to solve the async problem (#2386)

@mojoaxel
Copy link
Member

This was a breaking change. This should have triggered a MAJOR release. See #2511

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

Successfully merging this pull request may close these issues.

3 participants