Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fix(form): Floating Label for controlled value Invalid numbers
This also fixes the weird case where the user does a flow like:

- type "0123-"
- tab / blur field
- refocus field
- delete the entire string in one keystroke
- tab / blur field

Apparently change events do not get fired for this flow since
`"0123-" === ""` so going from `"" -> ""` doesn't fire a change
event....
  • Loading branch information
mlaursen committed Nov 23, 2020
1 parent e452aff commit ef1d764
Show file tree
Hide file tree
Showing 3 changed files with 186 additions and 17 deletions.
6 changes: 4 additions & 2 deletions packages/form/src/text-field/TextField.tsx
Expand Up @@ -162,15 +162,16 @@ export const TextField = forwardRef<HTMLInputElement, TextFieldProps>(
) {
const { id, value, defaultValue } = props;

const [focused, onFocus, onBlur] = useFocusState({
const [focused, onFocus, handleBlur] = useFocusState({
onBlur: propOnBlur,
onFocus: propOnFocus,
});

const [valued, onChange] = useValuedState({
const [valued, onChange, onBlur] = useValuedState<HTMLInputElement>({
value,
defaultValue,
onChange: propOnChange,
onBlur: handleBlur,
});

const { theme, underlineDirection } = useFormTheme({
Expand Down Expand Up @@ -231,6 +232,7 @@ export const TextField = forwardRef<HTMLInputElement, TextFieldProps>(
}
);

/* istanbul ignore next */
if (process.env.NODE_ENV !== "production") {
try {
const PropTypes = require("prop-types");
Expand Down
142 changes: 141 additions & 1 deletion packages/form/src/text-field/__tests__/TextField.tsx
@@ -1,4 +1,4 @@
import React from "react";
import React, { ReactElement, useState } from "react";
import { fireEvent, render } from "@testing-library/react";

import { TextField } from "../TextField";
Expand Down Expand Up @@ -69,6 +69,95 @@ describe("TextField", () => {
expect(label.className).toContain("rmd-floating-label--inactive");
});

it("should add the inactive floating label state when a number text field is blurred while containing an invalid value when controlled", () => {
function Test(): ReactElement {
const [value, setValue] = useState("");

return (
<TextField
id="text-field"
label="Label"
type="number"
value={value}
onChange={(event) => setValue(event.currentTarget.value)}
/>
);
}
const { getByRole, getByText } = render(<Test />);

const field = getByRole("spinbutton") as HTMLInputElement;
const label = getByText("Label");
expect(field).toHaveAttribute("value", "");
expect(label.className).not.toContain("rmd-floating-label--active");
expect(label.className).not.toContain("rmd-floating-label--inactive");

fireEvent.focus(field);
expect(label.className).toContain("rmd-floating-label--active");
expect(label.className).not.toContain("rmd-floating-label--inactive");

fireEvent.change(field, { target: { value: "123" } });
expect(label.className).toContain("rmd-floating-label--active");
expect(label.className).not.toContain("rmd-floating-label--inactive");

// TODO: Look into writing real browser tests since this isn't implemented in JSDOM
Object.defineProperty(field.validity, "badInput", {
writable: true,
value: true,
});
expect(field.validity.badInput).toBe(true);
fireEvent.change(field, {
target: { value: "123-" },
});
expect(field.validity.badInput).toBe(true);
expect(label.className).toContain("rmd-floating-label--active");
expect(label.className).not.toContain("rmd-floating-label--inactive");

fireEvent.blur(field);
expect(label.className).toContain("rmd-floating-label--active");
expect(label.className).toContain("rmd-floating-label--inactive");
});

it("should add the floating inactive state for a number field that is initially rendered with a value", () => {
const onBlur = jest.fn();
function Test(): ReactElement {
const [value, setValue] = useState("0");

return (
<TextField
id="text-field"
label="Label"
type="number"
value={value}
onBlur={onBlur}
onChange={(event) => setValue(event.currentTarget.value)}
/>
);
}
const { getByRole, getByText } = render(<Test />);

const field = getByRole("spinbutton") as HTMLInputElement;
const label = getByText("Label");
expect(field).toHaveAttribute("value", "0");
expect(label.className).toContain("rmd-floating-label--active");
expect(label.className).toContain("rmd-floating-label--inactive");

fireEvent.focus(field);
fireEvent.change(field, { target: { value: "" } });
fireEvent.blur(field);
expect(onBlur).toBeCalledTimes(1);
expect(label.className).not.toContain("rmd-floating-label--active");
expect(label.className).not.toContain("rmd-floating-label--inactive");

fireEvent.focus(field);
fireEvent.change(field, { target: { value: "3" } });
fireEvent.change(field, { target: { value: "3-" } });
fireEvent.change(field, { target: { value: "3" } });
fireEvent.blur(field);
expect(onBlur).toBeCalledTimes(2);
expect(label.className).toContain("rmd-floating-label--active");
expect(label.className).toContain("rmd-floating-label--inactive");
});

it("should not add the inactive floating label state when a non-number type has a badInput validity", () => {
const { getByRole, getByText } = render(
<TextField id="text-field" label="Label" type="url" defaultValue="" />
Expand Down Expand Up @@ -108,4 +197,55 @@ describe("TextField", () => {
expect(label.className).not.toContain("rmd-floating-label--active");
expect(label.className).not.toContain("rmd-floating-label--inactive");
});

it("should not add the inactive floating label state when a non-number type has a badInput validity when controlled", () => {
function Test(): ReactElement {
const [value, setValue] = useState("");

return (
<TextField
id="text-field"
label="Label"
type="url"
value={value}
onChange={(event) => setValue(event.currentTarget.value)}
/>
);
}
const { getByRole, getByText } = render(<Test />);

const field = getByRole("textbox") as HTMLInputElement;
const label = getByText("Label");
expect(field).toHaveAttribute("value", "");
expect(label.className).not.toContain("rmd-floating-label--active");
expect(label.className).not.toContain("rmd-floating-label--inactive");

fireEvent.focus(field);
expect(label.className).toContain("rmd-floating-label--active");
expect(label.className).not.toContain("rmd-floating-label--inactive");

// TODO: Look into writing real browser tests since this isn't implemented in JSDOM
Object.defineProperty(field.validity, "badInput", {
writable: true,
value: true,
});
fireEvent.change(field, { target: { value: "123" } });
expect(field.validity.badInput).toBe(true);
expect(label.className).toContain("rmd-floating-label--active");
expect(label.className).not.toContain("rmd-floating-label--inactive");

fireEvent.blur(field);
expect(field.validity.badInput).toBe(true);
expect(label.className).toContain("rmd-floating-label--active");
expect(label.className).toContain("rmd-floating-label--inactive");

fireEvent.focus(field);
expect(label.className).toContain("rmd-floating-label--active");
expect(label.className).not.toContain("rmd-floating-label--inactive");

fireEvent.change(field, { target: { value: "" } });
fireEvent.blur(field);
expect(label.className).not.toContain("rmd-floating-label--active");
expect(label.className).not.toContain("rmd-floating-label--inactive");
});
});
55 changes: 41 additions & 14 deletions packages/form/src/text-field/useValuedState.ts
@@ -1,11 +1,12 @@
import { useCallback } from "react";
import { FocusEvent, FocusEventHandler, useCallback } from "react";
import { useRefCache, useToggle } from "@react-md/utils";

type TextElement = HTMLInputElement | HTMLTextAreaElement | HTMLSelectElement;
type Value = string | number | (string | number)[];
type ChangeEventHandler<T extends TextElement> = React.ChangeEventHandler<T>;

interface Options<T extends TextElement> {
onBlur?: FocusEventHandler<T>;
onChange?: ChangeEventHandler<T>;
value?: Value;
defaultValue?: Value;
Expand All @@ -18,10 +19,11 @@ interface Options<T extends TextElement> {
* @private
*/
export function useValuedState<T extends TextElement>({
onBlur,
onChange,
value,
defaultValue,
}: Options<T>): [boolean, ChangeEventHandler<T> | undefined] {
}: Options<T>): [boolean, ChangeEventHandler<T>, FocusEventHandler<T>] {
const handler = useRefCache(onChange);
const [valued, enable, disable] = useToggle(() => {
if (typeof value === "undefined") {
Expand All @@ -30,8 +32,11 @@ export function useValuedState<T extends TextElement>({
);
}

// this isn't used for controlled components
return false;
if (typeof value === "string") {
return value.length > 0;
}

return typeof value === "number";
});

const handleChange = useCallback<React.ChangeEventHandler<T>>(
Expand All @@ -41,12 +46,17 @@ export function useValuedState<T extends TextElement>({
onChange(event);
}

if (event.currentTarget.value.length > 0) {
const input = event.currentTarget;
if (input.getAttribute("type") === "number") {
input.checkValidity();
if (input.validity.badInput) {
return;
}
}

if (input.value.length > 0) {
enable();
} else if (
event.currentTarget.getAttribute("type") !== "number" ||
!event.currentTarget.validity.badInput
) {
} else {
disable();
}
},
Expand All @@ -55,10 +65,27 @@ export function useValuedState<T extends TextElement>({
[enable, disable]
);

if (typeof value !== "undefined") {
const isValued = typeof value === "number" || value.length > 0;
return [isValued, onChange];
}
// This is **really** only for TextField components and the input
// type="number". When there is a badInput, a change event does not get fired
// again once it is "fixed" or emptied.
const handleBlur = useCallback(
(event: FocusEvent<T>) => {
if (onBlur) {
onBlur(event);
}

const input = event.currentTarget;
if (input.getAttribute("type") === "number") {
input.checkValidity();
if (input.validity.badInput || input.value.length > 0) {
return;
}

disable();
}
},
[onBlur, disable]
);

return [valued, handleChange];
return [valued, handleChange, handleBlur];
}

0 comments on commit ef1d764

Please sign in to comment.