-
Notifications
You must be signed in to change notification settings - Fork 115
Support new labelRenderer
and valueRenderer
properties
#18
Conversation
Temporarily closing this PR while I investigate a performance issue. Will re-open once I've identified it. |
@bvaughn thanks a lot! Keep me updated. |
Happy to help. Will do! I noticed a performance decrease in the branch of bvaughn/redux-devtools-filterable-log-monitor that integrated with this update and I just wanted to rule out the change here before submitting. I'll keep you posted. :) |
…e expanded by default
I identified the performance issue. My branch accidentally introduced a regression- all nodes were expanded by default instead of just the root. Easy enough to fix. :) |
```js | ||
<JSONTree | ||
labelRenderer={raw => <strong>raw</strong>} | ||
valueRenderer={raw => <em>raw</em>} |
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.
Should be <strong>{ raw }</strong>
(same for em
) instead
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.
Whoops! Nice catch. I need to pay more attention to my code examples. :)
Added one comment but this looks good. Thank you so much for cleaning it up! (and say hi to Treasure Data folks) I tried to run I'm fine w/ dropping the support for 0.13. Just let me know if I should. I'd also add this to <h3>More Fine Grained Rendering</h3>
<p>Pass <code>labelRenderer</code> or <code>valueRenderer</code>.</p>
<div>
<JSONTree data={ data }
labelRenderer={raw => <strong>{ raw }</strong>}
valueRenderer={raw => <em>{ raw }</em>} />
</div> |
…to fix an error. Added custom label/value renderer snippet to example App.
Not a problem! Thanks for the useful library. :) Hm. I'm unsure about the issue you pointed out. I don't specifically remember using any React 0.14.x features. That being said, updating the example package to React 0.14.x did resolve the problem- so if you don't mind me bumping that dependency with this PR- that would certainly be the most straight forward solution. I updated the example App with the blurb you suggested. Thanks for that. |
@bvaughn sounds good, thank you! |
Support new `labelRenderer` and `valueRenderer` properties
You're quite welcome! :) |
@bvaughn published 0.4.0. |
Thanks so much for the speedy update! |
In order to support bvaughn/redux-devtools-filterable-log-monitor/issues/8, I need finer-grained control over what is rendered for node keys and values. So I set out to add 2 new properties:
labelRenderer
andvalueRenderer
.Along the way I also noticed a lot of repetition between classes (
JSONBooleanNode
,JSONDateNode
,JSONFunctionNode
,JSONNullNode
,JSONNumberNode
, andJSONUndefinedNode
- and evenJSONArrayNode
,JSONIterableNode
, andJSONObjectNode
). So I did some refactoring to tidy things up a bit. I hope I didn't take too much liberty by doing this.I'd love to see this feature merged and I'd be willing to make any changes you feel necessary. Please feel free to reach out to me and we can chat. :)