search

harm-less / angular-sticky

Pure javascript AngularJS directive to make elements stick when scrolling
http://harm-less.github.io/angular-sticky
83 stars 38 forks source link

Pagescroll bug fix #30

Open pavel1992 opened 7 years ago

pavel1992 commented 7 years ago

When using angular sticky directive in project, found the bug with page scrolling at new element on page adding. It was because throttledResize function triggered even if window not actually resized. This pull request fixes that bug with comparing width before and after resize event.

harm-less commented 7 years ago

Hello,

Thanks for the pr. I notice it doesn't pass the unit tests, can you take a look at that? I've posted some code-review comments in your changes

pavel1992 commented 7 years ago

Hello, it seems test fails because it throws resize event with no width actually changing, so after my fix it doesnt triggers resize function. I don't know at this moment is this expected behaviour or not, but IMHO you shouldnt trigger resize when it was no really resizing.

'window.dispatchEvent(new Event('resize'));' <- thats why test fails.

harm-less commented 7 years ago

I don't remember exactly, but I'm pretty sure it's their for a reason. I think it will automatically be resolved if you fix the review comment I gave because than there's something to flush.

pavel1992 commented 7 years ago

Hm i m sorry, but i cant find where did u post review comments. And here i can see 'no reviews' in reviewers list.

harm-less commented 7 years ago

I posted it over here: image

pavel1992 commented 7 years ago

I couldnt see your comments because it seems you didnt finish your review. By the way, i ll think, how to do fixes you wrote about.