diff --git a/.changeset/rude-melons-own.md b/.changeset/rude-melons-own.md new file mode 100644 index 0000000..789562c --- /dev/null +++ b/.changeset/rude-melons-own.md @@ -0,0 +1,6 @@ +--- +"@solid-aria/button": patch +"@solid-aria/focus": patch +--- + +Fix user props not being passed down to the returned props object. (#69) diff --git a/packages/button/src/createButton.ts b/packages/button/src/createButton.ts index 45bd5d7..8cbb120 100644 --- a/packages/button/src/createButton.ts +++ b/packages/button/src/createButton.ts @@ -14,21 +14,27 @@ * OF ANY KIND, either express or implied. See the License for the specific language * governing permissions and limitations under the License. */ +/* eslint-disable solid/reactivity */ -import { createFocusable } from "@solid-aria/focus"; +import { createFocusable, CreateFocusableProps } from "@solid-aria/focus"; import { createPress } from "@solid-aria/interactions"; import { ElementType } from "@solid-aria/types"; -import { filterDOMProps } from "@solid-aria/utils"; import { combineProps } from "@solid-primitives/props"; -import { Accessor, createMemo, JSX, mergeProps, splitProps } from "solid-js"; +import { Accessor, JSX, mergeProps, splitProps } from "solid-js"; -import { AriaButtonProps } from "./types"; +import { AriaButtonProps, ElementOfType, PropsOfType } from "./types"; + +export interface ButtonAria { + /** + * A ref to apply onto the target element. + * Merge the given `props.ref` and all parents `PressResponder` refs. + */ + ref: (el: ElementOfType) => void; -export interface ButtonAria { /** * Props for the button element. */ - buttonProps: T; + buttonProps: Omit, "ref">; /** * Whether the button is currently pressed. @@ -36,54 +42,13 @@ export interface ButtonAria { isPressed: Accessor; } -// Order with overrides is important: 'button' should be default -export function createButton( - props: AriaButtonProps<"button">, - ref: Accessor -): ButtonAria>; - -export function createButton( - props: AriaButtonProps<"a">, - ref: Accessor -): ButtonAria>; - -export function createButton( - props: AriaButtonProps<"div">, - ref: Accessor -): ButtonAria>; - -export function createButton( - props: AriaButtonProps<"input">, - ref: Accessor -): ButtonAria>; - -export function createButton( - props: AriaButtonProps<"span">, - ref: Accessor -): ButtonAria>; - -export function createButton( - props: AriaButtonProps, - ref: Accessor -): ButtonAria>; - /** * Provides the behavior and accessibility implementation for a button component. Handles mouse, keyboard, and touch interactions, * focus behavior, and ARIA props for both native button elements and custom element types. * @param props - Props to be applied to the button. - * @param ref - A ref to a DOM element for the button. */ -export function createButton( - props: AriaButtonProps, - ref: Accessor -): ButtonAria> { - const defaultProps: AriaButtonProps<"button"> = { - elementType: "button", - type: "button" - }; - - // eslint-disable-next-line solid/reactivity - props = mergeProps(defaultProps, props); +export function createButton(props: AriaButtonProps): ButtonAria { + props = mergeProps({ elementType: "button", type: "button" }, props) as typeof props; const additionalButtonElementProps: JSX.ButtonHTMLAttributes = { get type() { @@ -120,12 +85,8 @@ export function createButton( } }; - const additionalProps = mergeProps( - createMemo(() => { - return props.elementType === "button" - ? additionalButtonElementProps - : additionalOtherElementProps; - }) + const additionalProps = mergeProps(() => + props.elementType === "button" ? additionalButtonElementProps : additionalOtherElementProps ); const [createPressProps] = splitProps(props, [ @@ -137,11 +98,12 @@ export function createButton( "preventFocusOnPress" ]); - const { pressProps, isPressed } = createPress(createPressProps); + const { pressProps, isPressed, ref: pressRef } = createPress(createPressProps); - const { focusableProps } = createFocusable(props, ref); - - const domProps = filterDOMProps(props, { labelable: true }); + // createFocusable has problems with the `props` type because if you pass a component as the `elementType` prop, there is no telling if the `ref` will be a HTML Element. + const { focusableProps, ref: focusRef } = createFocusable( + props as CreateFocusableProps + ); const baseButtonProps: JSX.HTMLAttributes = { get "aria-haspopup"() { @@ -173,9 +135,15 @@ export function createButton( additionalProps, focusableProps, pressProps, - domProps, baseButtonProps - ); - - return { buttonProps, isPressed }; + ) as Omit, "ref">; + + return { + buttonProps, + isPressed, + ref: el => { + focusRef(el as HTMLElement); + pressRef(el as HTMLElement); + } + }; } diff --git a/packages/button/src/types.ts b/packages/button/src/types.ts index 03a9f8e..2dbd5ad 100644 --- a/packages/button/src/types.ts +++ b/packages/button/src/types.ts @@ -23,7 +23,30 @@ import { FocusableProps, PressEvents } from "@solid-aria/types"; -import { JSX } from "solid-js"; +import { Component, JSX, Ref } from "solid-js"; + +// TODO: these probably should be added to @solid-aria/types + +/** + * Infers the type of `ref` element from the `elementType` prop. + * @example + * "button" -> HTMLButtonElement + * "div" -> HTMLDivElement + * Component<{ ref: Ref }> -> HTMLInputElement + */ +export type ElementOfType = T extends keyof JSX.IntrinsicElements + ? JSX.IntrinsicElements[T] extends JSX.CustomAttributes + ? U + : never + : T extends Component<{ ref: Ref }> + ? U + : never; + +export type PropsOfType = T extends keyof JSX.IntrinsicElements + ? JSX.IntrinsicElements[T] + : T extends Component + ? Parameters[0] + : never; interface ButtonProps extends PressEvents, FocusableProps { /** @@ -38,6 +61,11 @@ interface ButtonProps extends PressEvents, FocusableProps { } export interface AriaButtonElementTypeProps { + /** + * A ref to the target element. + */ + ref?: Ref>; + /** * The HTML element or SolidJS component used to render the button, e.g. 'div', 'a', or `Link`. * @default 'button' diff --git a/packages/button/test/createButton.test.tsx b/packages/button/test/createButton.test.tsx index 8c88440..148ff54 100644 --- a/packages/button/test/createButton.test.tsx +++ b/packages/button/test/createButton.test.tsx @@ -15,22 +15,22 @@ * governing permissions and limitations under the License. */ +import { JSX } from "solid-js"; + import { createButton } from "../src"; import { AriaButtonProps } from "../src/types"; describe("createButton", () => { it("handles defaults", () => { - let ref: any; const props = {}; - const { buttonProps } = createButton(props, () => ref); + const { buttonProps } = createButton(props); expect(typeof buttonProps.onClick).toBe("function"); }); it("handles elements other than button", () => { - let ref: any; const props: AriaButtonProps<"a"> = { elementType: "a" }; - const { buttonProps } = createButton(props, () => ref); + const { buttonProps } = createButton(props); expect(buttonProps.role).toBe("button"); expect(buttonProps.tabIndex).toBe(0); @@ -41,9 +41,8 @@ describe("createButton", () => { }); it("handles elements other than button disabled", () => { - let ref: any; const props: AriaButtonProps<"a"> = { elementType: "a", isDisabled: true }; - const { buttonProps } = createButton(props, () => ref); + const { buttonProps } = createButton(props); expect(buttonProps.role).toBe("button"); expect(buttonProps.tabIndex).toBeUndefined(); @@ -54,25 +53,22 @@ describe("createButton", () => { }); it("handles the rel attribute on anchors", () => { - let ref: any; const props: AriaButtonProps<"a"> = { elementType: "a", rel: "noopener noreferrer" }; - const { buttonProps } = createButton(props, () => ref); + const { buttonProps } = createButton(props); expect(buttonProps.rel).toBe("noopener noreferrer"); }); it("handles the rel attribute as a string on anchors", () => { - let ref: any; const props: AriaButtonProps<"a"> = { elementType: "a", rel: "search" }; - const { buttonProps } = createButton(props, () => ref); + const { buttonProps } = createButton(props); expect(buttonProps.rel).toBe("search"); }); it("handles input elements", () => { - let ref: any; const props: AriaButtonProps<"input"> = { elementType: "input", isDisabled: true }; - const { buttonProps } = createButton(props, () => ref); + const { buttonProps } = createButton(props); expect(buttonProps.role).toBe("button"); expect(buttonProps.tabIndex).toBeUndefined(); @@ -88,4 +84,27 @@ describe("createButton", () => { // @ts-ignore expect(buttonProps.rel).toBeUndefined(); }); + + it("user props are passed down to the returned button props", () => { + const onMouseMove = jest.fn(); + const props: AriaButtonProps<"button"> & JSX.IntrinsicElements["button"] = { + children: "Hello", + class: "test-class", + onMouseMove, + style: { color: "red" } + }; + + const { buttonProps } = createButton(props); + + expect(buttonProps.children).toBe("Hello"); + expect(buttonProps.class).toBe("test-class"); + expect(buttonProps.style).toEqual({ color: "red" }); + + expect(buttonProps).toHaveProperty("onMouseMove"); + expect(typeof buttonProps.onMouseMove).toBe("function"); + + // @ts-expect-error + buttonProps.onMouseMove(); + expect(onMouseMove).toHaveBeenCalled(); + }); }); diff --git a/packages/focus/src/createFocusable.ts b/packages/focus/src/createFocusable.ts index 2749407..b8d30a9 100644 --- a/packages/focus/src/createFocusable.ts +++ b/packages/focus/src/createFocusable.ts @@ -23,9 +23,16 @@ import { } from "@solid-aria/interactions"; import { combineProps } from "@solid-primitives/props"; import { access, MaybeAccessor } from "@solid-primitives/utils"; -import { Accessor, createSignal, JSX, onMount } from "solid-js"; +import { createSignal, JSX, onMount, Ref, splitProps } from "solid-js"; + +export interface CreateFocusableProps + extends CreateFocusProps, + CreateKeyboardProps { + /** + * A ref to the target element. + */ + ref?: Ref; -export interface CreateFocusableProps extends CreateFocusProps, CreateKeyboardProps { /** * Whether focus should be disabled. */ @@ -45,7 +52,13 @@ export interface CreateFocusableProps extends CreateFocusProps, CreateKeyboardPr excludeFromTabOrder?: MaybeAccessor; } -export interface FocusableResult { +export interface FocusableResult { + /** + * A ref to apply onto the target element. + * Merge the given `props.ref` and all parents `PressResponder` refs. + */ + ref: (el: El) => void; + /** * Props to spread onto the target element. */ @@ -57,26 +70,30 @@ export interface FocusableResult { /** * Make an element focusable, capable of auto focus and excludable from tab order. */ -export function createFocusable( - props: CreateFocusableProps, - ref: Accessor -): FocusableResult { +export function createFocusable( + props: CreateFocusableProps +): FocusableResult { + // eslint-disable-next-line solid/reactivity const [autofocus, setAutofocus] = createSignal(!!access(props.autofocus)); - const { focusProps } = createFocus(props); - const { keyboardProps } = createKeyboard(props); + const propsWithoutRef = splitProps(props, ["ref"])[1]; + + const { focusProps } = createFocus(propsWithoutRef); + const { keyboardProps } = createKeyboard(propsWithoutRef); - const focusableProps = { - ...combineProps(focusProps, keyboardProps), + const focusableProps = combineProps(propsWithoutRef, focusProps, keyboardProps, { get tabIndex() { return access(props.excludeFromTabOrder) && !access(props.isDisabled) ? -1 : undefined; } - }; + }); + + let target: El | undefined; + const ref = (el: El) => (props.ref as (el: El) => void)?.((target = el)); onMount(() => { - autofocus() && ref()?.focus(); + autofocus() && target?.focus(); setAutofocus(false); }); - return { focusableProps }; + return { focusableProps, ref }; } diff --git a/packages/focus/test/createFocusable.test.ts b/packages/focus/test/createFocusable.test.ts new file mode 100644 index 0000000..fa51555 --- /dev/null +++ b/packages/focus/test/createFocusable.test.ts @@ -0,0 +1,43 @@ +/* eslint-disable solid/reactivity */ +import { JSX } from "solid-js"; + +import { createFocusable, CreateFocusableProps } from "../src"; + +describe("createFocusable", () => { + test("user props are passed through", () => { + const onClick = jest.fn(); + const props: CreateFocusableProps & JSX.IntrinsicElements["button"] = { + children: "Hello", + class: "test-class", + onClick, + style: { color: "red" } + }; + + const { focusableProps } = createFocusable(props); + + expect(focusableProps.children).toBe("Hello"); + expect(focusableProps.class).toBe("test-class"); + expect(focusableProps.style).toEqual({ color: "red" }); + + expect(focusableProps).toHaveProperty("onClick"); + expect(typeof focusableProps.onClick).toBe("function"); + + // @ts-expect-error + focusableProps.onClick(); + expect(onClick).toHaveBeenCalled(); + }); + + test("handles ref", () => { + const userRef = jest.fn(); + const { focusableProps, ref } = createFocusable({ ref: userRef }); + + expect("ref" in focusableProps).toBeFalsy(); + + expect(typeof ref).toBe("function"); + expect(ref).not.toBe(userRef); + expect(userRef).not.toHaveBeenCalled(); + const el = document.createElement("button"); + ref(el); + expect(userRef).toHaveBeenCalledWith(el); + }); +}); diff --git a/packages/interactions/src/createPress.ts b/packages/interactions/src/createPress.ts index 92b092f..619feb6 100644 --- a/packages/interactions/src/createPress.ts +++ b/packages/interactions/src/createPress.ts @@ -14,6 +14,7 @@ * OF ANY KIND, either express or implied. See the License for the specific language * governing permissions and limitations under the License. */ +/* eslint-disable solid/reactivity */ import { PointerType } from "@solid-aria/types"; import { createGlobalListeners, focusWithoutScrolling } from "@solid-aria/utils"; @@ -74,12 +75,15 @@ export function createPress(props: CreatePressProps): if (context) { context.register(); - const [, contextProps] = splitProps(context, ["register", "ref"]); + const [, contextProps] = splitProps(context, ["register"]); + // eslint-disable-next-line solid/reactivity props = combineProps(contextProps, props); } const [, domProps] = splitProps(props, [ + // ref is omited, because user should populate it himself + "ref", "onPress", "onPressChange", "onPressStart", @@ -679,12 +683,9 @@ export function createPress(props: CreatePressProps): ); return { - ref: (el: T) => { - (props.ref as ((el: T) => void) | undefined)?.(el); - context?.ref?.(el); - }, + ref: (el: T) => (props.ref as ((el: T) => void) | undefined)?.(el), isPressed, - pressProps: combineProps(domProps, pressProps) as JSX.HTMLAttributes + pressProps: combineProps(domProps, pressProps) }; }