Skip to content
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

next: onpointerdown -> onclick #1011

Merged
merged 3 commits into from
Dec 19, 2024
Merged
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/yellow-lies-vanish.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"bits-ui": patch
---

revert to `onclick` events for most components except where it makes sense (like menus, select, etc.)
26 changes: 7 additions & 19 deletions packages/bits-ui/src/lib/bits/accordion/accordion.svelte.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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) {
Expand All @@ -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
);
Expand Down Expand Up @@ -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
Expand Down
22 changes: 7 additions & 15 deletions packages/bits-ui/src/lib/bits/collapsible/collapsible.svelte.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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({
Expand All @@ -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();
}
}
Expand All @@ -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
);
Expand Down
6 changes: 1 addition & 5 deletions packages/bits-ui/src/lib/bits/dialog/dialog.svelte.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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();
Expand Down
38 changes: 9 additions & 29 deletions packages/bits-ui/src/lib/bits/pagination/pagination.svelte.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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();
Expand All @@ -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
);
Expand Down Expand Up @@ -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);
}

Expand All @@ -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();
Expand All @@ -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
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down
35 changes: 20 additions & 15 deletions packages/bits-ui/src/lib/bits/popover/popover.svelte.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -23,7 +28,7 @@ class PopoverRootState {
this.open.current = !this.open.current;
}

close() {
handleClose() {
if (!this.open.current) return;
this.open.current = false;
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -97,7 +102,7 @@ class PopoverTriggerState {
//
onpointerdown: this.onpointerdown,
onkeydown: this.onkeydown,
onpointerup: this.onpointerup,
onclick: this.onclick,
}) as const
);
}
Expand Down Expand Up @@ -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(
Expand Down
4 changes: 3 additions & 1 deletion packages/tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Loading
Loading