feat(ffe-form-react): Pass refs for input and textarea #265
Conversation
packages/ffe-form-react/package.json
Outdated
| "react-motion": "^0.5.2", | ||
| "react-test-renderer": "^16.2.0", | ||
| "sinon": "^4.2.1" | ||
| }, | ||
| "peerDependencies": { | ||
| "@sb1/ffe-core": "^12.0.0", | ||
| "@sb1/ffe-form": "^9.0.0", | ||
| "react": "^16.2.0" | ||
| "react": "^16.3.0" |
There was a problem hiding this comment.
This should mirror the devDependencies version since consumers will actually need 16.4.x
| @@ -22,6 +25,15 @@ describe('<Input />', () => { | |||
| expect(wrapper.hasClass('ffe-input-field')).toBe(true); | |||
| }); | |||
|
|
|||
| it('passes refs', () => { | |||
There was a problem hiding this comment.
Maybe expand the description here with why this is needed? Future Developer should be happy with not having to search through the Git log 😉
There was a problem hiding this comment.
I mean, it's a feature / quirk of React - not sure if I want to go around and explain all of those. I'll add a comment, sure, but I don't think it's needed in a general sense.
|
We can make an exception with regards to the code coverage (use admin privileges), but I'd like it if this could wait until next week in case Enzyme is updated. Un-skipping these tests is one of those things that is quickly postponed and/or forgotten. |
|
I agree it's nice to keep up with news from React, but I also agree it's better to hold it off until Enzyme is ready to keep up. Although this probably isn't breaking any code, it would be nice to actually verify it with the tests before merging. |
|
I could solve this a different (and non-breaking way) by letting the user pass in an What do you think - is that a better approach perhaps? |
|
It seems like a lot of extra work just for some tests.. |
|
I'm saying I don't think we should create an API with the intention of breaking it later just over some test library that isn't updated yet. |
BREAKING CHANGE: Requires React 16.4.0 and above! This commit lets the consumer pass refs down to the `Input` and `TextArea` components, to handle focus et al. The only breaking change is the fact that React 16.4.0 is required. This commit also disables two test suites, since Enzyme doesn't yet support React's newest versions. For more information, please refer to this issue: Fixes #231.
selbekk
force-pushed
the
231-use-forwardRef
branch
from
168b8e1 to
6361636
Compare
|
Closing this in favor of a new branch (this one got stale). |
BREAKING CHANGE: Requires React 16.4.0 and above!
This commit lets the consumer pass refs down to the
InputandTextAreacomponents, to handle focus et al.The only breaking change is the fact that React 16.4.0 is required.
This commit also disables two test suites, since Enzyme doesn't yet support
React's newest versions. For more information, please refer to this
issue:
enzymejs/enzyme#1513
The support sounds like they're right around the corner it's awaiting a release), but I think this PR could be reviewed (and perhaps merged) before enzyme get their stuff together.
Fixes #231.