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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(react-container): add forwardRef #567

Merged
merged 2 commits into from
Jul 3, 2022

Conversation

danielsimao
Copy link
Contributor

Closes #566

馃摑 Description

Allow passing ref to Container

鉀筹笍 Current behavior (updates)

Doesn't allow to pass ref.

馃殌 New behavior

Allows passing ref.

馃挘 Is this a breaking change (Yes/No):

No

packages/react/src/container/container.tsx Show resolved Hide resolved
packages/react/src/container/container.tsx Outdated Show resolved Hide resolved
packages/react/src/container/container.tsx Outdated Show resolved Hide resolved
@danielsimao
Copy link
Contributor Author

here please pass a ref from the function useDomRef, something like this

   import {useDOMRef} from "../utils/dom";
   //....
   const domRef = useDOMRef(ref);
 // .....
 return (
      <StyledContainer
        ref={domRef}
//......

I made the change. Out of curiosity, could you please explain why you prefer this way, even tho you do not give any use to the ref created inside the component?

@jrgarciadev jrgarciadev changed the base branch from main to next July 2, 2022 20:23
Copy link
Member

@jrgarciadev jrgarciadev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot, @danielsimao 馃檹馃徎, I prefer the way of using the ref through the useDomRef because it customises the instance value that is exposed to parent components, it allows us to have better control of the internal ref, here's the official docs:
https://reactjs.org/docs/hooks-reference.html#useimperativehandle

@jrgarciadev
Copy link
Member

Could you please fix the conflicts? @danielsimao 馃檹馃徎

@danielsimao
Copy link
Contributor Author

@jrgarciadev done

@jrgarciadev
Copy link
Member

Thanks a lot! @danielsimao 馃殌

1 similar comment
@jrgarciadev
Copy link
Member

Thanks a lot! @danielsimao 馃殌

@jrgarciadev jrgarciadev merged commit 2ae0870 into nextui-org:next Jul 3, 2022
@jrgarciadev jrgarciadev added this to the 馃殌 v1.0.0-beta.10 milestone Jul 3, 2022
@jrgarciadev jrgarciadev mentioned this pull request Aug 26, 2022
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 this pull request may close these issues.

[BUG] - Container: doesn't accept ref prop
2 participants