-
-
Notifications
You must be signed in to change notification settings - Fork 9.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Show JSX if PropVal is a React element #1455
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1455 +/- ##
==========================================
- Coverage 14.6% 14.27% -0.33%
==========================================
Files 202 201 -1
Lines 4655 4636 -19
Branches 528 504 -24
==========================================
- Hits 680 662 -18
- Misses 3519 3553 +34
+ Partials 456 421 -35
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thanks for the PR!
Can you add an example of this in the kitchen sink example app? You should also take a quick glance at the existing info usages there and see that nothing breaks.
@@ -83,9 +87,16 @@ function previewObject(val, maxPropObjectKeys) { | |||
} | |||
|
|||
export default function PropVal(props) { | |||
const { maxPropObjectKeys, maxPropArrayLength, maxPropStringLength } = props; | |||
const { | |||
braceWrap: _branceWrap, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_braceWrap
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice catch
@@ -26,7 +26,7 @@ function getData(element) { | |||
} | |||
|
|||
if (typeof element === 'number') { | |||
data.text = String.toString(element); | |||
data.text = Number.prototype.toString.call(element); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this achieve something different than element + ''
?
@@ -46,6 +46,7 @@ export default function Node(props) { | |||
const { | |||
node, | |||
depth, | |||
showSourceOfProps, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this should get passed as a context instead.
To answer your question on the rendering part, I don't think any of us are married to a specific method. A lot of the decisions were made way back when it was hard to predict the complexity of info. If you think there's a more logical way to render it, we'd love a PR. |
I'll try to elaborate on a more robust way to render the source. I think the copy & paste use case is big enough issue for me to stop using padding-left as indentation. I'll add an example to the kitchen sink. Thanks for the heads up. |
@sventschui could you have a look at the merge conflicts? 🙇 |
@ndelangen I think you can close this PR. This will yield some errors. The source code generation thingy needs to be refactored |
Issue:
What I did
JSX elements within prop values are now rendered completely (in Story Source and Prop Table). The behaviour can be reverted by configuring
showSourceOfProps: false
.This yields some errors in the source code generation (with indentation).
I think it would be easier to do the JSX source generation outside of React components since there are a lot of "inverted" dependencies. Rendering the parent depends often on whether the child has multiple lines of code or just a single one.
There is some paddingLeft used for indentation currently what breaks copy & paste behaviour. I think some of the issues outlined could be fixed by using paddingLeft in some cases but I'd rather not do that (because copy & paste).
Would you be open to a diffrent approach on rendering the source code?
How to test
Create a story with a property value of type JSX element.