Skip to content

feat: add |nonself event modifier for non-interactive elements with event listeners #9029

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/tough-kids-develop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': minor
---

feat: add `|nonself` event modifier for non-interactive elements with event listeners
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ The following modifiers are available:
- `capture` — fires the handler during the _capture_ phase instead of the _bubbling_ phase
- `once` — remove the handler after the first time it runs
- `self` — only trigger handler if `event.target` is the element itself
- `nonself` — only trigger handler when `event.target` is not the element itself, e.g. if the event bubbled from a child element
- `trusted` — only trigger handler if `event.isTrusted` is `true`. I.e. if the event is triggered by a user action.

Modifiers can be chained together, e.g. `on:click|once|capture={...}`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ The full list of modifiers:
- `capture` — fires the handler during the _capture_ phase instead of the _bubbling_ phase ([MDN docs](https://developer.mozilla.org/en-US/docs/Learn/JavaScript/Building_blocks/Events#Event_bubbling_and_capture))
- `once` — remove the handler after the first time it runs
- `self` — only trigger handler if event.target is the element itself
- `nonself` — only trigger handler when `event.target` is not the element itself, e.g. if the event bubbled from a child element
- `trusted` — only trigger handler if `event.isTrusted` is `true`. I.e. if the event is triggered by a user action.

You can chain modifiers together, e.g. `on:click|once|capture={...}`.
2 changes: 1 addition & 1 deletion packages/svelte/src/compiler/compile/internal_exports.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

33 changes: 25 additions & 8 deletions packages/svelte/src/compiler/compile/nodes/Element.js
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ const valid_modifiers = new Set([
'passive',
'nonpassive',
'self',
'nonself',
'trusted'
]);
const passive_events = new Set(['wheel', 'touchstart', 'touchmove', 'touchend', 'touchcancel']);
Expand Down Expand Up @@ -598,7 +599,9 @@ export default class Element extends Node {
}
validate_attributes_a11y() {
const { component, attributes, handlers } = this;
/** @type {Map<string, Attribute>} */
const attribute_map = new Map();
/** @type {Map<string, EventHandler>} */
const handlers_map = new Map();
attributes.forEach((attribute) => attribute_map.set(attribute.name, attribute));
handlers.forEach((handler) => handlers_map.set(handler.name, handler));
Expand Down Expand Up @@ -780,7 +783,7 @@ export default class Element extends Node {
}
});
// click-events-have-key-events
if (handlers_map.has('click')) {
if (handlers_map.get('click')?.modifiers.has('nonself') === false) {
const role = attribute_map.get('role');
const is_non_presentation_role =
role?.is_static &&
Expand Down Expand Up @@ -851,8 +854,10 @@ export default class Element extends Node {
is_non_interactive_roles(role_static_value)) ||
(is_non_interactive_element(this.name, attribute_map) && !role))
) {
const has_interactive_handlers = handlers.some((handler) =>
a11y_recommended_interactive_handlers.has(handler.name)
const has_interactive_handlers = handlers.some(
(handler) =>
a11y_recommended_interactive_handlers.has(handler.name) &&
!handler.modifiers.has('nonself')
);
if (has_interactive_handlers) {
component.warn(
Expand All @@ -873,13 +878,19 @@ export default class Element extends Node {
!is_non_interactive_roles(role_static_value) &&
!is_abstract_role(role_static_value)
) {
const interactive_handlers = handlers
.map((handler) => handler.name)
.filter((handlerName) => a11y_interactive_handlers.has(handlerName));
if (interactive_handlers.length > 0) {
const interactive_handler_names = handlers
.filter(
(handler) =>
a11y_interactive_handlers.has(handler.name) && !handler.modifiers.has('nonself')
)
.map((handler) => handler.name);
if (interactive_handler_names.length > 0) {
component.warn(
this,
compiler_warnings.a11y_no_static_element_interactions(this.name, interactive_handlers)
compiler_warnings.a11y_no_static_element_interactions(
this.name,
interactive_handler_names
)
);
}
}
Expand Down Expand Up @@ -1265,6 +1276,12 @@ export default class Element extends Node {
compiler_errors.invalid_event_modifier_combination('passive', 'nonpassive')
);
}
if (handler.modifiers.has('self') && handler.modifiers.has('nonself')) {
return component.error(
handler,
compiler_errors.invalid_event_modifier_combination('self', 'nonself')
);
}
handler.modifiers.forEach((modifier) => {
if (!valid_modifiers.has(modifier)) {
return component.error(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export default class EventHandlerWrapper {
if (this.node.modifiers.has('stopImmediatePropagation'))
snippet = x`@stop_immediate_propagation(${snippet})`;
if (this.node.modifiers.has('self')) snippet = x`@self(${snippet})`;
if (this.node.modifiers.has('nonself')) snippet = x`@nonself(${snippet})`;
if (this.node.modifiers.has('trusted')) snippet = x`@trusted(${snippet})`;
const args = [];
const opts = ['nonpassive', 'passive', 'once', 'capture'].filter((mod) =>
Expand Down
48 changes: 38 additions & 10 deletions packages/svelte/src/runtime/internal/dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -361,49 +361,76 @@ export function listen(node, event, handler, options) {
}

/**
* @returns {(event: any) => any} */
* @template T
* @param {(event: Event) => T} fn
* @returns {(event: Event) => T}
*/
export function prevent_default(fn) {
/** @this HTMLElement */
return function (event) {
event.preventDefault();
// @ts-ignore
return fn.call(this, event);
};
}

/**
* @returns {(event: any) => any} */
* @template T
* @param {(event: Event) => T} fn
* @returns {(event: Event) => T}
*/
export function stop_propagation(fn) {
/** @this HTMLElement */
return function (event) {
event.stopPropagation();
// @ts-ignore
return fn.call(this, event);
};
}

/**
* @returns {(event: any) => any} */
* @template T
* @param {(event: Event) => T} fn
* @returns {(event: Event) => T}
*/
export function stop_immediate_propagation(fn) {
/** @this HTMLElement */
return function (event) {
event.stopImmediatePropagation();
// @ts-ignore
return fn.call(this, event);
};
}

/**
* @returns {(event: any) => void} */
* @template T
* @param {(event: Event) => T} fn
* @returns {(event: Event) => void}
*/
export function self(fn) {
/** @this HTMLElement */
return function (event) {
// @ts-ignore
if (event.target === this) fn.call(this, event);
};
}

/**
* @returns {(event: any) => void} */
* @template T
* @param {(event: Event) => T} fn
* @returns {(event: Event) => void}
*/
export function nonself(fn) {
/** @this HTMLElement */
return function (event) {
if (event.target !== event.currentTarget) fn.call(this, event);
};
}

/**
* @template T
* @param {(event: Event) => T} fn
* @returns {(event: Event) => void}
*/
export function trusted(fn) {
/** @this HTMLElement */
return function (event) {
// @ts-ignore
if (event.isTrusted) fn.call(this, event);
};
}
Expand All @@ -418,6 +445,7 @@ export function attr(node, attribute, value) {
if (value == null) node.removeAttribute(attribute);
else if (node.getAttribute(attribute) !== value) node.setAttribute(attribute, value);
}

/**
* List of attributes that should always be set through the attr method,
* because updating them through the property setter doesn't work reliably.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
export default {
html: `
<div>
<button>click me</button>
</div>
`,

async test({ assert, component, target, window }) {
const button = target.querySelector('button');
const event = new window.MouseEvent('click', { bubbles: true });

await button.parentNode.dispatchEvent(event);
assert.ok(!component.clicked);

await button.dispatchEvent(event);
assert.ok(component.clicked);
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<script>
export let clicked;
let f;

function handle_click(event) {
clicked = true;
}
f = handle_click;
</script>

<div on:click|nonself={f}>
<button>click me</button>
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
export default {
html: `
<div>
<button>click me</button>
</div>
`,

async test({ assert, component, target, window }) {
const button = target.querySelector('button');
const event = new window.MouseEvent('click', { bubbles: true });

await button.parentNode.dispatchEvent(event);
assert.ok(!component.clicked);

await button.dispatchEvent(event);
assert.ok(component.clicked);
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<script>
export let clicked;

function handle_click(event) {
clicked = true;
}
</script>

<div on:click|nonself={handle_click}>
<button>click me</button>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ export default {

async test({ assert, component, target, window }) {
const button = target.querySelector('button');
const event = new window.MouseEvent('click');
const event = new window.MouseEvent('click', { bubbles: true });

await button.dispatchEvent(event);

assert.ok(!component.inner_clicked);

await button.parentNode.dispatchEvent(event);
assert.ok(component.inner_clicked);
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
<button on:click={() => {}} />
<h1 contenteditable="true" on:keydown={() => {}}>Heading</h1>
<h1>Heading</h1>
<div role="paragraph" on:mouseup|nonself={() => {}}><slot name="interactive-element-goes-here"></slot></div>

<!-- INVALID -->
<div role="listitem" on:mousedown={() => {}} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,60 +3,60 @@
"code": "a11y-no-noninteractive-element-interactions",
"end": {
"column": 47,
"line": 10
"line": 11
},
"message": "A11y: Non-interactive element <div> should not be assigned mouse or keyboard event listeners.",
"start": {
"column": 0,
"line": 10
"line": 11
}
},
{
"code": "a11y-no-noninteractive-element-interactions",
"end": {
"column": 58,
"line": 11
"line": 12
},
"message": "A11y: Non-interactive element <h1> should not be assigned mouse or keyboard event listeners.",
"start": {
"column": 0,
"line": 11
"line": 12
}
},
{
"code": "a11y-no-noninteractive-element-interactions",
"end": {
"column": 50,
"line": 12
"line": 13
},
"message": "A11y: Non-interactive element <h1> should not be assigned mouse or keyboard event listeners.",
"start": {
"column": 0,
"line": 12
"line": 13
}
},
{
"code": "a11y-no-noninteractive-element-interactions",
"end": {
"column": 28,
"line": 13
"line": 14
},
"message": "A11y: Non-interactive element <p> should not be assigned mouse or keyboard event listeners.",
"start": {
"column": 0,
"line": 13
"line": 14
}
},
{
"code": "a11y-no-noninteractive-element-interactions",
"end": {
"column": 46,
"line": 14
"line": 15
},
"message": "A11y: Non-interactive element <div> should not be assigned mouse or keyboard event listeners.",
"start": {
"column": 0,
"line": 14
"line": 15
}
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,4 @@
<div role='article' tabindex='0' />
<article tabindex='0' />
<article tabindex='{0}' />
<div tabindex='0' on:click|nonself={() => {}}><slot name="interactive-element-goes-here"></slot></div>
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,17 @@
"column": 0,
"line": 16
}
},
{
"code": "a11y-no-noninteractive-tabindex",
"end": {
"column": 102,
"line": 17
},
"message": "A11y: noninteractive element cannot have nonnegative tabIndex value",
"start": {
"column": 0,
"line": 17
}
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
<div role={dynamicRole} on:click={() => {}} />
<!-- svelte-ignore a11y-no-noninteractive-element-interactions -->
<footer on:keydown={() => {}} />
<div on:click|nonself={() => {}}><slot name="interactive-element-goes-here"></slot></div>

<!-- invalid -->
<div on:keydown={() => {}} />
Expand Down
Loading