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

Need a ref prop on the Collapse component #1054

Closed
kilsen opened this issue Jun 1, 2018 · 5 comments · Fixed by #1105 or #1146
Closed

Need a ref prop on the Collapse component #1054

kilsen opened this issue Jun 1, 2018 · 5 comments · Fixed by #1105 or #1146

Comments

@kilsen
Copy link

kilsen commented Jun 1, 2018

Would it be possible to enhance the Collapse component to take a ref prop? I need to be able to determine the offsetHeight of the div element. Also it would be helpful if the Collapse component could emit events when the div has completed being shown (shown.bs.collapse) and completed being hidden (hidden.bs.collapse), so that I know that the height has changed. (I need this in order to re-render other components on the page.)

(Also . . . the version of reactstrap that is in the npm repo does not have the UncontrolledCollapse component.)

@TanninOne
Copy link

Same on Media.
I have a and I'd like to scroll down that list programmatically when items are added, so I need a ref.
Tbh. I think any reactstrap component needs a ref prop.

@kilsen
Copy link
Author

kilsen commented Jun 2, 2018

I was actually able to come up with a workaround in order to get access to the DOM element. The Collapse component accepts a tag prop . . . so, I created my own little stateless functional component that wraps a div, and passes a ref to the div. And I pass my own component in as the tag. (Effectively, it's a DOM "miner" -- it digs the DOM element out of the Collapse component for me.) But I agree; every reactstrap component needs a ref prop.

@illiteratewriter
Copy link
Member

@TheSharpieOne I would like to work on this one. I shouldn't use Forwarding Refs as they are not supported by React versions below 16.3, right?

@TheSharpieOne
Copy link
Member

@illiteratewriter, got for it. And yes, we have to avoid React.forwardRef since we allow React 16.0.0 for now. Just add innerRef and eventually we will refactor in a breaking change (deprecating innerRef at that point and eventually removing it).

@nickrobillard
Copy link

nickrobillard commented Jul 14, 2018

Why doesn't <Collapse> innerRef allow functions, like <Input> innerRef allows?

Input.propTypes = {
  ...
  innerRef: PropTypes.oneOfType([
    PropTypes.object,
    PropTypes.string,
    PropTypes.func,
  ])
};

(The change in 9d3126c only allows PropTypes.object.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants