next: onpointerdown -> onclick (#1011)

.changeset/yellow-lies-vanish.md

@@ -0,0 +1,5 @@
+---
+"bits-ui": patch
+---
+
+revert to `onclick` events for most components except where it makes sense (like menus, select, etc.)

packages/bits-ui/src/lib/bits/accordion/accordion.svelte.ts

@@ -1,6 +1,7 @@
 import { afterTick, useRefById } from "svelte-toolbelt";
+import { watch } from "runed";
 import type { Box, ReadableBoxedValues, WritableBoxedValues } from "$lib/internal/box.svelte.js";
-import type { BitsKeyboardEvent, BitsPointerEvent, WithRefProps } from "$lib/internal/types.js";
+import type { BitsKeyboardEvent, BitsMouseEvent, WithRefProps } from "$lib/internal/types.js";
 import {
 	getAriaDisabled,
 	getAriaExpanded,
@@ -213,30 +214,21 @@ class AccordionTriggerState {
 		this.#root = itemState.root;
 		this.#id = props.id;
 		this.#ref = props.ref;
-		this.onpointerdown = this.onpointerdown.bind(this);
-		this.onpointerup = this.onpointerup.bind(this);
 		this.onkeydown = this.onkeydown.bind(this);
+		this.onclick = this.onclick.bind(this);
 
 		useRefById({
 			id: props.id,
 			ref: this.#ref,
 		});
 	}
 
-	onpointerdown(e: BitsPointerEvent) {
+	onclick(e: BitsMouseEvent) {
 		if (this.#isDisabled) return;
-		if (e.pointerType === "touch" || e.button !== 0) return e.preventDefault();
+		if (e.button !== 0) return e.preventDefault();
 		this.#itemState.updateValue();
 	}
 
-	onpointerup(e: BitsPointerEvent) {
-		if (this.#isDisabled) return;
-		if (e.pointerType === "touch") {
-			e.preventDefault();
-			this.#itemState.updateValue();
-		}
-	}
-
 	onkeydown(e: BitsKeyboardEvent) {
 		if (this.#isDisabled) return;
 		if (e.key === kbd.SPACE || e.key === kbd.ENTER) {
@@ -261,8 +253,7 @@ class AccordionTriggerState {
 				[ACCORDION_TRIGGER_ATTR]: "",
 				tabindex: 0,
 				//
-				onpointerdown: this.onpointerdown,
-				onpointerup: this.onpointerup,
+				onclick: this.onclick,
 				onkeydown: this.onkeydown,
 			}) as const
 	);
@@ -311,11 +302,8 @@ class AccordionContentState {
 			};
 		});
 
-		$effect(() => {
-			this.present;
-			const node = this.#ref.current;
+		watch([() => this.present, () => this.#ref.current], ([_, node]) => {
 			if (!node) return;
-
 			afterTick(() => {
 				if (!this.#ref.current) return;
 				// get the dimensions of the element

packages/bits-ui/src/lib/bits/collapsible/collapsible.svelte.ts

@@ -3,7 +3,7 @@ import type { ReadableBoxedValues, WritableBoxedValues } from "$lib/internal/box
 import { getAriaExpanded, getDataDisabled, getDataOpenClosed } from "$lib/internal/attrs.js";
 import { createContext } from "$lib/internal/create-context.js";
 import { kbd } from "$lib/internal/kbd.js";
-import type { BitsKeyboardEvent, BitsPointerEvent } from "$lib/internal/types.js";
+import type { BitsKeyboardEvent, BitsMouseEvent, BitsPointerEvent } from "$lib/internal/types.js";
 
 const COLLAPSIBLE_ROOT_ATTR = "data-collapsible-root";
 const COLLAPSIBLE_CONTENT_ATTR = "data-collapsible-content";
@@ -176,8 +176,7 @@ class CollapsibleTriggerState {
 		this.#ref = props.ref;
 		this.#disabled = props.disabled;
 
-		this.onpointerdown = this.onpointerdown.bind(this);
-		this.onpointerup = this.onpointerup.bind(this);
+		this.onclick = this.onclick.bind(this);
 		this.onkeydown = this.onkeydown.bind(this);
 
 		useRefById({
@@ -186,23 +185,17 @@ class CollapsibleTriggerState {
 		});
 	}
 
-	onpointerdown(e: BitsPointerEvent) {
+	onclick(e: BitsMouseEvent) {
 		if (this.#isDisabled) return;
-		if (e.pointerType === "touch" || e.button !== 0) return e.preventDefault();
+		if (e.button !== 0) return e.preventDefault();
 		this.#root.toggleOpen();
 	}
 
-	onpointerup(e: BitsPointerEvent) {
-		if (this.#isDisabled) return;
-		if (e.pointerType === "touch") {
-			e.preventDefault();
-			this.#root.toggleOpen();
-		}
-	}
-
 	onkeydown(e: BitsKeyboardEvent) {
 		if (this.#isDisabled) return;
+
 		if (e.key === kbd.SPACE || e.key === kbd.ENTER) {
+			e.preventDefault();
 			this.#root.toggleOpen();
 		}
 	}
@@ -219,8 +212,7 @@ class CollapsibleTriggerState {
 				"data-disabled": getDataDisabled(this.#isDisabled),
 				[COLLAPSIBLE_TRIGGER_ATTR]: "",
 				//
-				onpointerdown: this.onpointerdown,
-				onpointerup: this.onpointerup,
+				onclick: this.onclick,
 				onkeydown: this.onkeydown,
 			}) as const
 	);

packages/bits-ui/src/lib/bits/dialog/dialog.svelte.ts

@@ -108,13 +108,10 @@ class DialogTriggerState {
 
 	onpointerdown = (e: BitsPointerEvent) => {
 		if (this.#disabled.current) return;
-		if (e.pointerType === "touch") return e.preventDefault();
 		if (e.button > 0) return;
 		// by default, it will attempt to focus this trigger on pointerdown
 		// since this also opens the dialog we want to prevent that behavior
 		e.preventDefault();
-
-		this.#root.handleOpen();
 	};
 
 	onkeydown = (e: BitsKeyboardEvent) => {
@@ -180,10 +177,9 @@ class DialogCloseState {
 
 	onpointerdown(e: BitsPointerEvent) {
 		if (this.#disabled.current) return;
-		if (e.pointerType === "touch") return e.preventDefault();
 		if (e.button > 0) return;
 		// by default, it will attempt to focus this trigger on pointerdown
-		// since this also opens the dialog we want to prevent that behavior
+		// since this also closes the dialog and restores focus we want to prevent that behavior
 		e.preventDefault();
 
 		this.#root.handleClose();

packages/bits-ui/src/lib/bits/pagination/pagination.svelte.ts

@@ -1,6 +1,6 @@
 import { useRefById } from "svelte-toolbelt";
 import type { Page, PageItem } from "./types.js";
-import type { BitsKeyboardEvent, BitsPointerEvent, WithRefProps } from "$lib/internal/types.js";
+import type { BitsKeyboardEvent, BitsMouseEvent, WithRefProps } from "$lib/internal/types.js";
 import type { ReadableBoxedValues, WritableBoxedValues } from "$lib/internal/box.svelte.js";
 import { getDataOrientation } from "$lib/internal/attrs.js";
 import { getElemDirection } from "$lib/internal/locale.js";
@@ -143,25 +143,16 @@ class PaginationPageState {
 			ref: this.#ref,
 		});
 
-		this.onpointerdown = this.onpointerdown.bind(this);
-		this.onpointerup = this.onpointerup.bind(this);
+		this.onclick = this.onclick.bind(this);
 		this.onkeydown = this.onkeydown.bind(this);
 	}
 
-	onpointerdown(e: BitsPointerEvent) {
+	onclick(e: BitsMouseEvent) {
 		if (this.#disabled.current) return;
-		if (e.pointerType === "touch") return e.preventDefault();
+		if (e.button !== 0) return;
 		this.#root.setPage(this.page.current.value);
 	}
 
-	onpointerup(e: BitsPointerEvent) {
-		if (this.#disabled.current) return;
-		if (e.pointerType === "touch") {
-			e.preventDefault();
-			this.#root.setPage(this.page.current.value);
-		}
-	}
-
 	onkeydown(e: BitsKeyboardEvent) {
 		if (e.key === kbd.SPACE || e.key === kbd.ENTER) {
 			e.preventDefault();
@@ -180,8 +171,7 @@ class PaginationPageState {
 				"data-selected": this.#isSelected ? "" : undefined,
 				[PAGE_ATTR]: "",
 				//
-				onpointerdown: this.onpointerdown,
-				onpointerup: this.onpointerup,
+				onclick: this.onclick,
 				onkeydown: this.onkeydown,
 			}) as const
 	);
@@ -217,8 +207,7 @@ class PaginationButtonState {
 			ref: this.#ref,
 		});
 
-		this.onpointerdown = this.onpointerdown.bind(this);
-		this.onpointerup = this.onpointerup.bind(this);
+		this.onclick = this.onclick.bind(this);
 		this.onkeydown = this.onkeydown.bind(this);
 	}
 
@@ -233,20 +222,12 @@ class PaginationButtonState {
 		return false;
 	});
 
-	onpointerdown(e: BitsPointerEvent) {
+	onclick(e: BitsMouseEvent) {
 		if (this.#disabled.current) return;
-		if (e.pointerType === "touch") return e.preventDefault();
+		if (e.button !== 0) return;
 		this.#action();
 	}
 
-	onpointerup(e: BitsPointerEvent) {
-		if (this.#disabled.current) return;
-		if (e.pointerType === "touch") {
-			e.preventDefault();
-			this.#action();
-		}
-	}
-
 	onkeydown(e: BitsKeyboardEvent) {
 		if (e.key === kbd.SPACE || e.key === kbd.ENTER) {
 			e.preventDefault();
@@ -264,8 +245,7 @@ class PaginationButtonState {
 				[NEXT_ATTR]: this.type === "next" ? "" : undefined,
 				disabled: this.#isDisabled,
 				//
-				onpointerdown: this.onpointerdown,
-				onpointerup: this.onpointerup,
+				onclick: this.onclick,
 				onkeydown: this.onkeydown,
 			}) as const
 	);

packages/bits-ui/src/lib/bits/popover/components/popover-content-static.svelte

@@ -35,13 +35,13 @@
 	function handleInteractOutside(e: PointerEvent) {
 		onInteractOutside(e);
 		if (e.defaultPrevented) return;
-		contentState.root.close();
+		contentState.root.handleClose();
 	}
 
 	function handleEscapeKeydown(e: KeyboardEvent) {
 		onEscapeKeydown(e);
 		if (e.defaultPrevented) return;
-		contentState.root.close();
+		contentState.root.handleClose();
 	}
 
 	function handleCloseAutoFocus(e: Event) {

packages/bits-ui/src/lib/bits/popover/components/popover-content.svelte

@@ -7,6 +7,7 @@
 	import { useId } from "$lib/internal/use-id.js";
 	import { getFloatingContentCSSVars } from "$lib/internal/floating-svelte/floating-utils.svelte.js";
 	import PopperLayerForceMount from "$lib/bits/utilities/popper-layer/popper-layer-force-mount.svelte";
+	import { isHTMLElement } from "$lib/internal/is.js";
 
 	let {
 		child,
@@ -35,13 +36,14 @@
 	function handleInteractOutside(e: PointerEvent) {
 		onInteractOutside(e);
 		if (e.defaultPrevented) return;
-		contentState.root.close();
+		if (isHTMLElement(e.target) && e.target.closest("[data-popover-trigger")) return;
+		contentState.root.handleClose();
 	}
 
 	function handleEscapeKeydown(e: KeyboardEvent) {
 		onEscapeKeydown(e);
 		if (e.defaultPrevented) return;
-		contentState.root.close();
+		contentState.root.handleClose();
 	}
 
 	function handleCloseAutoFocus(e: Event) {

packages/bits-ui/src/lib/bits/popover/popover.svelte.ts

@@ -1,9 +1,14 @@
-import { type ReadableBoxedValues, useRefById } from "svelte-toolbelt";
+import { type ReadableBoxedValues, afterSleep, afterTick, useRefById } from "svelte-toolbelt";
 import type { WritableBoxedValues } from "$lib/internal/box.svelte.js";
 import { kbd } from "$lib/internal/kbd.js";
 import { getAriaExpanded, getDataOpenClosed } from "$lib/internal/attrs.js";
 import { createContext } from "$lib/internal/create-context.js";
-import type { BitsKeyboardEvent, BitsPointerEvent, WithRefProps } from "$lib/internal/types.js";
+import type {
+	BitsKeyboardEvent,
+	BitsMouseEvent,
+	BitsPointerEvent,
+	WithRefProps,
+} from "$lib/internal/types.js";
 
 type PopoverRootStateProps = WritableBoxedValues<{
 	open: boolean;
@@ -23,7 +28,7 @@ class PopoverRootState {
 		this.open.current = !this.open.current;
 	}
 
-	close() {
+	handleClose() {
 		if (!this.open.current) return;
 		this.open.current = false;
 	}
@@ -51,23 +56,23 @@ class PopoverTriggerState {
 			},
 		});
 
+		this.onclick = this.onclick.bind(this);
 		this.onpointerdown = this.onpointerdown.bind(this);
-		this.onpointerup = this.onpointerup.bind(this);
 		this.onkeydown = this.onkeydown.bind(this);
 	}
 
-	onpointerdown(e: BitsPointerEvent) {
+	onclick(e: BitsMouseEvent) {
 		if (this.#disabled.current) return;
-		if (e.pointerType === "touch" || e.button !== 0) return e.preventDefault();
+		if (e.button !== 0) return;
 		this.#root.toggleOpen();
 	}
 
-	onpointerup(e: BitsPointerEvent) {
+	onpointerdown(e: BitsPointerEvent) {
 		if (this.#disabled.current) return;
-		if (e.pointerType === "touch") {
-			e.preventDefault();
-			this.#root.toggleOpen();
-		}
+		if (e.button !== 0) return;
+		// We prevent default to prevent focus from moving to the trigger
+		// since this action will open the popover and focus will move to the content
+		e.preventDefault();
 	}
 
 	onkeydown(e: BitsKeyboardEvent) {
@@ -97,7 +102,7 @@ class PopoverTriggerState {
 				//
 				onpointerdown: this.onpointerdown,
 				onkeydown: this.onkeydown,
-				onpointerup: this.onpointerup,
+				onclick: this.onclick,
 			}) as const
 	);
 }
@@ -158,14 +163,14 @@ class PopoverCloseState {
 		this.onkeydown = this.onkeydown.bind(this);
 	}
 
-	onclick(e: BitsPointerEvent) {
-		this.#root.close();
+	onclick(_: BitsPointerEvent) {
+		this.#root.handleClose();
 	}
 
 	onkeydown(e: BitsKeyboardEvent) {
 		if (!(e.key === kbd.ENTER || e.key === kbd.SPACE)) return;
 		e.preventDefault();
-		this.#root.close();
+		this.#root.handleClose();
 	}
 
 	props = $derived.by(

packages/tests/package.json

@@ -7,7 +7,8 @@
 		"check": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json",
 		"check:watch": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json --watch",
 		"sync": "svelte-kit sync",
-		"test": "pnpm sync && vitest"
+		"test": "pnpm sync && vitest",
+		"test:ui": "pnpm sync && vitest --ui"
 	},
 	"devDependencies": {
 		"@sveltejs/adapter-auto": "^3.3.1",
@@ -21,6 +22,7 @@
 		"@types/node": "^20.17.6",
 		"@types/resize-observer-browser": "^0.1.11",
 		"@types/testing-library__jest-dom": "^5.14.9",
+		"@vitest/ui": "^2.1.8",
 		"jest-axe": "^9.0.0",
 		"jsdom": "^24.1.3",
 		"resize-observer-polyfill": "^1.5.1",

packages/tests/src/tests/accordion/accordion-multi-test-controlled.svelte

@@ -25,7 +25,7 @@
 	{value}
 </div>
 
-<button data-testid="update-value" onclick={() => value.push("item-2")}> Update Value </button>
+<button data-testid="update-value" onclick={() => value.push("item-1")}> Update Value </button>
 
 <Accordion.Root
 	type="multiple"