Skip to content
This repository has been archived by the owner on Nov 2, 2019. It is now read-only.

Supporting refs for internal and external use #272

Closed
jamesplease opened this issue Aug 20, 2018 · 4 comments
Closed

Supporting refs for internal and external use #272

jamesplease opened this issue Aug 20, 2018 · 4 comments
Labels
enhancement New feature or request refactor Changing existing code

Comments

@jamesplease
Copy link
Owner

jamesplease commented Aug 20, 2018

Presently, there is a divide of sorts when it comes to Materialish refs. If a ref is used internally, then it cannot be used externally. And vice versa.

There are only a few situations when refs are needed internally. But those components do not have a nodeRef prop.

@conorhastings proposed a solution to this in #267 (comment) (CodeSandbox Example).

There is a catch: it doesn't support string refs. But that may okay, given the benefits that this API provides, and the fact that string refs are deprecated.

@jamesplease jamesplease added enhancement New feature or request refactor Changing existing code labels Aug 20, 2018
@conorhastings
Copy link

thinking further about this, not allowing string refs seems especially fine since the prop is nodeRef, they would actually have direct access via this.refs['string'] they would have to attach a direct ref to the materialish node and use this.refs['something'].children.refs['string']

I think calling out lack of support for string refs in nodeRef in docs seems appropriate since above workflow is really poor anyway

@jamesplease
Copy link
Owner Author

Over in #240 , this would be pretty useful to enable us to refocus the input after the clearable button is clicked.

@jamesplease
Copy link
Owner Author

jamesplease commented Aug 21, 2018

Here is what I'm thinking:

getRef = ref => {
  const { nodeRef } = this.props;

  if (typeof nodeRef === 'function') {
    nodeRef(ref);
  } else if (nodeRef && nodeRef.hasOwnProperty('current')) {
    nodeRef.current = ref;
  }

  this.nodeRef = ref;
}

@jamesplease
Copy link
Owner Author

Resolved in #277 . This will be released as 0.17.0; possibly tonight or tomorrow. I'm just doing a bit more testing to make sure that everything is good.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request refactor Changing existing code
Projects
None yet
Development

No branches or pull requests

2 participants