Skip to content

Commit

Permalink
Rule fix: Add stop and finish methods to no-animate
Browse files Browse the repository at this point in the history
These are core animation methods that aren't covered in
any other animation rules (e.g. `no-fade`).
  • Loading branch information
edg2s committed Nov 19, 2024
1 parent 4039675 commit dcfb237
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 6 deletions.
14 changes: 13 additions & 1 deletion docs/rules/no-animate.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

# no-animate

Disallows the [`.animate`](https://api.jquery.com/animate/) method. Use the `allowScroll` option to allow animations which are just used for scrolling. Prefer CSS transitions.
Disallows the [`.animate`](https://api.jquery.com/animate/)/[`.stop`](https://api.jquery.com/stop/)/[`.finish`](https://api.jquery.com/finish/) methods. Use the `allowScroll` option to allow animations which are just used for scrolling. Prefer CSS transitions.

📋 This rule is enabled in `plugin:no-jquery/slim`.

Expand All @@ -13,6 +13,8 @@ Disallows the [`.animate`](https://api.jquery.com/animate/) method. Use the `all
❌ Examples of **incorrect** code:
```js
$( 'div' ).animate();
$( 'div' ).stop();
$( 'div' ).finish();
$div.animate();
$( 'div' ).first().animate();
$( 'div' ).append( $( 'input' ).animate() );
Expand All @@ -28,6 +30,14 @@ animate();
[].animate();
div.animate();
div.animate;
stop();
[].stop();
div.stop();
div.stop;
finish();
[].finish();
div.finish();
div.finish;
```

❌ Examples of **incorrect** code with `[{"allowScroll":false}]` options:
Expand All @@ -39,6 +49,8 @@ $div.animate( { scrollTop: 100 } );
```js
$div.animate();
$div.animate( { scrollTop: 100, width: 300 } );
$( 'div' ).stop( { scrollTop: 100, scrollLeft: 200 } );
$( 'div' ).finish( { scrollTop: 100, scrollLeft: 200 } );
```

✔️ Examples of **correct** code with `[{"allowScroll":true}]` options:
Expand Down
10 changes: 6 additions & 4 deletions src/rules/no-animate.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@

const utils = require( '../utils.js' );

const methods = [ 'animate', 'stop', 'finish' ];

module.exports = {
meta: {
type: 'suggestion',
docs: {
description:
'Disallows the ' + utils.jQueryCollectionLink( 'animate' ) +
' method. Use the `allowScroll` option to allow animations which are just used for scrolling. Prefer CSS transitions.'
'Disallows the ' + methods.map( utils.jQueryCollectionLink ).join( '/' ) +
' methods. Use the `allowScroll` option to allow animations which are just used for scrolling. Prefer CSS transitions.'
},
schema: [
{
Expand All @@ -27,12 +29,12 @@ module.exports = {
'CallExpression:exit': ( node ) => {
if (
node.callee.type !== 'MemberExpression' ||
node.callee.property.name !== 'animate'
!methods.includes( node.callee.property.name )
) {
return;
}
const allowScroll = context.options[ 0 ] && context.options[ 0 ].allowScroll;
if ( allowScroll ) {
if ( node.callee.property.name === 'animate' && allowScroll ) {
const arg = node.arguments[ 0 ];
// Check properties list has more than just scrollTop/scrollLeft
if ( arg && arg.type === 'ObjectExpression' ) {
Expand Down
28 changes: 27 additions & 1 deletion tests/rules/no-animate.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@ ruleTester.run( 'no-animate', rule, {
'[].animate()',
'div.animate()',
'div.animate',
'stop()',
'[].stop()',
'div.stop()',
'div.stop',
'finish()',
'[].finish()',
'div.finish()',
'div.finish',
{
code: '$div.animate({scrollTop: 100})',
options: [ { allowScroll: true } ]
Expand All @@ -31,6 +39,14 @@ ruleTester.run( 'no-animate', rule, {
code: '$("div").animate()',
errors: [ error ]
},
{
code: '$("div").stop()',
errors: [ error ]
},
{
code: '$("div").finish()',
errors: [ error ]
},
{
code: '$div.animate()',
errors: [ error ]
Expand Down Expand Up @@ -73,6 +89,16 @@ ruleTester.run( 'no-animate', rule, {
code: '$div.animate({scrollTop: 100, width: 300})',
options: [ { allowScroll: true } ],
errors: [ errorNoScroll ]
}
},
{
code: '$("div").stop({scrollTop: 100, scrollLeft: 200})',
options: [ { allowScroll: true } ],
errors: [ error ]
},
{
code: '$("div").finish({scrollTop: 100, scrollLeft: 200})',
options: [ { allowScroll: true } ],
errors: [ error ]
},

Check failure on line 102 in tests/rules/no-animate.js

View workflow job for this annotation

GitHub Actions / build (18.x)

Unexpected trailing comma

Check failure on line 102 in tests/rules/no-animate.js

View workflow job for this annotation

GitHub Actions / build (20.x)

Unexpected trailing comma

Check failure on line 102 in tests/rules/no-animate.js

View workflow job for this annotation

GitHub Actions / build (22.x)

Unexpected trailing comma

Check failure on line 102 in tests/rules/no-animate.js

View workflow job for this annotation

GitHub Actions / build (18.x)

Unexpected trailing comma

Check failure on line 102 in tests/rules/no-animate.js

View workflow job for this annotation

GitHub Actions / build (20.x)

Unexpected trailing comma

Check failure on line 102 in tests/rules/no-animate.js

View workflow job for this annotation

GitHub Actions / build (22.x)

Unexpected trailing comma
]
} );

0 comments on commit dcfb237

Please sign in to comment.