angular-sticky icon indicating copy to clipboard operation
angular-sticky copied to clipboard

Pagescroll bug fix

Open pavel1992 opened this issue 7 years ago • 6 comments

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.

pavel1992 avatar Aug 09 '17 12:08 pavel1992

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

harm-less avatar Aug 09 '17 13:08 harm-less

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.

pavel1992 avatar Aug 09 '17 13:08 pavel1992

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.

harm-less avatar Aug 09 '17 13:08 harm-less

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

pavel1992 avatar Aug 09 '17 13:08 pavel1992

I posted it over here: image

harm-less avatar Aug 09 '17 14:08 harm-less

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.

pavel1992 avatar Aug 09 '17 14:08 pavel1992