Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: context prop to Visibility component #1959

Closed
frogbandit opened this issue Aug 12, 2017 · 7 comments
Closed

Feature request: context prop to Visibility component #1959

frogbandit opened this issue Aug 12, 2017 · 7 comments

Comments

@frogbandit
Copy link

Apologies if this is not the right forum to make this request, but it would be great to have a context prop for the Visibility component, much like you can change the context in Semantic UI: https://semantic-ui.com/behaviors/visibility.html#/settings.

For example. I want to change the scroll event listener to listen on the scroll of a surrounding component (i.e. a component which has a height less than the window height, but has an overflow so is scrollable).

Currently, when this component is scrolled, none of the Visibility component's props fire because it does not think that the window is being scrolled.

Thanks!

@levithomason
Copy link
Member

Yep, we should accept a component and/or DOM node for this. Note, we should also use the <Ref /> component from #1879 in order to ensure we can get the DOM node from components passed in.

@mariolamacchia
Copy link
Contributor

There is the same issue on the Sticky (#1747), I can fix both of them.

@levithomason I would just add a prop that accepts an HTML element. Or do you think of something different using <Ref />?

@frogbandit
Copy link
Author

Hi there, 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.

@mariolamacchia
Copy link
Contributor

@frogbandit context and scrollContext accept an html element. Here is an example of how to use context with Sticky.

Beware that React does not support using ref on stateless components.

@levithomason
Copy link
Member

@frogbandit @mariolamacchia Note we are adding a Ref component that will enable SUIR to support React elements everywhere we support DOM nodes. This includes stateless functional components. I responded here #1978 (comment) as well. You can see #1879 for the work being done.

@sbrouil
Copy link

sbrouil commented Nov 22, 2017

@levithomason with the current implementation v0.76.0 on Visibility component scroll event are only atteched to the context property in componentDidMount() so it doesn't work well when the context is changed.

    handleContextRef = (contextRef) => this.setState({ contextRef })

    render() {
        const { contextRef } = this.state; // contextRef is undefined no first render() call
        return (
            <div ref={this.handleContextRef}  style={{ height: 700, overflowX: 'auto' }} >
                <div style={{ height: 1200 }} />

                <Visibility
                    context={contextRef}
                   ...
            <div>
         );
     }

A workaround is to add a check on contextRef so that Visibility component is only mounted when contextRef is defined.

    render() {
        const { contextRef } = this.state; // contextRef is undefined no first render() call
        return (
            <div ref={this.handleContextRef}  style={{ height: 700, overflowX: 'auto' }} >
                <div style={{ height: 1200 }} />
               { contextRef &&
                <Visibility
                    context={contextRef}
                    ...
               }
            <div>
         );
     }

@layershifter
Copy link
Member

If you have an issue, create a new issue.

@Semantic-Org Semantic-Org locked and limited conversation to collaborators Nov 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants