-
Notifications
You must be signed in to change notification settings - Fork 114
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
Jewels/add custom viz and new relic component guide #1186
Jewels/add custom viz and new relic component guide #1186
Conversation
it seems like Snyk is failing on all three open PRs right now , but I don't have access to see what's causing the failure |
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.
@JuliaNocera Great job on this guide! Overall, I think it's very informative and makes great use of the dev site tools. I have a few suggestions on style and language , but feel free to push back.
Besides what's in the review comments, I have another note on the guide, in general: I think it'd be helpful to include in-progress images after each section that it makes sense. This way, users can visually track their progress.
A lot of these comments apply to the next PR, as well, so I'll let you respond to/modify these PRs and then do a final edit once that's all in.
Thanks!
|
||
<Intro> | ||
|
||
Now that you have a basic visualization, let's customize it! You can use all of the <Link to="explore-docs/intro-to-sdk">New Relic One SDK</Link> components in a Custom Visualization the same way you can use them in a Nerdlet. In this guide, we will add a new chart type and a `SegmentedControl` component. You will be able to dynamically swap between the Radar chart and the new chart right from the browser. |
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.
Throughout this guide, you switch between first and second person points of view:
First person:
In this guide, we will add a new chart type and a
SegmentedControl
component.
Second person:
You will be able to dynamically swap between the Radar chart and the new chart right from the browser.
In our dev site guides, we should try to use the second person point of view in all cases it makes sense:
In this guide, you'll add a new chart type and a
SegmentedControl
component.
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.
✅ (for both guides)
|
||
To get started, follow the <Link to="/build-apps/build-visualization">Build a custom visualization for dashboards</Link> guide to get setup with your <Link to="https://newrelic.com/signup?utm_source=developer-site">New Relic</Link> account, install the <Link to="/explore-docs/nr1-cli">New Relic One CLI</Link>, and create your first visualization. We will use the visualization you create in that guide as a starting point for this guide. | ||
|
||
## Setup the visualization component state |
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.
"Setup" is a noun. The phrasal verb "Set up" makes more sense here:
Set up the visualization component state
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.
✅
|
||
## Setup the visualization component state | ||
|
||
In this first set of steps you will add component state to your visualization template form the previous guide referrenced above. |
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.
To sound more conversational, it's best to prefer contractions:
In this first set of steps you'll add component state...
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.
"form" -> "from" ?
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.
"referrenced" -> "referenced" (sp)
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.
✅
From the root of the Nerdpack project you created in the previous guide, navigate to `/visualizations/<your-visualization>/index.js`. We will continue to work in the `index.js` file for the rest of this guide. Add a chart types constant above the first line of the class. | ||
|
||
|
||
```js |
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.
In the lead-in text, you directed the user to where in the file the new code should go:
Add a chart types constant above the first line of the class
Instead, you can show them where it should go by including the whole code file. I understand this is a large file, but I think avoiding confusion is worth it. Also the webpage renders it as a fixed-size codeblock.
You can also include the index.js file name (fileName
) and line highlights (lineHighlight
).
So, the result would look like:
This is very clear on where exactly the new code should go and allows the user to just copy and replace the whole file in their local project.
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.
✅
|
||
</Step> | ||
|
||
After these two steps, the beginning of you `index.js` file should look like this: |
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.
Above, I mentioned including the whole index.js file in these code blocks. If you do that, you can remove this text and the following codeblock.
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.
"of you" -> "of your"?
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.
✅
|
||
</Step> | ||
|
||
Now <Link to="/build-apps/build-visualization#render-the-visualization-in-local-development">render the visualziation in local development</Link> to see the new Treemap chart below your Radar chart. |
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.
typo: visualziation -> visualization
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.
✅
: | ||
<Treemap /> | ||
} | ||
``` |
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.
It'd be helpful to do a little bit of hand-holding here and explain what the conditional does, in addition to the directive "wrap these things in a ternary":
Here, you compare
selectedChart
toCHART_TYPES.Radar
. IfselectedChart
is aRadar
, you render aRadarChart
. Otherwise, you render aTreemap
.
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.
✅
|
||
## Before you begin | ||
|
||
To get started, follow the <Link to="/build-apps/build-visualization">Build a custom visualization for dashboards</Link> guide to get setup with your <Link to="https://newrelic.com/signup?utm_source=developer-site">New Relic</Link> account, install the <Link to="/explore-docs/nr1-cli">New Relic One CLI</Link>, and create your first visualization. We will use the visualization you create in that guide as a starting point for this guide. |
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.
Great use of links!
src/markdown-pages/build-apps/customize-visualization-with-New-Relic-components.mdx
Show resolved
Hide resolved
|
||
## Summary | ||
|
||
Congratulations on completing the steps in this example! You've learned how to: |
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 like how you've enumerated the learnings here.
|
||
<Step> | ||
|
||
Under the `RadarChart` closing tag in render, add the `Treemap` component and the neccessary props. |
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.
"neccessary" -> "necessary" (sp)
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.
✅
Awesome feedback @alexronquillo, I'll see if I can get everything addressed tonight :) |
…cription to switching logic
…for further reading
…ndex file at the end of the guide
…other charting libraries
Alright, I think I've updated both PRs with all of the suggestions. And this is ready for re-review 😄 |
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.
Hey @JuliaNocera Here are a few more comments. Most are just for consistency and a couple for bugs. Once this commit is in, we'll merge and I'll do a language edit later. Thank you so much!
<RadarChart> ... <RadarChart> | ||
: | ||
<Treemap /> | ||
} |
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.
For consistency, it'd be great to have all these codeblocks show the full-file like you did for the first code block. Particularly because there's a lot of generated code in there that users have to write around.
Add a chart types constant above the first line of the class and add the component state property with an initial state for `selectedChart`. | ||
|
||
|
||
```jsx fileName=visualizations/your-visualization/index.js lineHighlight=19-22,51-53 lineNumbers=true |
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 just realized that there's a bug in the site renderer. With lineNumbers=true
, if you copy just some of the lines instead of the entire file with the Copy button, you copy the actual line numbers as well. I suggest we remove lineNumbers
until that's fixed.
|
||
<Intro> | ||
|
||
Now that you have a basic visualization, let's customize it! You can use all of the <Link to="explore-docs/intro-to-sdk">New Relic One SDK</Link> components in a Custom Visualization the same way you can use them in a Nerdlet. In this guide, you'll add a new chart type and a `SegmentedControl` component. You'll be able to dynamically swap between the Radar chart and the new chart right from the browser. |
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.
Now that you have a basic visualization, let's customize it!
To keep second person POV here, you can change this to:
Now that you have a basic visualization, it's time to customize it!
|
||
</Step> | ||
|
||
Now the final return in your render block should look like this: |
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 think I missed this the first time, but here, you lead into the code with a colon. In other places you use a period. Let's stick with a colon in all cases for consistency. Also, when you've added full-file code for the other codeblocks, you can remove this reference altogether!
src/markdown-pages/build-apps/customize-visualization-with-New-Relic-components.mdx
Show resolved
Hide resolved
@JuliaNocera FYI I'm going to take over in adding these edits so we can get this out :) |
@alexronquillo I updated your suggestions and used a component that we have for mdx to show the diff between code blocks as steps of a tutorial! There's currently no documentation for it, but I will create a separate PR with documentation. :) |
@caylahamann Thank you! This is great. I'm gonna merge and come back for language-related edits later. |
🎉 This PR is included in version 1.36.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Description
This is the first of two new Custom Visualization guides I'd like to add. Tagging @jaesius for visibility.
Reviewer Notes
If there are links or steps needed to test this work, add them here.
Related Issue(s) / Ticket(s)
If there are any related GitHub Issues or JIRA tickets, add links to them here.
Screenshot(s)
If relevant, add screenshots here.
Use Conventional Commits
Please help the maintainers by leveraging the following conventional commit
standards in your pull request title and commit messages.
Use
chore
git commit -m "chore: adjusting config and content"
Use
fix
git commit -m "fix: typo and prop error in the code of conduct"
Use
feat
git commit -m "feat(media): creating a video landing page"