feat(Sticky|Visibility): add scroll context

[mariolamacchia]

Fixes #1959.

[codecov-io]

Codecov Report

Merging #1978 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master   #1978      +/-   ##
=========================================
+ Coverage    99.8%   99.8%   +<.01%     
=========================================
  Files         148     148              
  Lines        2568    2572       +4     
=========================================
+ Hits         2563    2567       +4     
  Misses          5       5

Impacted Files	Coverage Δ
src/behaviors/Visibility/Visibility.js	100% <100%> (ø)	⬆️
src/modules/Sticky/Sticky.js	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a4780cd...4f1980f. Read the comment docs.

[TimNZ]

@mariolamacchia Thanks.
If these (https://help.github.com/articles/checking-out-pull-requests-locally/) instructions work I'll pull down PR today and provide feedback.

[TimNZ]

@mariolamacchia Seems to work as expected.

My positioning requirements can be likely be handled using the on* events and css.

Thanks for the rapid PR.

[TimNZ]

@mariolamacchia I spoke too soon.

You didn't change the hardcoded window.* to use the scrollContext instead.
e.g.

window.innerHeight

->

this.props.scrollContext.innerHeight

Sticky sections are staying visible and stacking up as I scroll.

[levithomason]

I'd assume the description here would match the Visibility component:

/** The scroll context visibility should use. */

[levithomason]

nit: Let's match the style of the typings comment, no backticks.

[mariolamacchia]

I'm using the same descriptions from the SemanticUI docs. I'll use this one, without backticks, for both

[levithomason]

Based on the usage, looks like these should be .isRequired.

[mariolamacchia]

same as below

[levithomason]

Needs to be .isRequired here given usage in code.

[mariolamacchia]

There is a default value of window. isRequired means that the user must specify the value, right?

[levithomason]

I'm curious as to the reasoning behind context for Visibility and scrollContext for Sticky. Why different names for these?

<Visibility context={div} />

<Sticky scrollContext={div} />

[mariolamacchia]

@TimNZ scrollContext is only responsible for attaching the scroll event to a different element. If you want it to stick to something else, you should use context.

[mariolamacchia]

@levithomason I'm using the names from the SemanticUI docs, although it isn't very consistent:

• context in Sticky = which element the Sticky should stick to
• context in Visibility = scrollContext in Sticky = context of the scroll event

[levithomason]

Thanks for the explanation @mariolamacchia, let's keep the parity for now then 👍

[levithomason]

Released in semantic-ui-react@0.71.5

[theCuriousOne]

Visibility BREAKS server side rendering!!!

[frogbandit]

Hi @mariolamacchia, thank you for adding the context prop for the Visibility component! 👍 👍 for moving so fast on this.

However, I've tried using it and context doesn't seem to accept either a React ref or an HTML element. I took a look at the source code and it's just supposed to be a PropTypes.object. Can you give some more info on how to use it? i.e. what kind of element should be passed in.

[levithomason]

@frogbandit we have a new component being added to the library to solve this issue, #1879. It is a Ref component.

Stateless functional components do not support refs, so we cannot be sure that a component passed in on context will support refs. Once we land #1879 it will include support for all props that accept DOM nodes and/or components, like context and as.