From 21e761406a2d432faf4c56e36907650d2d5da9a1 Mon Sep 17 00:00:00 2001 From: Haopeng Wang Date: Tue, 7 Nov 2023 13:49:32 +0900 Subject: [PATCH] add validation details and string-list control (#2825) add validation details and string-list control --------- Co-authored-by: hoppe --- .../src/form/string-list-parameter.ts | 29 ++- .../bonito-core/src/form/validation-status.ts | 12 +- packages/bonito-ui/i18n/resources.resjson | 1 + .../form/__tests__/string-list.spec.tsx | 61 ++++++ .../form/default-form-control-resolver.tsx | 3 +- .../src/components/form/form-control.ts | 2 +- .../src/components/form/form.i18n.yml | 1 + .../bonito-ui/src/components/form/index.ts | 1 + .../src/components/form/string-list.tsx | 175 ++++++++++++++++++ .../bonito-ui/src/hooks/use-form-parameter.ts | 6 + packages/playground/src/demo-routes.tsx | 2 + .../src/demo/form/action-form-demo.tsx | 6 + .../demo/form/stringlist/stringlist-demo.tsx | 25 +++ .../playground/src/layout/demo-nav-menu.tsx | 5 + 14 files changed, 311 insertions(+), 18 deletions(-) create mode 100644 packages/bonito-ui/src/components/form/__tests__/string-list.spec.tsx create mode 100644 packages/bonito-ui/src/components/form/string-list.tsx create mode 100644 packages/playground/src/demo/form/stringlist/stringlist-demo.tsx diff --git a/packages/bonito-core/src/form/string-list-parameter.ts b/packages/bonito-core/src/form/string-list-parameter.ts index 45b67791cf..8c25def879 100644 --- a/packages/bonito-core/src/form/string-list-parameter.ts +++ b/packages/bonito-core/src/form/string-list-parameter.ts @@ -3,6 +3,10 @@ import { FormValues } from "./form"; import { AbstractParameter, ParameterName } from "./parameter"; import { ValidationStatus } from "./validation-status"; +export interface StringListValidationDetails { + [key: number]: string; +} + /** * A parameter with a value that is a list of strings */ @@ -10,28 +14,33 @@ export class StringListParameter< V extends FormValues, K extends ParameterName > extends AbstractParameter { - validateSync(): ValidationStatus { + validateSync() { let status = super.validateSync(); if (status.level === "ok") { status = this._validate(); } return status; } - private _validate(): ValidationStatus { + let hasError = false; + const vData: StringListValidationDetails = {}; if (this.value != null && Array.isArray(this.value)) { - for (const v of this.value) { + for (const [i, v] of this.value.entries()) { if (typeof v !== "string") { - // Found a non-string value - early out - return new ValidationStatus( - "error", - translate( - "bonito.core.form.validation.stringListValueError" - ) + hasError = true; + // Found a non-string value + vData[i] = translate( + "bonito.core.form.validation.stringValueError" ); } } } - return new ValidationStatus("ok"); + return hasError + ? new ValidationStatus( + "error", + translate("bonito.core.form.validation.stringListValueError"), + vData + ) + : new ValidationStatus("ok"); } } diff --git a/packages/bonito-core/src/form/validation-status.ts b/packages/bonito-core/src/form/validation-status.ts index 15f8cd6fc8..bdff798f58 100644 --- a/packages/bonito-core/src/form/validation-status.ts +++ b/packages/bonito-core/src/form/validation-status.ts @@ -2,12 +2,12 @@ * Represents the result of a given validation */ export class ValidationStatus { - level: "ok" | "warn" | "error" | "canceled"; - message?: string; forced?: boolean = false; - constructor(level: "ok" | "warn" | "error" | "canceled", message?: string) { - this.level = level; - this.message = message; - } + constructor( + public level: "ok" | "warn" | "error" | "canceled", + public message?: string, + // TODO: Make this a generic type + public details?: unknown + ) {} } diff --git a/packages/bonito-ui/i18n/resources.resjson b/packages/bonito-ui/i18n/resources.resjson index 0d135cdf99..1332e4d0e4 100644 --- a/packages/bonito-ui/i18n/resources.resjson +++ b/packages/bonito-ui/i18n/resources.resjson @@ -2,5 +2,6 @@ "bonito.ui.dataGrid.noResults": "No results found", "bonito.ui.form.buttons.apply": "Apply", "bonito.ui.form.buttons.discardChanges": "Discard changes", + "bonito.ui.form.delete": "Delete", "bonito.ui.form.showPassword": "Show password" } \ No newline at end of file diff --git a/packages/bonito-ui/src/components/form/__tests__/string-list.spec.tsx b/packages/bonito-ui/src/components/form/__tests__/string-list.spec.tsx new file mode 100644 index 0000000000..617c3657a1 --- /dev/null +++ b/packages/bonito-ui/src/components/form/__tests__/string-list.spec.tsx @@ -0,0 +1,61 @@ +import { StringListParameter } from "@azure/bonito-core/lib/form"; +import { initMockBrowserEnvironment } from "../../../environment"; +import { createParam } from "../../../form"; +import { StringList } from "../string-list"; +import { render, screen } from "@testing-library/react"; +import React from "react"; +import userEvent from "@testing-library/user-event"; +import { runAxe } from "../../../test-util"; + +describe("StringList form control", () => { + beforeEach(() => initMockBrowserEnvironment()); + + test("renders a list of strings", async () => { + const { container } = render( + <> + + + ); + const inputs = screen.getAllByRole("textbox"); + expect(inputs.length).toBe(3); + expect((inputs[0] as HTMLInputElement).value).toBe("foo"); + expect((inputs[1] as HTMLInputElement).value).toBe("bar"); + expect((inputs[2] as HTMLInputElement).value).toBe(""); + + const deleteButtons = screen.getAllByRole("button"); + expect(deleteButtons.length).toBe(2); + expect(await runAxe(container)).toHaveNoViolations(); + }); + + it("adds a new string when the last one is edited", async () => { + const onChange = jest.fn(); + render( + + ); + const inputs = screen.getAllByRole("textbox"); + const input = inputs[inputs.length - 1]; + input.focus(); + await userEvent.type(input, "baz"); + expect(onChange).toHaveBeenCalledWith(null, ["foo", "bar", "baz"]); + }); + + it("deletes a string when the delete button is clicked", async () => { + const onChange = jest.fn(); + render( + + ); + const deleteButton = screen.getAllByRole("button")[1]; + await userEvent.click(deleteButton); + expect(onChange).toHaveBeenCalledWith(null, ["foo"]); + }); +}); + +function createStringListParam() { + return createParam(StringListParameter, { + label: "String List", + value: ["foo", "bar"], + }); +} diff --git a/packages/bonito-ui/src/components/form/default-form-control-resolver.tsx b/packages/bonito-ui/src/components/form/default-form-control-resolver.tsx index 9dd98eca72..55a1c73dd2 100644 --- a/packages/bonito-ui/src/components/form/default-form-control-resolver.tsx +++ b/packages/bonito-ui/src/components/form/default-form-control-resolver.tsx @@ -21,6 +21,7 @@ import { FormControlResolver } from "./form-control-resolver"; import { LocationDropdown } from "./location-dropdown"; import { ResourceGroupDropdown } from "./resource-group-dropdown"; import { StorageAccountDropdown } from "./storage-account-dropdown"; +import { StringList } from "./string-list"; import { SubscriptionDropdown } from "./subscription-dropdown"; import { TextField } from "./text-field"; @@ -43,7 +44,7 @@ export class DefaultFormControlResolver implements FormControlResolver { ); } else if (param instanceof StringListParameter) { return ( - void; + onChange?: (event: React.FormEvent | null, value: V[K]) => void; } diff --git a/packages/bonito-ui/src/components/form/form.i18n.yml b/packages/bonito-ui/src/components/form/form.i18n.yml index 35bbf08b94..b5552fbd19 100644 --- a/packages/bonito-ui/src/components/form/form.i18n.yml +++ b/packages/bonito-ui/src/components/form/form.i18n.yml @@ -1,4 +1,5 @@ form: + delete: Delete buttons: apply: Apply discardChanges: Discard changes diff --git a/packages/bonito-ui/src/components/form/index.ts b/packages/bonito-ui/src/components/form/index.ts index 8b8f02a552..906dd6a5cb 100644 --- a/packages/bonito-ui/src/components/form/index.ts +++ b/packages/bonito-ui/src/components/form/index.ts @@ -13,3 +13,4 @@ export * from "./tab-selector"; export * from "./text-field"; export * from "./storage-account-dropdown"; export * from "./subscription-dropdown"; +export * from "./string-list"; diff --git a/packages/bonito-ui/src/components/form/string-list.tsx b/packages/bonito-ui/src/components/form/string-list.tsx new file mode 100644 index 0000000000..651e0a3d51 --- /dev/null +++ b/packages/bonito-ui/src/components/form/string-list.tsx @@ -0,0 +1,175 @@ +import { FormValues, ParameterName } from "@azure/bonito-core/lib/form"; +import { IconButton } from "@fluentui/react/lib/Button"; +import { Stack } from "@fluentui/react/lib/Stack"; +import { TextField } from "@fluentui/react/lib/TextField"; +import * as React from "react"; +import { useCallback, useMemo } from "react"; +import { useFormParameter, useUniqueId } from "../../hooks"; +import { FormControlProps } from "./form-control"; +import { useAppTheme } from "../../theme"; +import { translate } from "@azure/bonito-core"; + +export interface StringListValidationDetails { + [key: number]: string; +} + +export function StringList>( + props: FormControlProps +): JSX.Element { + const { className, style, param, onChange } = props; + + const id = useUniqueId("form-control", props.id); + const validationDetails = useFormParameter(param) + .validationDetails as StringListValidationDetails; + + const items = useMemo(() => { + const items: string[] = []; + if (param.value && Array.isArray(param.value)) { + for (const item of param.value) { + items.push(item); + } + } + // Add an empty item at the end + items.push(""); + return items; + }, [param.value]); + + const onItemChange = useCallback( + (index: number, value: string) => { + const newItems = [...items]; + if (index === items.length - 1) { + // Last item, add a new one + newItems.push(""); + } + newItems[index] = value; + param.value = newItems.slice(0, newItems.length - 1) as V[K]; + onChange?.(null, param.value); + }, + [items, param, onChange] + ); + + const onItemDelete = useCallback( + (index: number) => { + const newItems = [...items]; + newItems.splice(index, 1); + param.value = newItems.slice(0, newItems.length - 1) as V[K]; + onChange?.(null, param.value); + }, + [items, param, onChange] + ); + + return ( + + {items.map((item, index) => { + const errorMsg = validationDetails?.[index]; + return ( + + ); + })} + + ); +} + +interface StringListItemProps { + index: number; + value: string; + label?: string; + onChange: (index: number, value: string) => void; + onDelete: (index: number) => void; + placeholder?: string; + disableDelete?: boolean; + errorMsg?: string; +} + +function StringListItem(props: StringListItemProps) { + const { + index, + value, + label, + onChange, + onDelete, + disableDelete, + errorMsg, + placeholder, + } = props; + const styles = useStringListItemStyles(props); + const ariaLabel = `${label || ""} ${index + 1}`; + return ( + + + { + onChange(index, newValue || ""); + }} + /> + + { + onDelete(index); + }} + disabled={disableDelete} + /> + + ); +} + +function useStringListItemStyles(props: StringListItemProps) { + const theme = useAppTheme(); + const { disableDelete } = props; + + return React.useMemo(() => { + const itemPadding = { + padding: "11px 8px 11px 12px", + }; + return { + container: { + root: { + ":hover": { + backgroundColor: theme.palette.neutralLighter, + }, + }, + }, + input: { + root: { + ...itemPadding, + }, + field: { + height: "24px", + }, + fieldGroup: { + height: "24px", + "box-sizing": "content-box", + }, + }, + button: { + root: { + ...itemPadding, + visibility: disableDelete ? "hidden" : "initial", + }, + }, + }; + }, [theme.palette.neutralLighter, disableDelete]); +} diff --git a/packages/bonito-ui/src/hooks/use-form-parameter.ts b/packages/bonito-ui/src/hooks/use-form-parameter.ts index 4d1ffc0894..5c521f6352 100644 --- a/packages/bonito-ui/src/hooks/use-form-parameter.ts +++ b/packages/bonito-ui/src/hooks/use-form-parameter.ts @@ -78,6 +78,8 @@ export function useFormParameter< const [validationError, setValidationError] = useState< string | undefined >(); + const [validationDetails, setValidationDetails] = + useState(undefined); const [validationStatus, setValidationStatus] = useState< ValidationStatus | undefined >(); @@ -178,6 +180,7 @@ export function useFormParameter< setValidationStatus(snapshot.entryStatus[param.name]); if (dirty || param.validationStatus?.forced) { const msg = param.validationStatus?.message; + const details = param.validationStatus?.details; // Only set a visible validation error if the user has // interacted with the form control (ie: the parameter is // dirty) or validation is forced (usually the result of @@ -185,8 +188,10 @@ export function useFormParameter< // form) if (param.validationStatus?.level === "error") { setValidationError(msg); + setValidationDetails(details); } else { setValidationError(undefined); + setValidationDetails(undefined); } setDirty(true); } @@ -213,5 +218,6 @@ export function useFormParameter< loadingPromise, validationError, validationStatus, + validationDetails, }; } diff --git a/packages/playground/src/demo-routes.tsx b/packages/playground/src/demo-routes.tsx index 135d2a3100..542e91caaa 100644 --- a/packages/playground/src/demo-routes.tsx +++ b/packages/playground/src/demo-routes.tsx @@ -11,6 +11,7 @@ import { CheckboxDemo } from "./demo/form/checkbox/checkbox-demo"; import { NotificationDemo } from "./demo/form/notification-demo"; import { DataGridLoadMoreDemo } from "./demo/display/task-grid/task-data-grid"; import { TabSelectorDemo } from "./demo/form/tab-selector-demo"; +import { StringListDemo } from "./demo/form/stringlist/stringlist-demo"; export const DEMO_MAP = { default: () => , @@ -26,6 +27,7 @@ export const DEMO_MAP = { notification: () => , certificatedisplay: () => , dataGridLoadMore: () => , + stringlist: () => , }; export type DemoName = keyof typeof DEMO_MAP; diff --git a/packages/playground/src/demo/form/action-form-demo.tsx b/packages/playground/src/demo/form/action-form-demo.tsx index 5d43b33bbe..14b7046e76 100644 --- a/packages/playground/src/demo/form/action-form-demo.tsx +++ b/packages/playground/src/demo/form/action-form-demo.tsx @@ -2,6 +2,7 @@ import { AbstractAction, Action } from "@azure/bonito-core/lib/action"; import { Form, NumberParameter, + StringListParameter, StringParameter, ValidationStatus, } from "@azure/bonito-core/lib/form"; @@ -32,6 +33,7 @@ type CarFormValues = { model?: string; description?: string; milesPerCharge?: number; + addOns?: string[]; }; class CreateOrUpdateCarAction extends AbstractAction { @@ -114,6 +116,10 @@ class CreateOrUpdateCarAction extends AbstractAction { }, }); + carSection.param("addOns", StringListParameter, { + label: "Add-ons", + }); + form.item("summary", { dynamic: { hidden: (values) => values.make == null || values.model == null, diff --git a/packages/playground/src/demo/form/stringlist/stringlist-demo.tsx b/packages/playground/src/demo/form/stringlist/stringlist-demo.tsx new file mode 100644 index 0000000000..14bfcb452b --- /dev/null +++ b/packages/playground/src/demo/form/stringlist/stringlist-demo.tsx @@ -0,0 +1,25 @@ +import { StringListParameter } from "@azure/bonito-core/lib/form"; +import { createParam } from "@azure/bonito-ui"; +import { StringList } from "@azure/bonito-ui/lib/components/form"; +import React from "react"; +import { DemoComponentContainer } from "../../../layout/demo-component-container"; +import { DemoPane } from "../../../layout/demo-pane"; + +export const StringListDemo: React.FC = () => { + return ( + + + { + // eslint-disable-next-line no-console + console.log("String List change:", value); + }} + /> + + + ); +}; diff --git a/packages/playground/src/layout/demo-nav-menu.tsx b/packages/playground/src/layout/demo-nav-menu.tsx index 779fcf84bc..2dd8020daf 100644 --- a/packages/playground/src/layout/demo-nav-menu.tsx +++ b/packages/playground/src/layout/demo-nav-menu.tsx @@ -63,6 +63,11 @@ export const DemoNavMenu: React.FC = () => { name: "Notification", url: getDemoHash("notification"), }, + { + key: "StringList", + name: "StringList", + url: getDemoHash("stringlist"), + }, ], }, {