Edit and correct readme for better readability #4
Conversation
This file contains 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
mciastek
requested changes
Sep 11, 2018
README.md
Outdated
|
|
||
| **Sal** (_Scroll Animation Library_) is written with aim to provide performant and lightweight solution for animating elements on scroll. It's based on the [Intersection Observer](https://developer.mozilla.org/en-US/docs/Web/API/Intersection_Observer_API), which gives amazing performance in terms of checking the element's presence in viewport. | ||
| **Sal** (_Scroll Animation Library_) was created to provide a performant and lightweight solution for animating elements on scroll. It's based on the [Intersection Observer](https://developer.mozilla.org/en-US/docs/Web/API/Intersection_Observer_API), which gives amazing performance in terms of checking the element's presence in the viewport. |
README.md
Outdated
| @@ -1,8 +1,10 @@ | |||
| # Sal [](https://www.npmjs.com/package/sal.js) [](https://github.com/mciastek/sal/blob/master/LICENSE) [](https://travis-ci.org/mciastek/sal) | |||
|
|
|||
| Performance focused, lightweight (less than **2.5 kb**) scroll animation library, written in vanilla JavaScript. No dependencies! | |||
| Performance focused, lightweight (less than **2.5 kb** without Gzip) scroll animation library, written in vanilla JavaScript. No dependencies! | |||
There was a problem hiding this comment.
It's with Gzip. I think it should be 2.8kb. See screenshot below (gziped compression is on):
There was a problem hiding this comment.
Oh, right, I looked only at the JS file and forgot about counting CSS as well!
README.md
Outdated
| @@ -22,7 +24,7 @@ $ npm install --save sal.js | |||
| $ yarn add sal.js | |||
| ``` | |||
|
|
|||
| Then load it with your favorite module loader or use as global, when no modules | |||
| Load it with your favorite module loader or use as a global | |||
There was a problem hiding this comment.
At first I actually thought of explicitely documenting the "as a global" use-case. It could look like this (along with importing styles):
Load it with your favorite module loader:
// ES6 modules
import sal from 'sal.js'
// CommonJS modules
var sal = require('sal.js')or use as a global variable:
<script src="sal.js"></script>Import the styles in your preprocessor:
// Webpack
@import '~sal.js/sal.css';
// Other
@import './node_modules/sal.js/dist/sal.css';or in your document's head:
<link rel="stylesheet" type="text/css" href="sal.css" />What do you think?
There was a problem hiding this comment.
I would like to emphasise on using sal.js with Webpack, rather than as a global script. I'm planning to make a Wiki docs for that kind of tips and tricks. For now you can skip it.
| **Sal** (_Scroll Animation Library_) is written with aim to provide performant and lightweight solution for animating elements on scroll. It's based on the [Intersection Observer](https://developer.mozilla.org/en-US/docs/Web/API/Intersection_Observer_API), which gives amazing performance in terms of checking the element's presence in viewport. | ||
| **Sal** (_Scroll Animation Library_) was created to provide a performant and lightweight solution for animating elements on scroll. It's based on the [Intersection Observer](https://developer.mozilla.org/en-US/docs/Web/API/Intersection_Observer_API), which gives amazing performance in terms of checking the element's presence in the viewport. | ||
|
|
||
| **Note:** Intersection Observer API is an experimental technology so be sure to consult the [browser compatibility table](https://developer.mozilla.org/en-US/docs/Web/API/Intersection_Observer_API#Browser_compatibility) and consider using a [polyfill](https://github.com/w3c/IntersectionObserver/tree/master/polyfill). |
There was a problem hiding this comment.
Thanks for this note 👍 It's really important!
|
Please add the required changes and PR is ready to merge 🎉 |
Closed
|
Please see 7267211 for the fixes. Cheers. |
mciastek
approved these changes
Sep 13, 2018
|
Looks good to me. Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Hey, I've found your library through the Codrops Collective, it looks neat.
Thought I'd contribute by correcting the readme a little. Added a browser compatibilty note with a link to the polyfill also.
Tell me what you think.
Keep up the good work.