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

Do not reset newly created scroller; reset height on destroy #278

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

netzkind
Copy link

  • do not reset a newly created scroller, thus saving ressources and speeding creation up
  • set 'height'='' to $el on destroy - this is far from perfect, but it works for any site not setting the height to this element inline; without this, height will be stuck on destroy, so the pages rendering is not returned to normal

…peeding creation up

- set 'height'='' to $el on destroy - this is far from perfect, but it works for any site not setting the height to this element inline; without this, height will be stuck on destroy, so the pages rendering is not returned to normal
@kristerkari
Copy link
Collaborator

Is there a problem with the performance when creating the scroller? Just trying to figure out if this change is necessary.

@kristerkari
Copy link
Collaborator

@netzkind ping

@netzkind
Copy link
Author

netzkind commented Feb 5, 2015

Sorry, i didn't think you are serious about this.
The reset-method in nanoscroller does loads of things, none of which is necessary to do twice to a newly created element. If there is no need to do something, then why do it?

So yes, it's a performance-problem, especially when used on low-end-devices.

@kristerkari
Copy link
Collaborator

@netzkind Thanks for the explanation.

I'll need to test this out on different browsers before I can pull it in. Of course if you have time or interest to help with that, then merging these changes will be easier.

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