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

Not all components forward refs #1755

Closed
haynesjm42 opened this issue Mar 5, 2020 · 4 comments · Fixed by #2268
Closed

Not all components forward refs #1755

haynesjm42 opened this issue Mar 5, 2020 · 4 comments · Fixed by #2268

Comments

@haynesjm42
Copy link
Member

Grid does not forward its ref, and to make things even more complicated it already gets a ref to its outer frame for its own use. This library looks promising for dealing with assigning multiple refs to the same DOM element: https://www.npmjs.com/package/@seznam/compose-react-refs

Mobile panel is another component which does not forward refs. We need to audit all components to ensure we are forwarding appropriately.

See https://github.com/xh/hoist-react/compare/gridRef for an example of using that library for dealing with multiple refs.

@lbwexler
Copy link
Member

This is very promising and an important piece of the puzzle. See this discussion here:
facebook/react#13029

@cnrudd
Copy link
Member

cnrudd commented Mar 31, 2020

@lbwexler
Copy link
Member

We have added composeRefs to develop, so we should now have all the tools we need to work on this.

@lbwexler
Copy link
Member

Think we are now all familiar with the composeRefs tool, which has been great to let us wrangle external and internal refs whenever we need to and was looking to close this out,

Certainly for mobile Panel, I think we want to do this for consistency with Desktop Panel, and it seems like a basic function of Panel. I will create a PR for that now.

But I see that for the most part, our big signature components with corresponding Models -- Grid, DataView, DashContainer, Form, DockContainer, Chart,FilterChooser, GroupingChooser do not actually pass a forwardRef to the component they use for internal rendering.

I think it would be straightforward to do this, but I just want to make sure we want to? Is this for purpose of this to be able to get a Dom element for aligning things to these components or scrolling to them etc without putting them in other containers.

The reason I ask is that another option (and I know it sounds crazy) would be to somehow have the ref that is returned from these components be an imperative handle to the model of the component in question. That would be another "big" way of structuring apps, that I don' think we want to introduce, but just wanted to confirm here before we preclude it.

@lbwexler lbwexler linked a pull request Jan 29, 2021 that will close this issue
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants