From 31e079d591ad7df2de3869e0435688ebd8c6af4f Mon Sep 17 00:00:00 2001
From: gerjanvangeest <Gerjan.van.Geest@ing.com>
Date: Tue, 30 Jan 2024 10:19:09 +0100
Subject: [PATCH] feat(form): set focus to the first erroneous form element on
 submit

---
 .changeset/plenty-turtles-remember.md         |  7 ++
 docs/components/form/overview.md              | 27 +++++---
 docs/components/form/use-cases.md             | 24 +++----
 docs/fundamentals/systems/form/use-cases.md   | 21 ++++--
 packages/ui/components/form/src/LionForm.js   | 24 +++++++
 .../ui/components/form/test/lion-form.test.js | 68 ++++++++++++++++++-
 6 files changed, 142 insertions(+), 29 deletions(-)
 create mode 100644 .changeset/plenty-turtles-remember.md

diff --git a/.changeset/plenty-turtles-remember.md b/.changeset/plenty-turtles-remember.md
new file mode 100644
index 000000000..e6b58502b
--- /dev/null
+++ b/.changeset/plenty-turtles-remember.md
@@ -0,0 +1,7 @@
+---
+'@lion/ui': minor
+---
+
+BREAKING CHANGE:
+
+[form] set focus to the first erroneous form element on submit
diff --git a/docs/components/form/overview.md b/docs/components/form/overview.md
index 02225f2ae..6d08d6860 100644
--- a/docs/components/form/overview.md
+++ b/docs/components/form/overview.md
@@ -11,14 +11,25 @@ A web component that enhances the functionality of the native `form` component.
 It is designed to interact with (instances of) the [form controls](../../fundamentals/systems/form/overview.md).
 
 ```js preview-story
-export const main = () => html`
-  <lion-form>
-    <form>
-      <lion-input name="firstName" label="First Name" .modelValue=${'Foo'}></lion-input>
-      <lion-input name="lastName" label="Last Name" .modelValue=${'Bar'}></lion-input>
-    </form>
-  </lion-form>
-`;
+export const main = () => {
+  const submitHandler = ev => {
+    const formData = ev.target.serializedValue;
+    console.log('formData', formData);
+    fetch('/api/foo/', {
+      method: 'POST',
+      body: JSON.stringify(formData),
+    });
+  };
+  return html`
+    <lion-form @submit=${submitHandler}>
+      <form @submit=${ev => ev.preventDefault()}>
+        <lion-input name="firstName" label="First Name" .modelValue=${'Foo'}></lion-input>
+        <lion-input name="lastName" label="Last Name" .modelValue=${'Bar'}></lion-input>
+        <button>Submit</button>
+      </form>
+    </lion-form>
+  `;
+};
 ```
 
 ## Features
diff --git a/docs/components/form/use-cases.md b/docs/components/form/use-cases.md
index 5fd0f74ee..d8f7f895f 100644
--- a/docs/components/form/use-cases.md
+++ b/docs/components/form/use-cases.md
@@ -10,32 +10,28 @@ import '@lion/ui/define/lion-form.js';
 
 ## Submit & Reset
 
-To submit a form, use a regular button (or `LionButtonSubmit`) somewhere inside the native `<form>`.
+To submit a form, use a regular `<button>` (or `<lion-button-submit>`) somewhere inside the native `<form>`.
 
-Then, add a `submit` handler on the `lion-form`.
+Then, add a `submit` handler on the `<lion-form>`.
 
 You can use this event to do your own (pre-)submit logic, like getting the serialized form data and sending it to a backend API.
 
 Another example is checking if the form has errors, and focusing the first field with an error.
 
-To fire a submit from JavaScript, select the `lion-form` element and call `.submit()`.
+To fire a submit from JavaScript, select the `<lion-form>` element and call `.submit()`.
 
 ```js preview-story
 export const formSubmit = () => {
   loadDefaultFeedbackMessages();
   const submitHandler = ev => {
-    if (ev.target.hasFeedbackFor.includes('error')) {
-      const firstFormElWithError = ev.target.formElements.find(el =>
-        el.hasFeedbackFor.includes('error'),
-      );
-      firstFormElWithError.focus();
-      return;
-    }
     const formData = ev.target.serializedValue;
-    fetch('/api/foo/', {
-      method: 'POST',
-      body: JSON.stringify(formData),
-    });
+    console.log('formData', formData);
+    if (!ev.target.hasFeedbackFor?.includes('error')) {
+      fetch('/api/foo/', {
+        method: 'POST',
+        body: JSON.stringify(formData),
+      });
+    }
   };
   const submitViaJS = ev => {
     // Call submit on the lion-form element, in your own code you should use
diff --git a/docs/fundamentals/systems/form/use-cases.md b/docs/fundamentals/systems/form/use-cases.md
index c9dc04af3..1ab06851e 100644
--- a/docs/fundamentals/systems/form/use-cases.md
+++ b/docs/fundamentals/systems/form/use-cases.md
@@ -30,6 +30,8 @@ import '@lion/ui/define/lion-select.js';
 import '@lion/ui/define/lion-select-rich.js';
 import '@lion/ui/define/lion-switch.js';
 import '@lion/ui/define/lion-textarea.js';
+import '@lion/ui/define/lion-button-submit.js';
+import '@lion/ui/define/lion-button-reset.js';
 import { MinLength, Required } from '@lion/ui/form-core.js';
 import { loadDefaultFeedbackMessages } from '@lion/ui/validate-messages.js';
 ```
@@ -39,7 +41,6 @@ import { loadDefaultFeedbackMessages } from '@lion/ui/validate-messages.js';
 ```js preview-story
 export const main = () => {
   loadDefaultFeedbackMessages();
-  Required.getMessage = () => 'Please enter a value';
   return html`
     <lion-form>
       <form>
@@ -47,11 +48,13 @@ export const main = () => {
           <lion-input
             name="firstName"
             label="First Name"
+            .fieldName="${'first name'}"
             .validators="${[new Required()]}"
           ></lion-input>
           <lion-input
             name="lastName"
             label="Last Name"
+            .fieldName="${'last name'}"
             .validators="${[new Required()]}"
           ></lion-input>
         </lion-fieldset>
@@ -70,6 +73,7 @@ export const main = () => {
         <lion-textarea
           name="bio"
           label="Biography"
+          .fieldName="${'value'}"
           .validators="${[new Required(), new MinLength(10)]}"
           help-text="Please enter at least 10 characters"
         ></lion-textarea>
@@ -82,6 +86,7 @@ export const main = () => {
           label="What do you like?"
           name="checkers"
           .validators="${[new Required()]}"
+          .fieldName="${'value'}"
         >
           <lion-checkbox .choiceValue=${'foo'} label="I like foo"></lion-checkbox>
           <lion-checkbox .choiceValue=${'bar'} label="I like bar"></lion-checkbox>
@@ -90,6 +95,7 @@ export const main = () => {
         <lion-radio-group
           name="dinosaurs"
           label="Favorite dinosaur"
+          .fieldName="${'dinosaur'}"
           .validators="${[new Required()]}"
         >
           <lion-radio .choiceValue=${'allosaurus'} label="allosaurus"></lion-radio>
@@ -136,18 +142,23 @@ export const main = () => {
           label="Input range"
         ></lion-input-range>
         <lion-checkbox-group
-          .multipleChoice="${false}"
           name="terms"
-          .validators="${[new Required()]}"
+          .validators="${[
+            new Required('', {
+              getMessage: () => `Please accept our terms.`,
+            }),
+          ]}"
         >
-          <lion-checkbox label="I blindly accept all terms and conditions"></lion-checkbox>
+          <lion-checkbox
+            .choiceValue="${'true'}"
+            label="I blindly accept all terms and conditions"
+          ></lion-checkbox>
         </lion-checkbox-group>
         <lion-switch name="notifications" label="Notifications"></lion-switch>
         <lion-input-stepper max="5" min="0" name="rsvp">
           <label slot="label">RSVP</label>
           <div slot="help-text">Max. 5 guests</div>
         </lion-input-stepper>
-        <lion-textarea name="comments" label="Comments"></lion-textarea>
         <div class="buttons">
           <lion-button-submit>Submit</lion-button-submit>
           <lion-button-reset
diff --git a/packages/ui/components/form/src/LionForm.js b/packages/ui/components/form/src/LionForm.js
index f3e5f145e..fc7f9dca0 100644
--- a/packages/ui/components/form/src/LionForm.js
+++ b/packages/ui/components/form/src/LionForm.js
@@ -1,5 +1,9 @@
 import { LionFieldset } from '@lion/ui/fieldset.js';
 
+/**
+ * @typedef {import('../../form-core/types/registration/FormRegistrarMixinTypes.js').FormRegistrarHost} FormRegistrarHost
+ */
+
 const throwFormNodeError = () => {
   throw new Error(
     'No form node found. Did you put a <form> element inside your custom-form element?',
@@ -57,6 +61,10 @@ export class LionForm extends LionFieldset {
     ev.stopPropagation();
     this.submitGroup();
     this.dispatchEvent(new Event('submit', { bubbles: true }));
+
+    if (this.hasFeedbackFor?.includes('error')) {
+      this._setFocusOnFirstErroneousFormElement(this);
+    }
   }
 
   reset() {
@@ -78,6 +86,22 @@ export class LionForm extends LionFieldset {
     this.dispatchEvent(new Event('reset', { bubbles: true }));
   }
 
+  /**
+   * @param {FormRegistrarHost} element
+   * @protected
+   */
+  _setFocusOnFirstErroneousFormElement(element) {
+    const firstFormElWithError =
+      element.formElements.find(child => child.hasFeedbackFor.includes('error')) ||
+      element.formElements[0];
+
+    if (firstFormElWithError.formElements?.length > 0) {
+      this._setFocusOnFirstErroneousFormElement(firstFormElWithError);
+    } else {
+      firstFormElWithError._focusableNode.focus();
+    }
+  }
+
   /** @private */
   __registerEventsForLionForm() {
     this._formNode.addEventListener('submit', this._submit);
diff --git a/packages/ui/components/form/test/lion-form.test.js b/packages/ui/components/form/test/lion-form.test.js
index ba1a2a024..9b8420d92 100644
--- a/packages/ui/components/form/test/lion-form.test.js
+++ b/packages/ui/components/form/test/lion-form.test.js
@@ -1,6 +1,6 @@
 import { LionFieldset } from '@lion/ui/fieldset.js';
 import '@lion/ui/define/lion-fieldset.js';
-import { LionField } from '@lion/ui/form-core.js';
+import { LionField, Required } from '@lion/ui/form-core.js';
 import '@lion/ui/define/lion-field.js';
 import '@lion/ui/define/lion-validation-feedback.js';
 
@@ -191,7 +191,7 @@ describe('<lion-form>', () => {
     const el = await fixture(html`
       <lion-form>
         <form>
-          <button type="reset">submit</button>
+          <button type="reset">reset</button>
         </form>
       </lion-form>
     `);
@@ -202,4 +202,68 @@ describe('<lion-form>', () => {
     expect(dispatchSpy.args[0][0].type).to.equal('reset');
     expect(internalHandlerSpy).to.be.calledBefore(dispatchSpy);
   });
+
+  it('sets focus on submit to the first erroneous form element', async () => {
+    const el = await fixture(html`
+      <lion-form>
+        <form>
+          <${childTag} name="firstName" .modelValue=${'Foo'} .validators=${[
+      new Required(),
+    ]}></${childTag}>
+          <${childTag} name="lastName" .validators=${[new Required()]}></${childTag}>
+          <button type="submit">submit</button>
+        </form>
+      </lion-form>
+    `);
+    const button = /** @type {HTMLButtonElement} */ (el.querySelector('button'));
+    const dispatchSpy = spy(el, 'dispatchEvent');
+    button.click();
+    expect(dispatchSpy.args[0][0].type).to.equal('submit');
+    // @ts-ignore [allow-protected] in test
+    expect(document.activeElement).to.equal(el.formElements[1]._inputNode);
+  });
+
+  it('sets focus on submit to the first erroneous form element with a fieldset', async () => {
+    const el = await fixture(html`
+      <lion-form>
+        <form>
+          <lion-fieldset name="name">
+            <${childTag} name="firstName" .modelValue=${'Foo'} .validators=${[
+      new Required(),
+    ]}></${childTag}>
+            <${childTag} name="lastName" .validators=${[new Required()]}></${childTag}>
+          </lion-fieldset>
+          <button type="submit">submit</button>
+        </form>
+      </lion-form>
+    `);
+    const button = /** @type {HTMLButtonElement} */ (el.querySelector('button'));
+    const dispatchSpy = spy(el, 'dispatchEvent');
+    button.click();
+    expect(dispatchSpy.args[0][0].type).to.equal('submit');
+    const fieldset = el.formElements[0];
+    // @ts-ignore [allow-protected] in test
+    expect(document.activeElement).to.equal(fieldset.formElements[1]._inputNode);
+  });
+
+  it('sets focus on submit to the first form element within a erroneous fieldset', async () => {
+    const el = await fixture(html`
+      <lion-form>
+        <form>
+          <lion-fieldset name="name" .validators=${[new Required()]}>
+            <${childTag} name="firstName"></${childTag}>
+            <${childTag} name="lastName"></${childTag}>
+          </lion-fieldset>
+          <button type="submit">submit</button>
+        </form>
+      </lion-form>
+    `);
+    const button = /** @type {HTMLButtonElement} */ (el.querySelector('button'));
+    const dispatchSpy = spy(el, 'dispatchEvent');
+    button.click();
+    expect(dispatchSpy.args[0][0].type).to.equal('submit');
+    const fieldset = el.formElements[0];
+    // @ts-ignore [allow-protected] in test
+    expect(document.activeElement).to.equal(fieldset.formElements[0]._inputNode);
+  });
 });