Skip to content

Commit c9c6f11

Browse files
authored
Rule fix: In no-animate allow stop and finish when allowScroll (#345)
We can't tell what type of animation is being stopped, so assume it is an allowed scroll.
1 parent 78573f8 commit c9c6f11

File tree

3 files changed

+27
-22
lines changed

3 files changed

+27
-22
lines changed

docs/rules/no-animate.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,15 @@ $div.animate( { scrollTop: 100 } );
4949
```js
5050
$div.animate();
5151
$div.animate( { scrollTop: 100, width: 300 } );
52-
$( 'div' ).stop( { scrollTop: 100, scrollLeft: 200 } );
53-
$( 'div' ).finish( { scrollTop: 100, scrollLeft: 200 } );
5452
```
5553

5654
✔️ Examples of **correct** code with `[{"allowScroll":true}]` options:
5755
```js
5856
$div.animate( { scrollTop: 100 } );
5957
$div.animate( { scrollLeft: 200 } );
6058
$div.animate( { scrollTop: 100, scrollLeft: 200 } );
59+
$div.animate( { scrollTop: 100 } ).stop();
60+
$div.animate( { scrollTop: 100 } ).finish();
6161
```
6262

6363
## Resources

src/rules/no-animate.js

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,16 +34,23 @@ module.exports = {
3434
return;
3535
}
3636
const allowScroll = context.options[ 0 ] && context.options[ 0 ].allowScroll;
37-
if ( node.callee.property.name === 'animate' && allowScroll ) {
38-
const arg = node.arguments[ 0 ];
39-
// Check properties list has more than just scrollTop/scrollLeft
40-
if ( arg && arg.type === 'ObjectExpression' ) {
41-
if (
42-
arg.properties.every(
43-
( prop ) => prop.key.name === 'scrollTop' || prop.key.name === 'scrollLeft'
44-
)
45-
) {
46-
return;
37+
const name = node.callee.property.name;
38+
if ( allowScroll ) {
39+
if ( name === 'stop' || name === 'finish' ) {
40+
// We can't tell what animation we are stopping, so assume
41+
// it is an allowed scroll
42+
return;
43+
} else if ( name === 'animate' ) {
44+
const arg = node.arguments[ 0 ];
45+
// Check properties list has more than just scrollTop/scrollLeft
46+
if ( arg && arg.type === 'ObjectExpression' ) {
47+
if (
48+
arg.properties.every(
49+
( prop ) => prop.key.name === 'scrollTop' || prop.key.name === 'scrollLeft'
50+
)
51+
) {
52+
return;
53+
}
4754
}
4855
}
4956
}

tests/rules/no-animate.js

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,14 @@ ruleTester.run( 'no-animate', rule, {
3232
{
3333
code: '$div.animate({scrollTop: 100, scrollLeft: 200})',
3434
options: [ { allowScroll: true } ]
35+
},
36+
{
37+
code: '$div.animate({scrollTop: 100}).stop()',
38+
options: [ { allowScroll: true } ]
39+
},
40+
{
41+
code: '$div.animate({scrollTop: 100}).finish()',
42+
options: [ { allowScroll: true } ]
3543
}
3644
],
3745
invalid: [
@@ -89,16 +97,6 @@ ruleTester.run( 'no-animate', rule, {
8997
code: '$div.animate({scrollTop: 100, width: 300})',
9098
options: [ { allowScroll: true } ],
9199
errors: [ errorNoScroll ]
92-
},
93-
{
94-
code: '$("div").stop({scrollTop: 100, scrollLeft: 200})',
95-
options: [ { allowScroll: true } ],
96-
errors: [ errorNoScroll ]
97-
},
98-
{
99-
code: '$("div").finish({scrollTop: 100, scrollLeft: 200})',
100-
options: [ { allowScroll: true } ],
101-
errors: [ errorNoScroll ]
102100
}
103101
]
104102
} );

0 commit comments

Comments
 (0)