Improve README.md#5274
Merged
Merged
Conversation
etimberg
requested changes
Feb 16, 2018
| #### Selecting the Correct Build | ||
|
|
||
| Chart.js provides two different builds that are available for your use. The `Chart.js` and `Chart.min.js` files include Chart.js and the accompanying color parsing library. If this version is used and you require the use of the time axis, [Moment.js](http://momentjs.com/) will need to be included before Chart.js. | ||
| Chart.js provides two builds: `Chart.js`, `Chart.min.js`. Both `Chart.js` and `Chart.min.js` files include Chart.js as well as the color parsing library. If this version is used, you require to include [Moment.js](http://momentjs.com/) before Chart.js for the functionality of the time axis. |
Member
There was a problem hiding this comment.
I would change the second sentence to "If this version is used, you are required to include Moment.js before Chart.js to enable the functionality of the time axis."
Contributor
Author
| Chart.js provides two builds: `Chart.js`, `Chart.min.js`. Both `Chart.js` and `Chart.min.js` files include Chart.js as well as the color parsing library. If this version is used, you require to include [Moment.js](http://momentjs.com/) before Chart.js for the functionality of the time axis. | ||
|
|
||
| The `Chart.bundle.js` and `Chart.bundle.min.js` builds include Moment.js in a single file. This version should be used if you require time axes and want a single file to include, select this version. Do not use this build if your application already includes Moment.js. If you do, Moment.js will be included twice, increasing the page load time and potentially introducing version issues. | ||
| Both `Chart.bundle.js` and `Chart.bundle.min.js` builds include Moment.js in a single file. You should use this version if you require time axes and want to include a single file. You should not use this build if your application already included Moment.js. Otherwise, Moment.js will be included twice which results increasing page load time and possible version compatability issues. |
Member
There was a problem hiding this comment.
"which results increasing page load" -> "which results in increasing page load"
Member
|
Thanks @wla80, would you like to also update the documentation with the same changes? |
Contributor
Author
|
Hello @simonbrunel , you are welcome. I've already included the updates in installation.md. Thanks |
etimberg
previously approved these changes
Feb 17, 2018
simonbrunel
requested changes
Feb 17, 2018
| Chart.js provides two different builds that are available for your use. | ||
| Chart.js provides two builds: `Chart.js`, `Chart.min.js`. Both `Chart.js` and `Chart.min.js` files include Chart.js as well as the color parsing library. If this version is used, you are required to include [Moment.js](http://momentjs.com/) before Chart.js for the functionality of the time axis. | ||
|
|
||
| Both `Chart.bundle.js` and `Chart.bundle.min.js` builds include Moment.js in a single file. You should use this version if you require time axes and want to include a single file. You should not use this build if your application already included Moment.js. Otherwise, Moment.js will be included twice which results in increasing page load time and possible version compatability issues. |
Member
There was a problem hiding this comment.
I think it would be better to split this text in both following sub-sections, replacing the existing text.
Contributor
Author
There was a problem hiding this comment.
For some reason, I wasn't aware of the sub-sections. The text is split into sub-sections in both .md. Thanks for pointing out:)
Address the ambiguity of "Selecting the Correct Build" section
etimberg
approved these changes
Feb 18, 2018
simonbrunel
approved these changes
Feb 20, 2018
Member
|
Thanks @wla80 |
exwm
pushed a commit
to exwm/Chart.js
that referenced
this pull request
Apr 30, 2021
Address the ambiguity of "Selecting the Correct Build" section
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Improve the writing quality of "Selecting the Correct Build" section