Skip to content

Commit 892f404

Browse files
johnkeane475layershifter
authored andcommitted
fix(Visibility): handle context scroll instead of just window scroll (#3400)
* fix(Visibility): handle context scroll instead of just window scroll * fix(Visibility): refactor and uncomment failing test * add new test, fix broken behavior * add note * remove comment * Update Visibility.js * Update Visibility.js
1 parent ca81dd8 commit 892f404

File tree

3 files changed

+33
-7
lines changed

3 files changed

+33
-7
lines changed

src/behaviors/Visibility/Visibility.js

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ export default class Visibility extends Component {
209209
if (!isBrowser()) return
210210
const { context, fireOnMount, updateOn } = this.props
211211

212-
this.pageYOffset = window.pageYOffset
212+
this.pageYOffset = this.getPageYOffset()
213213
this.attachHandlers(context, updateOn)
214214

215215
if (fireOnMount) this.update()
@@ -310,7 +310,7 @@ export default class Visibility extends Component {
310310

311311
this.oldCalculations = this.calculations
312312
this.calculations = this.computeCalculations()
313-
this.pageYOffset = window.pageYOffset
313+
this.pageYOffset = this.getPageYOffset()
314314

315315
const {
316316
onBottomPassed,
@@ -364,7 +364,8 @@ export default class Visibility extends Component {
364364
const { bottom, height, top, width } = this.ref.current.getBoundingClientRect()
365365
const [topOffset, bottomOffset] = normalizeOffset(offset)
366366

367-
const direction = window.pageYOffset > this.pageYOffset ? 'down' : 'up'
367+
const newOffset = this.getPageYOffset()
368+
const direction = newOffset > this.pageYOffset ? 'down' : 'up'
368369
const topPassed = top < topOffset
369370
const bottomPassed = bottom < bottomOffset
370371

@@ -397,6 +398,17 @@ export default class Visibility extends Component {
397398
}
398399
}
399400

401+
getPageYOffset() {
402+
const { context } = this.props
403+
404+
if (context) {
405+
// Heads up! `window` doesn't have `pageYOffset` property
406+
return context === window ? window.pageYOffset : context.scrollTop
407+
}
408+
409+
return 0
410+
}
411+
400412
// ----------------------------------------
401413
// Render
402414
// ----------------------------------------

test/setup.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ let info
5050
let warn
5151
let error
5252

53-
const throwOnConsole = method => (...args) => {
53+
const throwOnConsole = (method) => (...args) => {
5454
throw new Error(
5555
`console.${method} should never be called but was called with:\n${args.join(' ')}`,
5656
)

test/specs/behaviors/Visibility/Visibility-test.js

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,20 @@ describe('Visibility', () => {
276276
calculations: { direction: 'up' },
277277
})
278278
})
279+
280+
it('gets direction from `context` element', () => {
281+
const context = document.createElement('div')
282+
const onUpdate = sandbox.spy()
283+
284+
sandbox.stub(context, 'scrollTop').value(0)
285+
mount(<Visibility context={context} onUpdate={onUpdate} />)
286+
287+
sandbox.stub(context, 'scrollTop').value(5)
288+
domEvent.scroll(context)
289+
onUpdate.should.have.been.calledWithMatch(null, {
290+
calculations: { direction: 'down' },
291+
})
292+
})
279293
})
280294
})
281295

@@ -367,8 +381,8 @@ describe('Visibility', () => {
367381
const opts = { [callbackName]: callback }
368382

369383
const offset = 10
370-
const falsyCond = _.map(_.first(falsy), value => value + offset)
371-
const truthyCond = _.map(_.first(truthy), value => value + offset)
384+
const falsyCond = _.map(_.first(falsy), (value) => value + offset)
385+
const truthyCond = _.map(_.first(truthy), (value) => value + offset)
372386

373387
wrapperMount(<Visibility {...opts} offset={offset} />)
374388
mockScroll(...truthyCond)
@@ -546,7 +560,7 @@ describe('Visibility', () => {
546560
describe('updateOn', () => {
547561
beforeEach(() => {
548562
requestAnimationFrame.restore()
549-
sandbox.stub(window, 'requestAnimationFrame').callsFake(fn => setTimeout(() => fn(), 0))
563+
sandbox.stub(window, 'requestAnimationFrame').callsFake((fn) => setTimeout(() => fn(), 0))
550564
})
551565

552566
it('defaults to "events"', () => {

0 commit comments

Comments
 (0)