react-lazyload icon indicating copy to clipboard operation
react-lazyload copied to clipboard

Published 20 hours ago • verified publisher icon twobin

Attach to both the window and the parent if overvlow parent is present

Open ninio opened this issue 7 years ago • 10 comments

Proposal to fix #172

NB: Breaks the test checking for rendering only once since it attaches to both parent and window scroll: https://github.com/jasonslyvia/react-lazyload/blob/552315242635d0bf36c5387d644e6eded6641ce6/test/specs/lazyload.spec.js#L74

ninio avatar Apr 02 '18 14:04 ninio

Coverage Status

Coverage remained the same at 75.904% when pulling fe76110e18208831167ec6e77a3151888f27ee96 on ninio:master into 552315242635d0bf36c5387d644e6eded6641ce6 on jasonslyvia:master.

coveralls avatar Apr 02 '18 14:04 coveralls

Seems reasonable, care to fix the tests by the way?

jasonslyvia avatar Apr 03 '18 11:04 jasonslyvia

Yep, I will try to fix them. It might take me some time since I don't fully understand them.

ninio avatar Apr 03 '18 13:04 ninio

Turns out I was not handling unsubscribing of the window scroll which was why the tests failed.

ninio avatar Apr 03 '18 15:04 ninio

Any chance we can get this committed with a new version pushed out? 😄

imjoshdean-tc avatar Apr 13 '18 20:04 imjoshdean-tc

@jasonslyvia?

imjoshdean-tc avatar Apr 16 '18 19:04 imjoshdean-tc

Hey, @jasonslyvia

Is there anything else I can help you with regarding this issue? Maybe a short description of the fix?

Best, Ninio

ninio avatar Apr 18 '18 12:04 ninio

Hey @imjoshdean-tc I will review your PR soon.

ameerthehacker avatar Aug 30 '18 03:08 ameerthehacker

When is "soon?" It's been almost three weeks...

imjoshdean-tc avatar Sep 17 '18 20:09 imjoshdean-tc

hey @imjoshdean-tc sorry for the inconvenience. Within this weekend I will pass it throw, I appreciate your patience!

ameerthehacker avatar Sep 19 '18 09:09 ameerthehacker