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

feat: upstream FormikField #1054

Merged
merged 2 commits into from
Mar 20, 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
1 change: 1 addition & 0 deletions HACKING.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ Finally, in your project, add the resolutions for `react` and `react-dom` to `pa
```
"resolutions": {
"@canonical/react-components": "portal:path_to_react_components",
"formik": "portal:path_to_react_components/node_modules/formik",
"react": "portal:path_to_react_components/node_modules/react",
"react-dom": "portal:path_to_react_components/node_modules/react-dom"
}
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
"eslint-plugin-react-hooks": "4.6.0",
"eslint-plugin-storybook": "0.6.15",
"eslint-plugin-testing-library": "6.2.0",
"formik": "2.4.5",
"jest": "29.7.0",
"npm-package-json-lint": "7.1.0",
"prettier": "3.2.4",
Expand Down Expand Up @@ -111,6 +112,7 @@
"peerDependencies": {
"@types/react": "^17.0.2 || ^18.0.0",
"@types/react-dom": "^17.0.2 || ^18.0.0",
"formik": "^2.4.5",
"react": "^17.0.2 || ^18.0.0",
"react-dom": "^17.0.2 || ^18.0.0",
"vanilla-framework": "^3.15.1 || ^4.0.0"
Expand Down
17 changes: 10 additions & 7 deletions src/components/Card/Card.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,15 @@ export const Highlighted = {
};

export const Overlay = {
render: () => (
args: {
title: "Web browsing",
overlay: true,
children: `Renowned for speed and security, Ubuntu and Firefox make browsing
the web a pleasure again. Ubuntu also includes Chrome, Opera and
other browsers that can be installed from the Ubuntu Software
Centre.`,
},
render: (args) => (
<section
className="p-strip--image is-light"
style={{
Expand All @@ -48,12 +56,7 @@ export const Overlay = {
>
<Row>
<Col size={6} emptyLarge={7}>
<Card title="Web browsing" overlay>
Renowned for speed and security, Ubuntu and Firefox make browsing
the web a pleasure again. Ubuntu also includes Chrome, Opera and
other browsers that can be installed from the Ubuntu Software
Centre.
</Card>
<Card {...args} />
</Col>
</Row>
</section>
Expand Down
82 changes: 82 additions & 0 deletions src/components/FormikField/FormikField.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import React from "react";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the docs doesn't render as expected. For instance:

  • when pressing Show code, the entire code is presented, but I assume we only want the bit from render to be displayed.
  • the default value of component prop in props table is not listed in DEFAULT column.

The first issue also appears in Card stories, which is one of the few other .stories.tsx docs. There are also some slight inconsistencies in presentation between .stories.tsx and .stories.mdx docs e.g. the Props header missing in .stories.tsx docs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the docs doesn't render as expected. For instance:

  • when pressing Show code, the entire code is presented, but I assume we only want the bit from render to be displayed.

There's an open issue here, with a workaround that I've applied to both files: storybookjs/storybook#22281.

  • the default value of component prop in props table is not listed in DEFAULT column.

It looks like the table couldn't handle the Input component as a default but this can be set via a jsdoc string so I've done that.

The first issue also appears in Card stories, which is one of the few other .stories.tsx docs. There are also some slight inconsistencies in presentation between .stories.tsx and .stories.mdx docs e.g. the Props header missing in .stories.tsx docs.

It looks like it was decided not to customise the page too much and stick with the CSF defaults: #1010 (comment).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation and links, and for also fixing the inconsistency in Card stories!

import type { Meta, StoryObj } from "@storybook/react";

import FormikField from "./FormikField";
import Select from "../Select";
import { Formik } from "formik";

const meta: Meta<typeof FormikField> = {
title: "FormikField",
component: FormikField,
tags: ["autodocs"],
};

export default meta;

type Story = StoryObj<typeof FormikField>;

export const Default: Story = {
args: {
name: "username",
label: "Username",
type: "text",
},
render: (args) => (
<Formik initialValues={{ username: "" }} onSubmit={() => {}}>
<FormikField {...args} />
</Formik>
),
};

export const Fields: Story = {
args: {
component: Select,
name: "release",
label: "Release",
options: [
{ value: "", disabled: true, label: "Select an option" },
{ value: "1", label: "Cosmic Cuttlefish" },
{ value: "2", label: "Bionic Beaver" },
{ value: "3", label: "Xenial Xerus" },
],
},
parameters: {
docs: {
description: {
story: `
Any React Components input can be provided to FormikField (e.g. Input, Textarea or Select) or you may provide a custom component.

Any additional props that need to be passed can be given to FormikField.
`,
},
},
},
render: (args) => (
<Formik initialValues={{ release: "" }} onSubmit={() => {}}>
<FormikField {...args} />
</Formik>
),
};

export const Errors: Story = {
args: Default.args,
parameters: {
docs: {
description: {
story: `
Formik parameters are passed to the field using Formik's \`useField\`. This means that validation and errors, state handlers etc. should all just work.
`,
},
},
},
render: (args) => (
<Formik
initialErrors={{ username: "This username has already been taken." }}
initialTouched={{ username: true }}
initialValues={{ username: "" }}
onSubmit={() => {}}
>
<FormikField {...args} />
</Formik>
),
};
51 changes: 51 additions & 0 deletions src/components/FormikField/FormikField.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import React from "react";
import { render, screen } from "@testing-library/react";
import { Formik } from "formik";

import FormikField from "./FormikField";

describe("FormikField", () => {
it("can set a different component", () => {
const Component = () => <select />;
render(
<Formik initialValues={{}} onSubmit={jest.fn()}>
<FormikField component={Component} name="username" />
</Formik>
);

expect(screen.getByRole("combobox")).toBeInTheDocument();
expect(screen.queryByRole("textbox")).not.toBeInTheDocument();
});

it("can pass errors", () => {
render(
<Formik
initialErrors={{ username: "Uh oh!" }}
initialTouched={{ username: true }}
initialValues={{ username: "" }}
onSubmit={jest.fn()}
>
<FormikField name="username" />
</Formik>
);

expect(screen.getByRole("textbox")).toHaveAccessibleErrorMessage(
"Error: Uh oh!"
);
});

it("can hide the errors", () => {
render(
<Formik
initialErrors={{ username: "Uh oh!" }}
initialTouched={{ username: true }}
initialValues={{ username: "" }}
onSubmit={jest.fn()}
>
<FormikField displayError={false} name="username" />
</Formik>
);

expect(screen.getByRole("textbox")).not.toHaveAccessibleErrorMessage();
});
});
54 changes: 54 additions & 0 deletions src/components/FormikField/FormikField.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import React from "react";
import { useField } from "formik";
import {
type ComponentProps,
type ComponentType,
type ElementType,
type HTMLProps,
} from "react";
import Input from "components/Input";

export type Props<C extends ElementType | ComponentType = typeof Input> = {
/**
* The component to display.
* @default Input
*/
component?: C;
/**
* This can be used to hide errors returned by Formik.
*/
displayError?: boolean;
/**
* The name of the field as given to Formik.
*/
name: string;
value?: HTMLProps<HTMLElement>["value"];
} & ComponentProps<C>;

/**
* This component makes it easier to use Vanilla form inputs with Formik. It
* makes use of Formik's context to automatically map errors, values, states
* etc. onto the provided field.
*/
const FormikField = <C extends ElementType | ComponentType = typeof Input>({
component: Component = Input,
displayError = true,
name,
value,
label,
...props
}: Props<C>): JSX.Element => {
const [field, meta] = useField({ name, type: props.type, value });

return (
<Component
aria-label={label}
error={meta.touched && displayError ? meta.error : null}
label={label}
{...field}
{...props}
/>
);
};

export default FormikField;
1 change: 1 addition & 0 deletions src/components/FormikField/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default, type Props as FormikFieldProps } from "./FormikField";
6 changes: 6 additions & 0 deletions src/components/Input/Input.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@
expect(screen.getByRole("textbox")).toHaveAccessibleName("text label");
});

it("can display JSX labels for text inputs", () => {
render(<Input type="text" label={<>text label</>} />);
expect(screen.getByText("text label")).toHaveClass("p-form__label");
expect(screen.getByRole("textbox")).toHaveAccessibleName("text label");
});

it("moves the label for radio buttons", () => {
render(<Input type="radio" label="text label" />);
expect(
Expand Down Expand Up @@ -44,9 +50,9 @@

it("can take focus with delay", async () => {
render(<Input takeFocus takeFocusDelay={10} />);
expect(screen.getByRole("textbox")).not.toBe(document.activeElement);

Check warning on line 53 in src/components/Input/Input.test.tsx

View workflow job for this annotation

GitHub Actions / Lint, build and test

Avoid direct Node access. Prefer using the methods from Testing Library
await new Promise((r) => setTimeout(r, 10));
expect(screen.getByRole("textbox")).toBe(document.activeElement);

Check warning on line 55 in src/components/Input/Input.test.tsx

View workflow job for this annotation

GitHub Actions / Lint, build and test

Avoid direct Node access. Prefer using the methods from Testing Library
});

it("sets aria-invalid to false when there is no error", () => {
Expand Down
1 change: 0 additions & 1 deletion src/components/Input/Input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ const Input = ({
"aria-errormessage": hasError ? validationId : null,
"aria-invalid": hasError,
id: inputId,
label: label,
required: required,
...inputProps,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ exports[`<TablePaginationControls /> renders table pagination controls and match
aria-invalid="false"
class="p-form-validation__input u-no-margin--bottom pagination-input"
id="paginationPageInput"
label="Page number"
type="number"
value="0"
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ exports[`<TablePagination /> renders table pagination and matches the snapshot 1
aria-invalid="false"
class="p-form-validation__input u-no-margin--bottom pagination-input"
id="paginationPageInput"
label="Page number"
type="number"
value="1"
/>
Expand Down
2 changes: 2 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export { default as ContextualMenu } from "./components/ContextualMenu";
export { default as EmptyState } from "./components/EmptyState";
export { default as Field } from "./components/Field";
export { default as Form } from "./components/Form";
export { default as FormikField } from "./components/FormikField";
export { default as Icon, ICONS } from "./components/Icon";
export { default as Input } from "./components/Input";
export { default as Label } from "./components/Label";
Expand Down Expand Up @@ -93,6 +94,7 @@ export type {
export type { EmptyStateProps } from "./components/EmptyState";
export type { FieldProps } from "./components/Field";
export type { FormProps } from "./components/Form";
export type { FormikFieldProps } from "./components/FormikField";
export type { IconProps } from "./components/Icon";
export type { InputProps } from "./components/Input";
export type { LabelProps } from "./components/Label";
Expand Down
Loading
Loading