mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-03-22 13:47:19 +08:00
feat: show validation errors only after form submission attempt
- Remove always-visible red asterisk from required fields - Add showError prop to FormInput for conditional error display - Add hasAttemptedSubmit state to track form submission attempts - Show red border only when: showError=true AND required AND empty - Add aria-invalid attribute for accessibility - Prevent form submission when required fields are empty - Update unit tests for new validation behavior Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
@@ -71,18 +71,6 @@ describe("FormInput", () => {
|
||||
expect(input).toHaveAttribute("placeholder", "Enter text here");
|
||||
});
|
||||
|
||||
it("should show required asterisk when required is true", () => {
|
||||
render(<FormInput {...defaultProps} required />);
|
||||
|
||||
expect(screen.getByText("*")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("should not show required asterisk when required is false", () => {
|
||||
render(<FormInput {...defaultProps} required={false} />);
|
||||
|
||||
expect(screen.queryByText("*")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("should have aria-required attribute when required", () => {
|
||||
render(<FormInput {...defaultProps} required />);
|
||||
|
||||
@@ -120,4 +108,65 @@ describe("FormInput", () => {
|
||||
expect(label).toHaveAttribute("for", "form-input-test-input");
|
||||
expect(input).toHaveAttribute("id", "form-input-test-input");
|
||||
});
|
||||
|
||||
describe("error state", () => {
|
||||
it("should not show error border by default", () => {
|
||||
render(<FormInput {...defaultProps} required />);
|
||||
|
||||
const input = screen.getByTestId("form-input-test-input");
|
||||
expect(input).toHaveClass("border-[#242424]");
|
||||
expect(input).not.toHaveClass("border-red-500");
|
||||
});
|
||||
|
||||
it("should not show error border when showError is false", () => {
|
||||
render(<FormInput {...defaultProps} required showError={false} />);
|
||||
|
||||
const input = screen.getByTestId("form-input-test-input");
|
||||
expect(input).toHaveClass("border-[#242424]");
|
||||
expect(input).not.toHaveClass("border-red-500");
|
||||
});
|
||||
|
||||
it("should show error border when showError is true and field is empty and required", () => {
|
||||
render(<FormInput {...defaultProps} required showError />);
|
||||
|
||||
const input = screen.getByTestId("form-input-test-input");
|
||||
expect(input).toHaveClass("border-red-500");
|
||||
});
|
||||
|
||||
it("should not show error border when showError is true but field has value", () => {
|
||||
render(<FormInput {...defaultProps} required showError value="filled" />);
|
||||
|
||||
const input = screen.getByTestId("form-input-test-input");
|
||||
expect(input).not.toHaveClass("border-red-500");
|
||||
expect(input).toHaveClass("border-[#242424]");
|
||||
});
|
||||
|
||||
it("should not show error border when showError is true but field is not required", () => {
|
||||
render(<FormInput {...defaultProps} required={false} showError />);
|
||||
|
||||
const input = screen.getByTestId("form-input-test-input");
|
||||
expect(input).not.toHaveClass("border-red-500");
|
||||
});
|
||||
|
||||
it("should have aria-invalid true when showing error", () => {
|
||||
render(<FormInput {...defaultProps} required showError />);
|
||||
|
||||
const input = screen.getByTestId("form-input-test-input");
|
||||
expect(input).toHaveAttribute("aria-invalid", "true");
|
||||
});
|
||||
|
||||
it("should have aria-invalid false when not showing error", () => {
|
||||
render(<FormInput {...defaultProps} required showError={false} />);
|
||||
|
||||
const input = screen.getByTestId("form-input-test-input");
|
||||
expect(input).toHaveAttribute("aria-invalid", "false");
|
||||
});
|
||||
|
||||
it("should show error border on textarea when showError is true and empty", () => {
|
||||
render(<FormInput {...defaultProps} rows={4} required showError />);
|
||||
|
||||
const textarea = screen.getByTestId("form-input-test-input");
|
||||
expect(textarea).toHaveClass("border-red-500");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -164,4 +164,56 @@ describe("InformationRequestForm", () => {
|
||||
|
||||
expect(screen.getByText("ENTERPRISE$SELF_HOSTED_DESCRIPTION")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
describe("form validation", () => {
|
||||
it("should not show error state before form submission", () => {
|
||||
renderWithRouter();
|
||||
|
||||
const nameInput = screen.getByTestId("form-input-name");
|
||||
const companyInput = screen.getByTestId("form-input-company");
|
||||
const emailInput = screen.getByTestId("form-input-email");
|
||||
const messageInput = screen.getByTestId("form-input-message");
|
||||
|
||||
expect(nameInput).toHaveAttribute("aria-invalid", "false");
|
||||
expect(companyInput).toHaveAttribute("aria-invalid", "false");
|
||||
expect(emailInput).toHaveAttribute("aria-invalid", "false");
|
||||
expect(messageInput).toHaveAttribute("aria-invalid", "false");
|
||||
});
|
||||
|
||||
it("should not navigate when form is submitted with empty fields", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderWithRouter();
|
||||
|
||||
const submitButton = screen.getByRole("button", { name: "ENTERPRISE$FORM_SUBMIT" });
|
||||
await user.click(submitButton);
|
||||
|
||||
expect(mockNavigate).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("should navigate when form is submitted with all fields filled", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderWithRouter();
|
||||
|
||||
await user.type(screen.getByTestId("form-input-name"), "John Doe");
|
||||
await user.type(screen.getByTestId("form-input-company"), "Acme Inc");
|
||||
await user.type(screen.getByTestId("form-input-email"), "john@example.com");
|
||||
await user.type(screen.getByTestId("form-input-message"), "Hello world");
|
||||
|
||||
const submitButton = screen.getByRole("button", { name: "ENTERPRISE$FORM_SUBMIT" });
|
||||
await user.click(submitButton);
|
||||
|
||||
expect(mockNavigate).toHaveBeenCalledWith("/");
|
||||
});
|
||||
|
||||
it("should have valid aria-invalid state when field has value", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderWithRouter();
|
||||
|
||||
const nameInput = screen.getByTestId("form-input-name");
|
||||
await user.type(nameInput, "John Doe");
|
||||
|
||||
// Field with value should not be invalid
|
||||
expect(nameInput).toHaveAttribute("aria-invalid", "false");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -7,6 +7,7 @@ interface FormInputProps {
|
||||
type?: "text" | "email";
|
||||
rows?: number;
|
||||
required?: boolean;
|
||||
showError?: boolean;
|
||||
}
|
||||
|
||||
export function FormInput({
|
||||
@@ -18,10 +19,15 @@ export function FormInput({
|
||||
type = "text",
|
||||
rows,
|
||||
required = false,
|
||||
showError = false,
|
||||
}: FormInputProps) {
|
||||
const inputId = `form-input-${id}`;
|
||||
const inputClassName =
|
||||
"w-full min-h-10 rounded border border-[#242424] bg-[#050505] px-3 py-2 text-sm leading-5 text-white placeholder:text-[#8C8C8C] placeholder:leading-5 focus:border-white focus:outline-none transition-colors";
|
||||
const hasError = showError && required && !value.trim();
|
||||
const inputClassName = `w-full min-h-10 rounded border bg-[#050505] px-3 py-2 text-sm leading-5 text-white placeholder:text-[#8C8C8C] placeholder:leading-5 focus:outline-none transition-colors ${
|
||||
hasError
|
||||
? "border-red-500 focus:border-red-500"
|
||||
: "border-[#242424] focus:border-white"
|
||||
}`;
|
||||
|
||||
return (
|
||||
<div className="flex flex-col gap-1.5 w-full">
|
||||
@@ -30,7 +36,6 @@ export function FormInput({
|
||||
className="text-sm font-medium leading-5 text-[#FAFAFA] cursor-pointer"
|
||||
>
|
||||
{label}
|
||||
{required && <span className="text-red-500 ml-1" aria-hidden="true">*</span>}
|
||||
</label>
|
||||
{rows ? (
|
||||
<textarea
|
||||
@@ -42,6 +47,7 @@ export function FormInput({
|
||||
rows={rows}
|
||||
required={required}
|
||||
aria-required={required}
|
||||
aria-invalid={hasError}
|
||||
aria-label={label}
|
||||
className={`${inputClassName} h-auto resize-none`}
|
||||
/>
|
||||
@@ -55,6 +61,7 @@ export function FormInput({
|
||||
placeholder={placeholder}
|
||||
required={required}
|
||||
aria-required={required}
|
||||
aria-invalid={hasError}
|
||||
aria-label={label}
|
||||
className={inputClassName}
|
||||
/>
|
||||
|
||||
@@ -28,9 +28,23 @@ export function InformationRequestForm({
|
||||
email: "",
|
||||
message: "",
|
||||
});
|
||||
const [hasAttemptedSubmit, setHasAttemptedSubmit] = useState(false);
|
||||
|
||||
const handleSubmit = (e: React.FormEvent) => {
|
||||
e.preventDefault();
|
||||
setHasAttemptedSubmit(true);
|
||||
|
||||
// Check if all required fields are filled
|
||||
const isValid =
|
||||
formData.name.trim() &&
|
||||
formData.company.trim() &&
|
||||
formData.email.trim() &&
|
||||
formData.message.trim();
|
||||
|
||||
if (!isValid) {
|
||||
return;
|
||||
}
|
||||
|
||||
// TODO: Implement form submission
|
||||
navigate("/");
|
||||
};
|
||||
@@ -84,6 +98,7 @@ export function InformationRequestForm({
|
||||
value={formData.name}
|
||||
placeholder={t(I18nKey.ENTERPRISE$FORM_NAME_PLACEHOLDER)}
|
||||
required
|
||||
showError={hasAttemptedSubmit}
|
||||
onChange={(value) =>
|
||||
setFormData((prev) => ({ ...prev, name: value }))
|
||||
}
|
||||
@@ -95,6 +110,7 @@ export function InformationRequestForm({
|
||||
value={formData.company}
|
||||
placeholder={t(I18nKey.ENTERPRISE$FORM_COMPANY_PLACEHOLDER)}
|
||||
required
|
||||
showError={hasAttemptedSubmit}
|
||||
onChange={(value) =>
|
||||
setFormData((prev) => ({ ...prev, company: value }))
|
||||
}
|
||||
@@ -107,6 +123,7 @@ export function InformationRequestForm({
|
||||
value={formData.email}
|
||||
placeholder={t(I18nKey.ENTERPRISE$FORM_EMAIL_PLACEHOLDER)}
|
||||
required
|
||||
showError={hasAttemptedSubmit}
|
||||
onChange={(value) =>
|
||||
setFormData((prev) => ({ ...prev, email: value }))
|
||||
}
|
||||
@@ -119,6 +136,7 @@ export function InformationRequestForm({
|
||||
placeholder={messagePlaceholder}
|
||||
rows={4}
|
||||
required
|
||||
showError={hasAttemptedSubmit}
|
||||
onChange={(value) =>
|
||||
setFormData((prev) => ({ ...prev, message: value }))
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user