Skip to content

Commit 8ead115

Browse files
NullVoxPopuliNullVoxPopuli
committed
PR feedback
1 parent 5486047 commit 8ead115

4 files changed

Lines changed: 145 additions & 73 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ rules in templates can be disabled with eslint directives with mustache or html
186186
| [template-no-aria-hidden-body](docs/rules/template-no-aria-hidden-body.md) | disallow aria-hidden on body element | | 🔧 | |
187187
| [template-no-aria-unsupported-elements](docs/rules/template-no-aria-unsupported-elements.md) | disallow ARIA roles, states, and properties on elements that do not support them | | | |
188188
| [template-no-autofocus-attribute](docs/rules/template-no-autofocus-attribute.md) | disallow autofocus attribute | | 🔧 | |
189-
| [template-no-down-event-binding](docs/rules/template-no-down-event-binding.md) | disallow mouse down event bindings | | | |
189+
| [template-no-down-event-binding](docs/rules/template-no-down-event-binding.md) | disallow pointer down event bindings | | | |
190190
| [template-no-empty-headings](docs/rules/template-no-empty-headings.md) | disallow empty heading elements | | | |
191191
| [template-no-heading-inside-button](docs/rules/template-no-heading-inside-button.md) | disallow heading elements inside button elements | | | |
192192
| [template-no-invalid-aria-attributes](docs/rules/template-no-invalid-aria-attributes.md) | disallow invalid aria-* attributes | | | |

‎docs/rules/template-no-down-event-binding.md‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@
22

33
<!-- end auto-generated rule header -->
44

5-
Disallows mouse down and touch start event bindings.
5+
Disallows pointer down event bindings (`mousedown`, `pointerdown`).
66

7-
Mouse down and touch start events can cause accessibility issues because they don't work well with keyboard navigation. Use `click` or `keydown` events instead.
7+
Pointer down events fire before the user releases the pointer, which can cause accessibility issues — actions triggered on down events don't allow users to cancel by moving the pointer away before releasing. Bind to the corresponding pointer up event instead.
88

99
## Rule Details
1010

11-
This rule disallows the use of `mousedown` and `touchstart` events in templates.
11+
This rule disallows the use of `mousedown`, `onmousedown`, `pointerdown`, and `onpointerdown` events in templates, whether via `{{on}}`, `{{action on=...}}`, or HTML attributes.
1212

1313
## Examples
1414

@@ -22,7 +22,7 @@ Examples of **incorrect** code for this rule:
2222

2323
```gjs
2424
<template>
25-
<div {{on "touchstart" this.handleTouchStart}}>Content</div>
25+
<div {{on "pointerdown" this.handlePointerDown}}>Content</div>
2626
</template>
2727
```
2828

@@ -32,23 +32,29 @@ Examples of **incorrect** code for this rule:
3232
</template>
3333
```
3434

35+
```gjs
36+
<template>
37+
<div {{action this.handler on="mousedown"}}></div>
38+
</template>
39+
```
40+
3541
Examples of **correct** code for this rule:
3642

3743
```gjs
3844
<template>
39-
<button {{on "click" this.handleClick}}>Click</button>
45+
<button {{on "mouseup" this.handleMouseUp}}>Click</button>
4046
</template>
4147
```
4248

4349
```gjs
4450
<template>
45-
<button {{on "keydown" this.handleKeyDown}}>Press</button>
51+
<div {{on "pointerup" this.handlePointerUp}}>Content</div>
4652
</template>
4753
```
4854

4955
```gjs
5056
<template>
51-
<div {{on "mouseup" this.handleMouseUp}}>Content</div>
57+
<button {{on "click" this.handleClick}}>Click</button>
5258
</template>
5359
```
5460

@@ -63,16 +69,17 @@ Replace:
6369
With:
6470

6571
```gjs
66-
<button {{on "click" this.action}}>
72+
<button {{on "mouseup" this.action}}>
6773
```
6874

69-
Or for keyboard support:
75+
Or use the more modern pointer event:
7076

7177
```gjs
72-
<button {{on "click" this.action}} {{on "keydown" this.handleKey}}>
78+
<button {{on "pointerup" this.action}}>
7379
```
7480

7581
## References
7682

77-
- [eslint-plugin-ember template-no-down-event-binding](https://github.com/ember-cli/eslint-plugin-ember/blob/master/docs/rules/template-no-down-event-binding.md)
78-
- [MDN - Mouse events](https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent)
83+
- [ember-template-lint no-pointer-down-event-binding](https://github.com/ember-template-lint/ember-template-lint/blob/master/docs/rule/no-pointer-down-event-binding.md)
84+
- [MDN - Pointer events](https://developer.mozilla.org/en-US/docs/Web/API/Pointer_events)
85+
- [MDN - mousedown event](https://developer.mozilla.org/en-US/docs/Web/API/Element/mousedown_event)

‎lib/rules/template-no-down-event-binding.js‎

Lines changed: 29 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,50 +3,58 @@ module.exports = {
33
meta: {
44
type: 'problem',
55
docs: {
6-
description: 'disallow mouse down event bindings',
6+
description: 'disallow pointer down event bindings',
77
category: 'Accessibility',
88
url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/template-no-down-event-binding.md',
99
templateMode: 'both',
1010
},
1111
fixable: null,
1212
schema: [],
1313
messages: {
14-
unexpected:
15-
'Avoid mousedown/touchstart events. Use click or keydown events instead for better accessibility.',
14+
unexpected: 'Avoid binding to a pointer `down` event; bind to a pointer `up` event instead.',
1615
},
1716
},
1817

1918
create(context) {
20-
const DOWN_EVENTS = new Set(['mousedown', 'touchstart']);
19+
const DOWN_EVENTS = new Set(['mousedown', 'onmousedown', 'pointerdown', 'onpointerdown']);
20+
21+
function isDownEvent(name) {
22+
return DOWN_EVENTS.has(name.toLowerCase());
23+
}
2124

2225
return {
2326
GlimmerElementNode(node) {
24-
// Check for onmousedown/ontouchstart attributes
27+
// Check for onmousedown/onpointerdown HTML attributes
2528
if (node.attributes) {
2629
for (const attr of node.attributes) {
27-
if (attr.name && DOWN_EVENTS.has(attr.name.replace('on', ''))) {
28-
context.report({
29-
node: attr,
30-
messageId: 'unexpected',
31-
});
30+
if (attr.name && attr.name.startsWith('on') && isDownEvent(attr.name)) {
31+
context.report({ node: attr, messageId: 'unexpected' });
3232
}
3333
}
3434
}
3535

36-
// Check for {{on "mousedown"}} or {{on "touchstart"}} modifiers
36+
// Check modifiers: {{on "mousedown"}} and {{action ... on="mousedown"}}
3737
if (node.modifiers) {
3838
for (const modifier of node.modifiers) {
39-
if (
40-
modifier.path?.type === 'GlimmerPathExpression' &&
41-
modifier.path.original === 'on' &&
42-
modifier.params?.length > 0
43-
) {
39+
if (modifier.path?.type !== 'GlimmerPathExpression') {
40+
continue;
41+
}
42+
43+
if (modifier.path.original === 'on' && modifier.params?.length > 0) {
4444
const eventParam = modifier.params[0];
45-
if (eventParam.type === 'GlimmerStringLiteral' && DOWN_EVENTS.has(eventParam.value)) {
46-
context.report({
47-
node: modifier,
48-
messageId: 'unexpected',
49-
});
45+
if (eventParam.type === 'GlimmerStringLiteral' && isDownEvent(eventParam.value)) {
46+
context.report({ node: modifier, messageId: 'unexpected' });
47+
}
48+
}
49+
50+
if (modifier.path.original === 'action') {
51+
const onPair = modifier.hash?.pairs?.find((p) => p.key === 'on');
52+
if (
53+
onPair &&
54+
onPair.value?.type === 'GlimmerStringLiteral' &&
55+
isDownEvent(onPair.value.value)
56+
) {
57+
context.report({ node: modifier, messageId: 'unexpected' });
5058
}
5159
}
5260
}

‎tests/lib/rules/template-no-down-event-binding.js‎

Lines changed: 96 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -16,56 +16,113 @@ const ruleTester = new RuleTester({
1616

1717
ruleTester.run('template-no-down-event-binding', rule, {
1818
valid: [
19-
`<template>
20-
<button {{on "click" this.handleClick}}>Click</button>
21-
</template>`,
22-
`<template>
23-
<button {{on "keydown" this.handleKeyDown}}>Press</button>
24-
</template>`,
25-
`<template>
26-
<div {{on "mouseup" this.handleMouseUp}}>Content</div>
27-
</template>`,
19+
'<template><button {{on "click" this.handleClick}}>Click</button></template>',
20+
'<template><button {{on "keydown" this.handleKeyDown}}>Press</button></template>',
21+
'<template><div {{on "mouseup" this.handleMouseUp}}>Content</div></template>',
22+
'<template><div {{on "pointerup" this.handlePointerUp}}>Content</div></template>',
23+
'<template><div {{action this.handler on="click"}}></div></template>',
24+
'<template><div {{action this.handler on="mouseup"}}></div></template>',
25+
// Case-insensitive: MOUSEUP is fine
26+
'<template><div {{on "MOUSEUP" this.handler}}>Content</div></template>',
27+
// onmouseup attribute is fine
28+
'<template><input type="text" onmouseup="myFunction()"></template>',
29+
// Component arguments are not flagged (could be any prop name)
30+
'<template><MyComponent @mouseDown={{this.doSomething}} /></template>',
2831
],
2932

3033
invalid: [
3134
{
32-
code: `<template>
33-
<button {{on "mousedown" this.handleMouseDown}}>Click</button>
34-
</template>`,
35+
code: '<template><button {{on "mousedown" this.handleMouseDown}}>Click</button></template>',
3536
output: null,
36-
errors: [
37-
{
38-
message:
39-
'Avoid mousedown/touchstart events. Use click or keydown events instead for better accessibility.',
40-
type: 'GlimmerElementModifierStatement',
41-
},
42-
],
37+
errors: [{ messageId: 'unexpected', type: 'GlimmerElementModifierStatement' }],
4338
},
4439
{
45-
code: `<template>
46-
<div {{on "touchstart" this.handleTouchStart}}>Content</div>
47-
</template>`,
40+
code: '<template><div {{on "pointerdown" this.handlePointerDown}}>Content</div></template>',
4841
output: null,
49-
errors: [
50-
{
51-
message:
52-
'Avoid mousedown/touchstart events. Use click or keydown events instead for better accessibility.',
53-
type: 'GlimmerElementModifierStatement',
54-
},
55-
],
42+
errors: [{ messageId: 'unexpected', type: 'GlimmerElementModifierStatement' }],
5643
},
44+
// Case-insensitive
5745
{
58-
code: `<template>
59-
<div onmousedown={{this.handleMouseDown}}>Content</div>
60-
</template>`,
46+
code: '<template><div {{on "MouseDown" this.handler}}>Content</div></template>',
6147
output: null,
62-
errors: [
63-
{
64-
message:
65-
'Avoid mousedown/touchstart events. Use click or keydown events instead for better accessibility.',
66-
type: 'GlimmerAttrNode',
67-
},
68-
],
48+
errors: [{ messageId: 'unexpected', type: 'GlimmerElementModifierStatement' }],
49+
},
50+
// HTML attributes
51+
{
52+
code: '<template><div onmousedown={{this.handleMouseDown}}>Content</div></template>',
53+
output: null,
54+
errors: [{ messageId: 'unexpected', type: 'GlimmerAttrNode' }],
55+
},
56+
{
57+
code: '<template><input type="text" onmousedown="myFunction()"></template>',
58+
output: null,
59+
errors: [{ messageId: 'unexpected', type: 'GlimmerAttrNode' }],
60+
},
61+
{
62+
code: '<template><div onpointerdown={{this.handlePointerDown}}>Content</div></template>',
63+
output: null,
64+
errors: [{ messageId: 'unexpected', type: 'GlimmerAttrNode' }],
65+
},
66+
// {{action}} modifier with on= hash pair
67+
{
68+
code: '<template><div {{action this.handler on="mousedown"}}></div></template>',
69+
output: null,
70+
errors: [{ messageId: 'unexpected', type: 'GlimmerElementModifierStatement' }],
71+
},
72+
{
73+
code: '<template><div {{action this.handler on="pointerdown"}}></div></template>',
74+
output: null,
75+
errors: [{ messageId: 'unexpected', type: 'GlimmerElementModifierStatement' }],
76+
},
77+
// on= is not the first hash pair
78+
{
79+
code: '<template><div {{action this.handler preventDefault=true on="mousedown"}}></div></template>',
80+
output: null,
81+
errors: [{ messageId: 'unexpected', type: 'GlimmerElementModifierStatement' }],
82+
},
83+
],
84+
});
85+
86+
const hbsRuleTester = new RuleTester({
87+
parser: require.resolve('ember-eslint-parser/hbs'),
88+
parserOptions: { ecmaVersion: 2022, sourceType: 'module' },
89+
});
90+
91+
hbsRuleTester.run('template-no-down-event-binding', rule, {
92+
valid: [
93+
'<div {{on "mouseup" this.doSomething}}></div>',
94+
'<div {{action this.doSomething on="mouseup"}}></div>',
95+
'<input type="text" onmouseup="myFunction()">',
96+
// Component arguments are not flagged
97+
'{{my-component mouseDown=this.doSomething}}',
98+
'<MyComponent @mouseDown={{this.doSomething}} />',
99+
],
100+
invalid: [
101+
{
102+
code: '<div {{on "mousedown" this.doSomething}}></div>',
103+
output: null,
104+
errors: [{ messageId: 'unexpected', type: 'GlimmerElementModifierStatement' }],
105+
},
106+
{
107+
code: '<div {{action this.doSomething on="mousedown"}}></div>',
108+
output: null,
109+
errors: [{ messageId: 'unexpected', type: 'GlimmerElementModifierStatement' }],
110+
},
111+
// on= is not the first hash pair
112+
{
113+
code: '<div {{action this.doSomething preventDefault=true on="mousedown"}}></div>',
114+
output: null,
115+
errors: [{ messageId: 'unexpected', type: 'GlimmerElementModifierStatement' }],
116+
},
117+
{
118+
code: '<input type="text" onmousedown="myFunction()">',
119+
output: null,
120+
errors: [{ messageId: 'unexpected', type: 'GlimmerAttrNode' }],
121+
},
122+
{
123+
code: '<div {{on "pointerdown" this.doSomething}}></div>',
124+
output: null,
125+
errors: [{ messageId: 'unexpected', type: 'GlimmerElementModifierStatement' }],
69126
},
70127
],
71128
});

0 commit comments

Comments
 (0)