Forwarding refs to <Input>-component in ffe-form-react

[paandahl]

Which package is this issue related to: ffe-form-react

Describe your issue (screenshots welcome!)

This is more of a technical feature request. I am working on another internal component at SB1, that adds managed formatting to an input. I want to use the Input from ffe-form, but need to access the DOM ref to manage selection/cursor position. I currently achieve this with ReactDOM.findDOMNode().

There are two potential ways to improve this:

1. Give Input a new property inputRef, that takes a callback and passes it to ref on the contained .

2. Upgrade to React 16.3.0 and use the new forwardRef API.

If there is consensus on such a change, I could make a PR.

BR, Preben Ludviksen

[wkillerud]

Input spreads all given props except className, inline, and textLike (which are used to build the className value) to the input element, so I think what you need is supported already.

const Input = ({ className, inline, textLike, ...rest }) => {
    return (
        <input
            className={classNames(
                'ffe-input-field',
                { 'ffe-input-field--inline': inline },
                { 'ffe-input-field--text-like': textLike },
                className,
            )}
            {...rest} {/* <-- This stuff right here */}
        />
    );
};

So if you set a ref property on Input in your code you should get the ref for the input element.

<Input
   inline={true}
   textLike={true}
   ref={_ref => { this._refs.input = _ref; }}
/>

[ivarni]

I thought the ref prop was kinda magic and never actually passed to the component, like key?

[wkillerud]

Oh, I see.

Then it would be nice to use the new forwardRef feature from React to avoid reinventing the wheel with a custom prop. That means a breaking change, but it's a no-hassle upgrade so it should be fine I'd say :)

[selbekk]

+1 on the forwardRef approach. I'll submit a PR shortly - I have a few minutes to spare anyways, and I've longed for an excuse to try out this API =)

[selbekk]

So I've fixed this in the PR, but enzyme (our React testing framework) doesn't support the feature yet.

I've fixed this temporarily by disabling tests for two components, but I'm not sure if this is good enough for a merge. The fix is merged to master, but not released yet (hasn't been for 3 months).