-
Notifications
You must be signed in to change notification settings - Fork 53
Create and initialise default ref prop for Startdust components #417
Comments
There is a long standing issue with similar thing in SUIR, Semantic-Org/Semantic-UI-React#2844. There is also working prototype that uses
I should mention that it should be fully compatible with new const ref = React.createRef()
typeof ref // is object This is a relevant thing for simple components. But is it the expected behaviour for the |
Also I should note that https://reactjs.org/docs/react-dom.html#finddomnode I played around it, https://codesandbox.io/s/6n2z4o7jr Looks as correct solution, but not sure. |
What we actually know?
The PR Semantic-Org/Semantic-UI-React#2844 had a little problem, internal components will be able to forward refs with a composition and no any need to wrap them somehow. Here is example of the wrong tree: <Button as={List.Item} ref={buttonRef} /> // =>
<Button ref={buttonRef}>
<Ref innerRef={buttonRef}>
<List.Item forwaredRef={buttonRef} />
</Ref>
</Button> However, there is a simple solution if we will wrap all our components with This means that Needs more checks but now we have a thing for discussion. |
I also should note that return {
$$typeof: REACT_FORWARD_REF_TYPE,
render,
}; https://github.com/facebook/react/blob/v16.5.2/packages/react/src/forwardRef.js#L42 |
Q1: Should we add automatically refs?This means that a component will receive a <Button ref={node => console.log(node)} /> // will point to `button` element I should note that React's tree will be changed: <ForwardRef>
<Button />
</ForwardRef> Q2: Should we add automatic refs to all our components?This means that some our components will be an edge case: <Button ref={node => console.log(node)} /> // will point to `button` element
<Input ref={node => console.log(node)} /> // will point to `input` element
<Popup ref={node => console.log(node)} /> // will point to what??? As proposed solution: <Popup
ref={node => console.log(node)} // throw a warning and will give `null`
contentRef={node => console.log(node)} // will point to `div`
trigger={<Button />}
triggerRef={node => console.log(node)} // will point to `button`
/> |
This comment describes our solutions to this issue. Automatic refsAll our components will implement automatic refs that will point to matching DOM elements. <Button ref={node => console.log(node)} /> // will point to `button` element
<Input ref={node => console.log(node)} /> // will point to `input` element BackgroundYes, it violates React's behaviour when you will have an instance of a component in When you're dealing with an instance you're starting to expore DOM methods like This also solves your problem with libraries like <Segment ref={(node) => {
node.focus() // will fail now
node.getBoundingClientRect() // and this too
}} /> Edge casesWe decided to go with this API for complex components and we will not expose refs for component parts. You can use shorthand API for this: <Popup
// Will throw a warning and will give `null`
// Use refs to component's parts via shorthands
ref={node => console.log(node)}
/>
<Popup
// Use shorthand API there, it will point to `div`
content={{ content: 'Foo', ref: node => console.log(node)}}
// We just clone element, please you refs there
trigger={<Button ref={node => console.log(node)} />}
// Your component doesn't support refs? Use Ref!
trigger={(
<Ref innerRef={node => console.log(node)} />
<MaterialButton />
</Ref>
)}
/> Implementation[x]
|
Feature Request
Create default ref for Startdust components that should point to the first DOM Element in the renderComponent.
Problem description
While developing the Dropdown component and using other Stardust components, I've used Input and List and had to manually expose refs for those components in order to use them in the Dropdown implementation (input focus, list passed to Downshift).
Proposed solution
A default ref prop should be added for each component probably in the renderComponent and initialised with the first DOMElement from the render function. (the List should've had the ElementType
<ul>
).This ref prop should also be easy to override in case we want custom behaviour.
This default ref prop should not impact the addition of other ref-like props. For instance, in Input, we can still have the inputRef pointing to the
<input>
and the default ref pointing to the<div>
enclosing it.The text was updated successfully, but these errors were encountered: