-
Notifications
You must be signed in to change notification settings - Fork 12k
Reorganize extension docs #4178
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
docs/developers/plugins.md
Outdated
| Plugins are the most efficient way to customize or change the default behavior of a chart. They have been introduced at [version 2.1.0](https://github.com/chartjs/Chart.js/releases/tag/2.1.0) (global plugins only) and extended at [version 2.5.0](https://github.com/chartjs/Chart.js/releases/tag/2.5.0) (per chart plugins and options). | ||
|
|
||
| ## Popular Plugins | ||
| ## Popular Chart Type Plugins |
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 charts are not plugins, we could move this new section in the charts.md file before the Dataset Controller Interface section?
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 updated the PR to put it in between where we list all the built-in charts and the popular plugins. It'd make sense to link to this section from charts.md, but I'm not quite sure how to do that without digging in a bit more
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 don't think comparison.md is a good place for this information, notes/extensions.md seems a better candidate. Actually, we should also move the "Popular Plugins" into that file, then I would organize extensions.md as:
# Popular Extensions
...
## Charts
...
## Plugins
...
## Integrations
### Angular
...
### React
...Agree about links from charts.md and plugins.md
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 the suggestion. I like this solution a lot. Updated accordingly
c508c9c to
3c1537e
Compare
simonbrunel
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.
A few more minor feedback :)
docs/notes/extensions.md
Outdated
| @@ -1,26 +1,45 @@ | |||
| # Popular Extensions | |||
|
|
|||
| Many extensions can be found on the [Chart.js GitHub organization](https://github.com/chartjs) or on the [npm registry](https://www.npmjs.com/search?q=chartjs-plugin-). | |||
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.
The npm link should be changed for https://www.npmjs.com/search?q=chartjs- (plugins- removed), and we should add back this link at the end of the ##Plugins section:
In addition, many plugins can be found on the [npm registry](https://www.npmjs.com/search?q=chartjs-plugin-).
We can do the same for charts:
In addition, many charts can be found on the [npm registry](https://www.npmjs.com/search?q=chartjs-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.
Take a look and see if I understood correctly
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.
Almost :) I would not repeat ... on the [Chart.js GitHub organization](https://github.com/chartjs) or ... in the charts and plugins sections.
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.
done
docs/notes/extensions.md
Outdated
|
|
||
| ## Charts | ||
|
|
||
| - <a href="https://github.com/chartjs/chartjs-chart-financial" target="_blank">Financial</a> - Adds financial chart types such as a candlestick. |
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 with other entries, the name should be chartjs-chart-financial instead of Financial
docs/notes/extensions.md
Outdated
|
|
||
| ## Integrations | ||
|
|
||
| There are many extensions which are available for use with popular frameworks. Some particularly notable ones are listed here. |
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 would remove that line
simonbrunel
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!
No description provided.