-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat(dev-toolbar): Dev Toolbar Docs #12117
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
Bundle ReportChanges will increase total bundle size by 1.8kB (0.01%) ⬆️. This is within the configured threshold ✅ Detailed changes
|
drguthals
left a comment
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.
Left comments throughout
| } | ||
| ``` | ||
|
|
||
| [MockFeatureFlagIntegration.tsx](https://github.com/getsentry/sentry-toolbar/blob/301c31d561a81e3fd8ffc9532aad3a60af685916/src/env/demo/MockFeatureFlagIntegration.tsx) is an example adapter to use as a reference. |
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.
This feels like it needs to be above the interface but I can't think of how I would word it.
| ```html | ||
| <script src="https://browser.sentry-cdn.com/sentry-toolbar/latest/toolbar.min.js"></script> | ||
| ``` | ||
| It is recommended that you add the script tag at the bottom of the page so it doesn’t block other critical JavaScript. |
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.
Since placement is important, can we be a little more specific here? Should it be at the body of the <Body> element?
| </html> | ||
| ``` | ||
|
|
||
| ### Unmounting The Toolbar |
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'm a little confused by this, why would you want to mount the toolbar and then later unmount it on the same page? Am I misunderstanding this? Is there a brief use case we can provide to illustrate when you would want to do this?
docs/product/dev-toolbar/setup.mdx
Outdated
| </script> | ||
| ``` | ||
|
|
||
| ## Init Configuration Fields |
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 other parts of the docs I've more commonly seen these referred to as 'options', should we stick to that convention here? I'm also open to 'fields' being the correct term, if that's the case we should update other docs to maintain consistency.
@lizokm Do you have any thoughts on 'fields' vs 'options'?
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've changed it to 'options' to stick with the convention
| | ----- | ----- | ----- | ----- | | ||
| | `organizationSlug` | `string` | The organization that users should login to. For example \'acme\' | *Required Value* | | ||
| | `projectIdOrSlug` | `string \| number` | The project for which this website/webapp is associated. | *Required Value* | | ||
| | `environment (optional)` | `string \| string[] \| undefined` | The environment of this deployment. Used to narrow search results in the Toolbar UI. Set to `undefined` or `""` or `[]` if you want to see results from all environments. | `undefined` | |
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.
| | `environment (optional)` | `string \| string[] \| undefined` | The environment of this deployment. Used to narrow search results in the Toolbar UI. Set to `undefined` or `""` or `[]` if you want to see results from all environments. | `undefined` | | |
| | `environment (optional)` | `string \| string[] \| undefined` | The environment of this deployment. Used to narrow search results in the Toolbar UI. Set to `undefined` or `""` or `[]` if you want to see results from all environments. | `undefined` | |
Do we need to share all the different ways to set this to see results for all environments here('undefined', '""', or '[]')? It would simplify things if we just picked one.
coolguyzone
left a comment
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.
Thanks for adding this! Added some suggestions, looks good to me 🥳
Co-authored-by: Alex Krawiec <[email protected]>
Product docs for the dev toolbar
Fixes #12097