feat: introduce rowRenderer #40
Conversation
|
Hey @P0lip , need a hand with this refactoring? |
| validations={{ | ||
| ...('annotations' in schemaNode && | ||
| schemaNode.annotations.default && { default: schemaNode.annotations.default }), | ||
| ...('validations' in schemaNode && schemaNode.validations), |
There was a problem hiding this comment.
@P0lip it might be just me but I find it a bit intimidating to read. Would you mind wrapping this with a small helper function? E.g. getAnnotations() + getValidations()?
| const { hideTopBar, name, treeStore, maxRows, className, onGoToRef } = props; | ||
| const { hideTopBar, name, treeStore, maxRows, className, onGoToRef, rowRenderer: customRowRenderer } = props; | ||
|
|
||
| React.useEffect( |
There was a problem hiding this comment.
I was thinking the other day on our code coverage in our React repos. It's tough to test React I agree but here's a though: why don't we embrace a pattern where we encapsulate all 'logic' before the return/render function inside external controller?
The idea is that you would move those effects in a separate component / function which could then be unit tested very easily.
| [onGoToRef, node], | ||
| ); | ||
|
|
||
| return ( |
There was a problem hiding this comment.
Similarly to my other comment all code between line 14 and return should/could be unit tested. What do you think?
chris-miaskowski
left a comment
chris-miaskowski
left a comment
There was a problem hiding this comment.
I gave your code a light pass and left some comments, curious to hear your thoughts.
I'll leave it up to @lottamus to give the final approval (trying to encourage the 'code owners' thingy and seems that you and Chris mostly own this bit)
|
Going to go ahead and merge this to keep things moving along. Regarding the extra unit tests, I think it does still make sense and perhaps we tackle it in a separate PR? |
|
🎉 This PR is included in version 2.6.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
List of changes:
rowRendererRight(please refer to feat: introduce rowRendererRight #37 (comment) for more details)Needed to unyalc JSV in Studio.