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

Use forwardRef with IconBase #336

Open
with-heart opened this issue Jul 8, 2020 · 11 comments · May be fixed by #367
Open

Use forwardRef with IconBase #336

with-heart opened this issue Jul 8, 2020 · 11 comments · May be fixed by #367

Comments

@with-heart
Copy link

Is your feature request related to a problem? Please describe.

Chakra UI provides a Tooltip component that displays a tooltip when the component's children are hovered. In order to make this functionality work, we depend on the outermost child of Tooltip accepting a ref. Since IconBase does not forward refs, we are unable to make Tooltip work correctly if the outermost child is a react-icons icon.

See chakra-ui/chakra-ui#683 for a related issue on our end.

Describe the solution you'd like

Integrate React.forwardRef into IconBase so that icons generated by react-icons can accept a ref, increasing their flexibility and reusability.

See https://reactjs.org/docs/forwarding-refs.html#forwarding-refs-to-dom-components for more details.

Describe alternatives you've considered

The only solution we have for our users is that they should wrap their react-icons icons with a component that forwards refs if they would like to wrap the icon with Tooltip.

Additional context

This would be a breaking change. https://reactjs.org/docs/forwarding-refs.html#note-for-component-library-maintainers

@dzcpy
Copy link

dzcpy commented Aug 23, 2023

Really need this. Please consider it

@divx-alisson-ninomiya
Copy link

Bumping this issue. Would be nice to have this.

@Duality1k
Copy link

been 3 years and still no fix, crazy

@with-heart
Copy link
Author

@KeUnstackDetachProcess if someone was to reimplement #367 (it's 3 years old at this point) and address the concerns in the PR comments, I'm sure @kamijin-fanta would be open to accepting the change. Just need someone to champion the change.

@wuifdesign
Copy link

would be really nice to have this

@DamSenViet
Copy link

Just found out I have use case for using the motion HOC from framer-motion. Would love to see a ref that's passed to the svg itself.

@fauri13
Copy link

fauri13 commented Jan 11, 2024

I've created a PR that should do the trick.

@kamijin-fanta
Copy link
Member

I may provide an API to add wrapper elements in the future. #719 Are the props to be added "innerRef" which always exposes a reference to the SVG element or "ref" which exposes a reference to the top level element?

@fauri13
Copy link

fauri13 commented Jan 16, 2024

I may provide an API to add wrapper elements in the future. #719 Are the props to be added "innerRef" which always exposes a reference to the SVG element or "ref" which exposes a reference to the top level element?

In my PR, it forwards the ref using React.forwardRef, so it exposes the "ref" prop correctly.

@DamSenViet
Copy link

DamSenViet commented Jan 16, 2024

Had a chance to look at fauri's PR. The ref is set up to the svg element inside the IconBase.

@vincerubinetti
Copy link

Can't believe this wasn't in here from the start.

However I think this will no longer be a problem with React 19, because the ref should automatically be forwarded (assuming all other props are passed e.g. {...props}).

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.

9 participants