From 1347549164a69c1c8ffbdb57aa9d87c3daddeab8 Mon Sep 17 00:00:00 2001 From: Damian Tarnawski Date: Thu, 1 Sep 2022 16:10:33 +0000 Subject: [PATCH 1/6] fix: createButton and createFocusable now are passing props through --- packages/button/src/createButton.ts | 23 +++++------------------ packages/focus/src/createFocusable.ts | 15 +++++++-------- 2 files changed, 12 insertions(+), 26 deletions(-) diff --git a/packages/button/src/createButton.ts b/packages/button/src/createButton.ts index 45bd5d7..c8403a5 100644 --- a/packages/button/src/createButton.ts +++ b/packages/button/src/createButton.ts @@ -14,13 +14,13 @@ * 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 { 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"; @@ -82,7 +82,6 @@ export function createButton( type: "button" }; - // eslint-disable-next-line solid/reactivity props = mergeProps(defaultProps, props); const additionalButtonElementProps: JSX.ButtonHTMLAttributes = { @@ -120,12 +119,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, [ @@ -141,8 +136,6 @@ export function createButton( const { focusableProps } = createFocusable(props, ref); - const domProps = filterDOMProps(props, { labelable: true }); - const baseButtonProps: JSX.HTMLAttributes = { get "aria-haspopup"() { return props["aria-haspopup"]; @@ -169,13 +162,7 @@ export function createButton( } }; - const buttonProps = combineProps( - additionalProps, - focusableProps, - pressProps, - domProps, - baseButtonProps - ); + const buttonProps = combineProps(additionalProps, focusableProps, pressProps, baseButtonProps); return { buttonProps, isPressed }; } diff --git a/packages/focus/src/createFocusable.ts b/packages/focus/src/createFocusable.ts index 2749407..0a4db3c 100644 --- a/packages/focus/src/createFocusable.ts +++ b/packages/focus/src/createFocusable.ts @@ -45,11 +45,11 @@ export interface CreateFocusableProps extends CreateFocusProps, CreateKeyboardPr excludeFromTabOrder?: MaybeAccessor; } -export interface FocusableResult { +export interface FocusableResult { /** * Props to spread onto the target element. */ - focusableProps: JSX.HTMLAttributes; + focusableProps: Props & JSX.HTMLAttributes; } // TODO: add all the focus provider stuff when needed @@ -57,21 +57,20 @@ export interface FocusableResult { /** * Make an element focusable, capable of auto focus and excludable from tab order. */ -export function createFocusable( - props: CreateFocusableProps, +export function createFocusable( + props: Props, ref: Accessor -): FocusableResult { +): FocusableResult { const [autofocus, setAutofocus] = createSignal(!!access(props.autofocus)); const { focusProps } = createFocus(props); const { keyboardProps } = createKeyboard(props); - const focusableProps = { - ...combineProps(focusProps, keyboardProps), + const focusableProps = combineProps(props, focusProps, keyboardProps, { get tabIndex() { return access(props.excludeFromTabOrder) && !access(props.isDisabled) ? -1 : undefined; } - }; + }) as Props; onMount(() => { autofocus() && ref()?.focus(); From e59b6d704db5c7027b1a3aff03cca7d1410e2aeb Mon Sep 17 00:00:00 2001 From: Damian Tarnawski Date: Thu, 1 Sep 2022 16:11:13 +0000 Subject: [PATCH 2/6] test: add tests to button and focus for user props passing --- packages/button/test/createButton.test.tsx | 26 ++++++++++++++++++ packages/focus/test/createFocusable.test.ts | 30 +++++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 packages/focus/test/createFocusable.test.ts diff --git a/packages/button/test/createButton.test.tsx b/packages/button/test/createButton.test.tsx index 8c88440..2e89627 100644 --- a/packages/button/test/createButton.test.tsx +++ b/packages/button/test/createButton.test.tsx @@ -15,6 +15,8 @@ * governing permissions and limitations under the License. */ +import { JSX } from "solid-js"; + import { createButton } from "../src"; import { AriaButtonProps } from "../src/types"; @@ -88,4 +90,28 @@ describe("createButton", () => { // @ts-ignore expect(buttonProps.rel).toBeUndefined(); }); + + it("user props are passed down to the returned button props", () => { + const ref = document.createElement("button"); + const onMouseMove = jest.fn(); + const props: AriaButtonProps<"button"> & JSX.IntrinsicElements["button"] = { + children: "Hello", + class: "test-class", + onMouseMove, + style: { color: "red" } + }; + + const { buttonProps } = createButton(props, () => ref); + + 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/test/createFocusable.test.ts b/packages/focus/test/createFocusable.test.ts new file mode 100644 index 0000000..4a25bd5 --- /dev/null +++ b/packages/focus/test/createFocusable.test.ts @@ -0,0 +1,30 @@ +/* eslint-disable solid/reactivity */ +import { JSX } from "solid-js"; + +import { createFocusable, CreateFocusableProps } from "../src"; + +describe("createFocusable", () => { + test("user props are passed through", () => { + const ref = document.createElement("button"); + const onClick = jest.fn(); + const props: CreateFocusableProps & JSX.IntrinsicElements["button"] = { + children: "Hello", + class: "test-class", + onClick, + style: { color: "red" } + }; + + const { focusableProps } = createFocusable(props, () => ref); + + 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(); + }); +}); From 5e0bdc07f4e087f9dc37759e34ff9c017f6ebf56 Mon Sep 17 00:00:00 2001 From: Damian Tarnawski Date: Thu, 1 Sep 2022 16:14:30 +0000 Subject: [PATCH 3/6] chore: add changeset --- .changeset/rude-melons-own.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/rude-melons-own.md 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) From b5df53680e6b6ef223fec4b978dcf7762262a8ae Mon Sep 17 00:00:00 2001 From: Damian Tarnawski Date: Fri, 2 Sep 2022 09:28:32 +0000 Subject: [PATCH 4/6] fix: createFocusable types weren't right --- packages/focus/src/createFocusable.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/focus/src/createFocusable.ts b/packages/focus/src/createFocusable.ts index 0a4db3c..e39d506 100644 --- a/packages/focus/src/createFocusable.ts +++ b/packages/focus/src/createFocusable.ts @@ -45,11 +45,11 @@ export interface CreateFocusableProps extends CreateFocusProps, CreateKeyboardPr excludeFromTabOrder?: MaybeAccessor; } -export interface FocusableResult { +export interface FocusableResult { /** * Props to spread onto the target element. */ - focusableProps: Props & JSX.HTMLAttributes; + focusableProps: JSX.HTMLAttributes; } // TODO: add all the focus provider stuff when needed @@ -57,10 +57,10 @@ export interface FocusableResult { /** * Make an element focusable, capable of auto focus and excludable from tab order. */ -export function createFocusable( - props: Props, +export function createFocusable( + props: CreateFocusableProps, ref: Accessor -): FocusableResult { +): FocusableResult { const [autofocus, setAutofocus] = createSignal(!!access(props.autofocus)); const { focusProps } = createFocus(props); @@ -70,7 +70,7 @@ export function createFocusable( get tabIndex() { return access(props.excludeFromTabOrder) && !access(props.isDisabled) ? -1 : undefined; } - }) as Props; + }); onMount(() => { autofocus() && ref()?.focus(); From f0016c75ab7796d199b62d1542faf66d96526b36 Mon Sep 17 00:00:00 2001 From: Damian Tarnawski Date: Fri, 2 Sep 2022 10:03:20 +0000 Subject: [PATCH 5/6] fix: createTextField wasn't proparely handing passing props through by createFocusable --- packages/textfield/src/createTextField.ts | 199 ++++++++++-------- .../textfield/test/createTextField.test.ts | 31 ++- 2 files changed, 138 insertions(+), 92 deletions(-) diff --git a/packages/textfield/src/createTextField.ts b/packages/textfield/src/createTextField.ts index 38e074c..643cf6e 100644 --- a/packages/textfield/src/createTextField.ts +++ b/packages/textfield/src/createTextField.ts @@ -31,16 +31,15 @@ import { Validation, ValueBase } from "@solid-aria/types"; -import { callHandler, filterDOMProps } from "@solid-aria/utils"; -import { combineProps } from "@solid-primitives/props"; -import { Accessor, createMemo, JSX, mergeProps } from "solid-js"; +import { callHandler } from "@solid-aria/utils"; +import { Accessor, JSX, mergeProps, splitProps } from "solid-js"; type DefaultElementType = "input"; /** * The intrinsic HTML element names that `createTextField` supports; e.g. `input`, `textarea`. */ -type TextFieldIntrinsicElements = keyof Pick; +type TextFieldIntrinsicElements = "input" | "textarea"; /** * The HTML element interfaces that `createTextField` supports based on what is @@ -121,6 +120,23 @@ export interface TextFieldAria; } +const inputPropKeys = [ + "type", + "pattern", + "aria-errormessage", + "aria-activedescendant", + "aria-autocomplete", + "aria-haspopup", + "value", + "defaultValue", + "autocomplete", + "maxLength", + "minLength", + "name", + "placeholder", + "inputMode" +] as const; + /** * Provides the behavior and accessibility implementation for a text field. * @param props - Props for the text field. @@ -141,93 +157,96 @@ export function createTextField; - const { focusableProps } = createFocusable(props, ref); - const { labelProps, fieldProps, descriptionProps, errorMessageProps } = createField(props); - - const domProps = filterDOMProps(props, { labelable: true }); - - const baseInputProps: JSX.HTMLAttributes = mergeProps( - { - get type() { - return props.inputElementType === "input" ? props.type : undefined; - }, - get pattern() { - return props.inputElementType === "input" ? props.pattern : undefined; - }, - get disabled() { - return props.isDisabled; - }, - get readOnly() { - return props.isReadOnly; - }, - get "aria-required"() { - return props.isRequired || undefined; - }, - get "aria-invalid"() { - return props.validationState === "invalid" || undefined; - }, - get "aria-errormessage"() { - return props["aria-errormessage"]; - }, - get "aria-activedescendant"() { - return props["aria-activedescendant"]; - }, - get "aria-autocomplete"() { - return props["aria-autocomplete"]; - }, - get "aria-haspopup"() { - return props["aria-haspopup"]; - }, - get value() { - return props.value; - }, - get defaultValue() { - return props.value ? undefined : props.defaultValue; - }, - get autocomplete() { - return props.autocomplete; - }, - get maxLength() { - return props.maxLength; - }, - get minLength() { - return props.minLength; - }, - get name() { - return props.name; - }, - get placeholder() { - return props.placeholder; - }, - get inputMode() { - return props.inputMode; - }, - - // Change events - onChange: e => props.onChange?.((e.target as HTMLInputElement).value), - - // Clipboard events - onCopy: e => callHandler(props.onCopy, e), - onCut: e => callHandler(props.onCut, e), - onPaste: e => callHandler(props.onPaste, e), - - // Composition events - onCompositionEnd: e => callHandler(props.onCompositionEnd, e), - onCompositionStart: e => callHandler(props.onCompositionStart, e), - onCompositionUpdate: e => callHandler(props.onCompositionUpdate, e), - - // Selection events - onSelect: e => callHandler(props.onSelect, e), - - // Input events - onBeforeInput: e => callHandler(props.onBeforeInput, e), - onInput: e => callHandler(props.onInput, e) - } as JSX.HTMLAttributes, - focusableProps, - fieldProps - ); + // local props are separated so that they don't mess with mergeProps + // e.g. the `type` prop should return `undefined` if the element is not an input + // but `mergeProps` will search for the first defined value (ignoring undefined) + const localProps = splitProps(props, inputPropKeys)[1]; + + const { focusableProps } = createFocusable(localProps, ref); + const { labelProps, fieldProps, descriptionProps, errorMessageProps } = createField(localProps); + + const baseInputProps: JSX.IntrinsicElements["input"] & { defaultValue: string | undefined } = { + get type() { + return props.inputElementType === "input" ? props.type : undefined; + }, + get pattern() { + return props.inputElementType === "input" ? props.pattern : undefined; + }, + get disabled() { + return props.isDisabled; + }, + get readOnly() { + return props.isReadOnly; + }, + get "aria-required"() { + return props.isRequired || undefined; + }, + get "aria-invalid"() { + return props.validationState === "invalid" || undefined; + }, + get "aria-errormessage"() { + return props["aria-errormessage"]; + }, + get "aria-activedescendant"() { + return props["aria-activedescendant"]; + }, + get "aria-autocomplete"() { + return props["aria-autocomplete"]; + }, + get "aria-haspopup"() { + return props["aria-haspopup"]; + }, + get value() { + return props.value; + }, + get defaultValue() { + return props.value ? undefined : props.defaultValue; + }, + get autocomplete() { + return props.autocomplete; + }, + get maxLength() { + return props.maxLength; + }, + get minLength() { + return props.minLength; + }, + get name() { + return props.name; + }, + get placeholder() { + return props.placeholder; + }, + get inputMode() { + return props.inputMode; + }, + + // Change events + onChange: e => props.onChange?.((e.target as HTMLInputElement).value), + + // Clipboard events + onCopy: e => callHandler(props.onCopy, e), + onCut: e => callHandler(props.onCut, e), + onPaste: e => callHandler(props.onPaste, e), - const inputProps = combineProps(domProps, baseInputProps); + // Composition events + onCompositionEnd: e => callHandler(props.onCompositionEnd, e), + onCompositionStart: e => callHandler(props.onCompositionStart, e), + onCompositionUpdate: e => callHandler(props.onCompositionUpdate, e), + + // Selection events + onSelect: e => callHandler(props.onSelect, e), + + // Input events + onBeforeInput: e => callHandler(props.onBeforeInput, e), + onInput: e => callHandler(props.onInput, e) + }; + + const inputProps = mergeProps( + focusableProps, + fieldProps, + baseInputProps + ) as TextFieldInputProps; return { labelProps, diff --git a/packages/textfield/test/createTextField.test.ts b/packages/textfield/test/createTextField.test.ts index b7b5bbd..b01c4d9 100644 --- a/packages/textfield/test/createTextField.test.ts +++ b/packages/textfield/test/createTextField.test.ts @@ -16,9 +16,9 @@ */ import { callHandler } from "@solid-aria/utils"; -import { createRoot } from "solid-js"; +import { createRoot, JSX } from "solid-js"; -import { createTextField } from "../src"; +import { AriaTextFieldProps, createTextField } from "../src"; describe("createTextField", () => { it("should use default props if no props are provided", () => @@ -142,6 +142,33 @@ describe("createTextField", () => { expect((inputProps as any).type).toBeUndefined(); expect((inputProps as any).pattern).toBeUndefined(); + dispose(); + })); + + it("user props are passed through", () => + createRoot(dispose => { + const ref = document.createElement("input"); + const onMouseMove = jest.fn(); + const props: AriaTextFieldProps<"input"> & JSX.IntrinsicElements["input"] = { + children: "Hello", + class: "test-class", + onMouseMove, + style: { color: "red" } + }; + + const { inputProps } = createTextField(props, () => ref); + + expect(inputProps.children).toBe("Hello"); + expect(inputProps.class).toBe("test-class"); + expect(inputProps.style).toEqual({ color: "red" }); + + expect(inputProps).toHaveProperty("onMouseMove"); + expect(typeof inputProps.onMouseMove).toBe("function"); + + // @ts-expect-error + inputProps.onMouseMove(); + expect(onMouseMove).toHaveBeenCalled(); + dispose(); })); }); From 0dfc90370cae05077324b3d2400325f64d168ffa Mon Sep 17 00:00:00 2001 From: Damian Tarnawski Date: Fri, 2 Sep 2022 12:43:02 +0200 Subject: [PATCH 6/6] update changeset --- .changeset/rude-melons-own.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.changeset/rude-melons-own.md b/.changeset/rude-melons-own.md index 789562c..b855fe9 100644 --- a/.changeset/rude-melons-own.md +++ b/.changeset/rude-melons-own.md @@ -1,6 +1,7 @@ --- "@solid-aria/button": patch "@solid-aria/focus": patch +"@solid-aria/textfield": patch --- Fix user props not being passed down to the returned props object. (#69)