From a539058253ca5763047394680596e1510fdd2210 Mon Sep 17 00:00:00 2001
From: Cory LaViska <cory@abeautifulsite.net>
Date: Mon, 6 Feb 2023 17:18:01 -0500
Subject: [PATCH] validate even with novalidate; fixes #1164

---
 docs/resources/changelog.md                   |  1 +
 src/components/checkbox/checkbox.test.ts      | 12 ++++++++++
 src/components/input/input.test.ts            | 12 ++++++++++
 .../radio-group/radio-group.test.ts           | 19 +++++++++++++++
 src/components/range/range.test.ts            | 13 +++++++++++
 src/components/select/select.test.ts          | 20 ++++++++++++++++
 src/components/switch/switch.test.ts          | 12 ++++++++++
 src/components/textarea/textarea.test.ts      | 12 ++++++++++
 src/internal/form.ts                          | 23 +++++--------------
 9 files changed, 107 insertions(+), 17 deletions(-)

diff --git a/docs/resources/changelog.md b/docs/resources/changelog.md
index f3dcae645..6c9cb3430 100644
--- a/docs/resources/changelog.md
+++ b/docs/resources/changelog.md
@@ -15,6 +15,7 @@ New versions of Shoelace are released as-needed and generally occur when a criti
 - Fixed a bug in the template for `<sl-select>` that caused the `form-control-help-text` part to not be in the same location as other form controls [#1178](https://github.com/shoelace-style/shoelace/issues/1178)
 - Fixed a bug in `<sl-checkbox>` and `<sl-switch>` that caused the browser to scroll incorrectly when focusing on a control in a container with overflow [#1169](https://github.com/shoelace-style/shoelace/issues/1169)
 - Fixed a bug in `<sl-menu-item>` that caused the `click` event to be emitted when the item was disabled [#1113](https://github.com/shoelace-style/shoelace/issues/1113)
+- Fixed a bug in form controls that erroneously prevented validation states from being set when `novalidate` was used on the containing form [#1164](https://github.com/shoelace-style/shoelace/issues/1164)
 - Improved the behavior of `<sl-dropdown>` in Safari so keyboard interaction works the same as in other browsers [#1177](https://github.com/shoelace-style/shoelace/issues/1177)
 - Improved the [icons](/components/icon) page so it's not as sluggish in Safari [#1122](https://github.com/shoelace-style/shoelace/issues/1122)
 
diff --git a/src/components/checkbox/checkbox.test.ts b/src/components/checkbox/checkbox.test.ts
index 43a1138d3..98bb4aff8 100644
--- a/src/components/checkbox/checkbox.test.ts
+++ b/src/components/checkbox/checkbox.test.ts
@@ -199,6 +199,18 @@ describe('<sl-checkbox>', () => {
 
       expect(formData.get('a')).to.equal('1');
     });
+
+    it('should receive validation attributes ("states") even when novalidate is used on the parent form', async () => {
+      const el = await fixture<HTMLFormElement>(html` <form novalidate><sl-checkbox required></sl-checkbox></form> `);
+      const checkbox = el.querySelector<SlCheckbox>('sl-checkbox')!;
+
+      expect(checkbox.hasAttribute('data-required')).to.be.true;
+      expect(checkbox.hasAttribute('data-optional')).to.be.false;
+      expect(checkbox.hasAttribute('data-invalid')).to.be.true;
+      expect(checkbox.hasAttribute('data-valid')).to.be.false;
+      expect(checkbox.hasAttribute('data-user-invalid')).to.be.false;
+      expect(checkbox.hasAttribute('data-user-valid')).to.be.false;
+    });
   });
 
   describe('when resetting a form', () => {
diff --git a/src/components/input/input.test.ts b/src/components/input/input.test.ts
index f5a7637b2..50792fbb5 100644
--- a/src/components/input/input.test.ts
+++ b/src/components/input/input.test.ts
@@ -155,6 +155,18 @@ describe('<sl-input>', () => {
       expect(el.hasAttribute('data-user-invalid')).to.be.true;
       expect(el.hasAttribute('data-user-valid')).to.be.false;
     });
+
+    it('should receive validation attributes ("states") even when novalidate is used on the parent form', async () => {
+      const el = await fixture<HTMLFormElement>(html` <form novalidate><sl-input required></sl-input></form> `);
+      const input = el.querySelector<SlInput>('sl-input')!;
+
+      expect(input.hasAttribute('data-required')).to.be.true;
+      expect(input.hasAttribute('data-optional')).to.be.false;
+      expect(input.hasAttribute('data-invalid')).to.be.true;
+      expect(input.hasAttribute('data-valid')).to.be.false;
+      expect(input.hasAttribute('data-user-invalid')).to.be.false;
+      expect(input.hasAttribute('data-user-valid')).to.be.false;
+    });
   });
 
   describe('when submitting a form', () => {
diff --git a/src/components/radio-group/radio-group.test.ts b/src/components/radio-group/radio-group.test.ts
index 6d0692170..cf5069d70 100644
--- a/src/components/radio-group/radio-group.test.ts
+++ b/src/components/radio-group/radio-group.test.ts
@@ -130,6 +130,25 @@ describe('<sl-radio-group>', () => {
       expect(radioGroup.hasAttribute('data-user-invalid')).to.be.true;
       expect(radioGroup.hasAttribute('data-user-valid')).to.be.false;
     });
+
+    it('should receive validation attributes ("states") even when novalidate is used on the parent form', async () => {
+      const el = await fixture<HTMLFormElement>(html`
+        <form novalidate>
+          <sl-radio-group required>
+            <sl-radio value="1"></sl-radio>
+            <sl-radio value="2"></sl-radio>
+          </sl-radio-group>
+        </form>
+      `);
+      const radioGroup = el.querySelector<SlRadioGroup>('sl-radio-group')!;
+
+      expect(radioGroup.hasAttribute('data-required')).to.be.true;
+      expect(radioGroup.hasAttribute('data-optional')).to.be.false;
+      expect(radioGroup.hasAttribute('data-invalid')).to.be.true;
+      expect(radioGroup.hasAttribute('data-valid')).to.be.false;
+      expect(radioGroup.hasAttribute('data-user-invalid')).to.be.false;
+      expect(radioGroup.hasAttribute('data-user-valid')).to.be.false;
+    });
   });
 
   it('should show a constraint validation error when setCustomValidity() is called', async () => {
diff --git a/src/components/range/range.test.ts b/src/components/range/range.test.ts
index dbe9537f5..0d9db3b25 100644
--- a/src/components/range/range.test.ts
+++ b/src/components/range/range.test.ts
@@ -169,6 +169,19 @@ describe('<sl-range>', () => {
       expect(range.hasAttribute('data-user-valid')).to.be.false;
     });
 
+    it('should receive validation attributes ("states") even when novalidate is used on the parent form', async () => {
+      const el = await fixture<HTMLFormElement>(html` <form novalidate><sl-range></sl-range></form> `);
+      const range = el.querySelector<SlRange>('sl-range')!;
+
+      range.setCustomValidity('Invalid value');
+      await range.updateComplete;
+
+      expect(range.hasAttribute('data-invalid')).to.be.true;
+      expect(range.hasAttribute('data-valid')).to.be.false;
+      expect(range.hasAttribute('data-user-invalid')).to.be.false;
+      expect(range.hasAttribute('data-user-valid')).to.be.false;
+    });
+
     it('should be present in form data when using the form attribute and located outside of a <form>', async () => {
       const el = await fixture<HTMLFormElement>(html`
         <div>
diff --git a/src/components/select/select.test.ts b/src/components/select/select.test.ts
index 09ca9948a..7a119f9a3 100644
--- a/src/components/select/select.test.ts
+++ b/src/components/select/select.test.ts
@@ -294,6 +294,26 @@ describe('<sl-select>', () => {
       expect(el.hasAttribute('data-user-invalid')).to.be.true;
       expect(el.hasAttribute('data-user-valid')).to.be.false;
     });
+
+    it('should receive validation attributes ("states") even when novalidate is used on the parent form', async () => {
+      const el = await fixture<HTMLFormElement>(html`
+        <form novalidate>
+          <sl-select required>
+            <sl-option value="option-1">Option 1</sl-option>
+            <sl-option value="option-2">Option 2</sl-option>
+            <sl-option value="option-3">Option 3</sl-option>
+          </sl-select>
+        </form>
+      `);
+      const select = el.querySelector<SlSelect>('sl-select')!;
+
+      expect(select.hasAttribute('data-required')).to.be.true;
+      expect(select.hasAttribute('data-optional')).to.be.false;
+      expect(select.hasAttribute('data-invalid')).to.be.true;
+      expect(select.hasAttribute('data-valid')).to.be.false;
+      expect(select.hasAttribute('data-user-invalid')).to.be.false;
+      expect(select.hasAttribute('data-user-valid')).to.be.false;
+    });
   });
 
   describe('when submitting a form', () => {
diff --git a/src/components/switch/switch.test.ts b/src/components/switch/switch.test.ts
index cc2902c3a..8276b7e75 100644
--- a/src/components/switch/switch.test.ts
+++ b/src/components/switch/switch.test.ts
@@ -217,6 +217,18 @@ describe('<sl-switch>', () => {
 
       expect(formData.get('a')).to.equal('1');
     });
+
+    it('should receive validation attributes ("states") even when novalidate is used on the parent form', async () => {
+      const el = await fixture<HTMLFormElement>(html` <form novalidate><sl-switch required></sl-switch></form> `);
+      const slSwitch = el.querySelector<SlSwitch>('sl-switch')!;
+
+      expect(slSwitch.hasAttribute('data-required')).to.be.true;
+      expect(slSwitch.hasAttribute('data-optional')).to.be.false;
+      expect(slSwitch.hasAttribute('data-invalid')).to.be.true;
+      expect(slSwitch.hasAttribute('data-valid')).to.be.false;
+      expect(slSwitch.hasAttribute('data-user-invalid')).to.be.false;
+      expect(slSwitch.hasAttribute('data-user-valid')).to.be.false;
+    });
   });
 
   describe('when resetting a form', () => {
diff --git a/src/components/textarea/textarea.test.ts b/src/components/textarea/textarea.test.ts
index 711f24801..caf1314be 100644
--- a/src/components/textarea/textarea.test.ts
+++ b/src/components/textarea/textarea.test.ts
@@ -171,6 +171,18 @@ describe('<sl-textarea>', () => {
       expect(el.hasAttribute('data-user-invalid')).to.be.true;
       expect(el.hasAttribute('data-user-valid')).to.be.false;
     });
+
+    it('should receive validation attributes ("states") even when novalidate is used on the parent form', async () => {
+      const el = await fixture<HTMLFormElement>(html` <form novalidate><sl-textarea required></sl-textarea></form> `);
+      const textarea = el.querySelector<SlTextarea>('sl-textarea')!;
+
+      expect(textarea.hasAttribute('data-required')).to.be.true;
+      expect(textarea.hasAttribute('data-optional')).to.be.false;
+      expect(textarea.hasAttribute('data-invalid')).to.be.true;
+      expect(textarea.hasAttribute('data-valid')).to.be.false;
+      expect(textarea.hasAttribute('data-user-invalid')).to.be.false;
+      expect(textarea.hasAttribute('data-user-valid')).to.be.false;
+    });
   });
 
   describe('when submitting a form', () => {
diff --git a/src/internal/form.ts b/src/internal/form.ts
index f10772b05..527341b96 100644
--- a/src/internal/form.ts
+++ b/src/internal/form.ts
@@ -294,23 +294,12 @@ export class FormControlController implements ReactiveController {
     //
     // See this RFC for more details: https://github.com/shoelace-style/shoelace/issues/1011
     //
-    if (this.form?.noValidate) {
-      // Form validation is disabled, remove the attributes
-      host.removeAttribute('data-required');
-      host.removeAttribute('data-optional');
-      host.removeAttribute('data-invalid');
-      host.removeAttribute('data-valid');
-      host.removeAttribute('data-user-invalid');
-      host.removeAttribute('data-user-valid');
-    } else {
-      // Form validation is enabled, set the attributes
-      host.toggleAttribute('data-required', required);
-      host.toggleAttribute('data-optional', !required);
-      host.toggleAttribute('data-invalid', !isValid);
-      host.toggleAttribute('data-valid', isValid);
-      host.toggleAttribute('data-user-invalid', !isValid && hasInteracted);
-      host.toggleAttribute('data-user-valid', isValid && hasInteracted);
-    }
+    host.toggleAttribute('data-required', required);
+    host.toggleAttribute('data-optional', !required);
+    host.toggleAttribute('data-invalid', !isValid);
+    host.toggleAttribute('data-valid', isValid);
+    host.toggleAttribute('data-user-invalid', !isValid && hasInteracted);
+    host.toggleAttribute('data-user-valid', isValid && hasInteracted);
   }
 
   /**