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

Link component should forward ref #1053

Closed
pzi opened this issue Jun 8, 2022 · 1 comment
Closed

Link component should forward ref #1053

pzi opened this issue Jun 8, 2022 · 1 comment

Comments

@pzi
Copy link
Contributor

pzi commented Jun 8, 2022

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

As JSS exports a shared Link component it should accept a ref prop in order to access their DOM nodes and for managing focus, selection, or animations.

[...] it can be useful for some kinds of components, especially in reusable component libraries.

Source: https://reactjs.org/docs/forwarding-refs.html

Describe the solution you'd like

Set up the Link component using React.forwardRef to allow refs to be passed to the underlying anchor. This change would have to be made in nextjs and react jss packages.

https://github.com/Sitecore/jss/blob/v20.0.3/packages/sitecore-jss-nextjs/src/components/Link.tsx#L20
https://github.com/Sitecore/jss/blob/v20.0.3/packages/sitecore-jss-react/src/components/Link.tsx#L41

Additional information

Not having a ref prop means consumers don't have access to the underlying DOM components.

Unsure if you would be happy to release it as a minor version given this note in the React docs:

Note for component library maintainers

When you start using forwardRef in a component library, you should treat it as a breaking change and release a new major version of your library. This is because your library likely has an observably different behavior (such as what refs get assigned to, and what types are exported), and this can break apps and other libraries that depend on the old behavior.

Additionally, I assume there will be other components that would benefit from having a ref prop.

@pzi pzi changed the title Link component should forward refs Link component should forward ref Jun 8, 2022
@illiakovalenko
Copy link
Contributor

Added in #1080, will be released in the next major version

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

No branches or pull requests

2 participants