jonom/jquery-focuspoint

using debounce or requestAnimationFrame for MUCH better performance

Closed this issue · 6 comments

note that calling a callback directly on the resize event is considered a bad habit if major DOM calculations are to be made. This creates havoc (layout thrashing). more about this here:
https://mattandre.ws/2014/05/really-fixing-layout-thrashing/

also, try not to write so much of var. only one single var on the top of the scope is the right way to go..

also, this is bad:
image.css('left', hShift + 'px');
image.css('top', vShift + 'px');

better:
image.css({left:hShift, top:vShift });

best:
image[0].style.cssText = 'left:' + hShift + 'px;' + 'top:' + vShift + 'px;';

since going through jQuery to change the DOM is slower than changing it directly. and since you are doing these changes excessively on the resize event, performance considerations should take place.

Thanks again @yairEO! Please feel free to submit a pull request for this sort of thing - I'm not a javascript expert so appreciate any input you have to make it a better plugin.

On throttling - you can disable the attachment to the resize event through a config setting so you can implement your own throttling, but if you know an easy way to build it in please feel free to submit a PR.

Cheers

Hi,

Great plugin, is very neat and a great idea.

Also as a suggestion you could look into using transforms instead left/top and give a little z value in order to pass the transform to the GPU. That will also improve performance.

Sorry I didn't pulled a request but there's nothing to be compared yet.

I'll try to use your code with GreenSock in order to improve the animation side of it and get back with what I've done.

Cheers,
Rodrigo.

Thanks Rodrigo, I look forward to seeing what you come up with

Great improvements guys.

@yairEO I've added some basic throttling to the plugin - would appreciate your feedback as to whether or not it's on the right track.

the throttling seems to be in order, I haven't checked it yet, only looked at the code.
Also, I would use a lot less variables, so everything with x and y will become an array of [x,y]..it fits better in terms of grouping variables which share the same purpose. Also, try to avoid in all costs writing again and again $(this) and $(self). your are re-creating a jQuery object again and again. Instead, you should cache it by saving it to a variable, like var $elm = $(this) or whatever you think best describes it. I personally create an empty object called DOM - var DOM = {} and fill it up with cached DOM elements, but here you probably have only one, so my first example will suffice.
Also, regarding your $(window).resize... function - It's best practice never to explicitly write .resize( but to use the .on method with a custom namespcae, like so $(window).on('resize.focuspoint', function(){});