fixes #1823 (#1826)

docs/pages/resources/changelog.md

@@ -14,15 +14,16 @@ New versions of Shoelace are released as-needed and generally occur when a criti
 
 ## Next
 
+- Added the `hover-bridge` feature to `<sl-popup>` to support better tooltip accessibility [#1734]
+- Added the `loading` attribute and the `spinner` and `spinner__base` part to `<sl-menu-item>` [#1700]
 - Fixed files that did not have `.js` extensions. [#1770]
 - Fixed `<sl-dialog>` not accounting for elements with hidden dialog controls like `<video>` [#1755]
-- Added the `loading` attribute and the `spinner` and `spinner__base` part to `<sl-menu-item>` [#1700]
 - Fixed focus trapping not scrolling elements into view. [#1750]
 - Fixed more performance issues with focus trapping performance. [#1750]
-- Added the `hover-bridge` feature to `<sl-popup>` to support better tooltip accessibility [#1734]
 - Fixed a bug in `<sl-input>` and `<sl-textarea>` that made it work differently from `<input>` and `<textarea>` when using defaults [#1746]
 - Fixed a bug in `<sl-select>` that prevented it from closing when tabbing to another select inside a shadow root [#1763]
 - Fixed a bug in `<sl-spinner>` that caused the animation to appear strange in certain circumstances [#1787]
+- Fixed a bug that caused form controls to submit even after they were removed from the DOM [#1823]
 - Fixed a bug that caused empty `<sl-radio-group>` elements to log an error in the console [#1795]
 - Fixed a bug that caused modal scroll locking to conflict with the `scrollbar-gutter` property [#1805]
 - Fixed a bug in `<sl-option>` that caused slotted content to show up when calling `getTextLabel()` [#1730]

src/internal/form.test.ts

@@ -1,6 +1,7 @@
 import '../../../dist/shoelace.js';
+import sinon from 'sinon';
 
-import { expect, fixture, html } from '@open-wc/testing';
+import { expect, fixture, html, waitUntil } from '@open-wc/testing';
 
 // Reproduction of this issue: https://github.com/shoelace-style/shoelace/issues/1703
 it('Should still run form validations if an element is removed', async () => {
@@ -19,3 +20,59 @@ it('Should still run form validations if an element is removed', async () => {
   expect(form.checkValidity()).to.equal(false);
   expect(form.reportValidity()).to.equal(false);
 });
+
+it('should submit the correct form values', async () => {
+  const form = await fixture<HTMLFormElement>(html`
+    <form>
+      <sl-input name="a" value="1"></sl-input>
+      <sl-input name="b" value="2"></sl-input>
+      <sl-input name="c" value="3"></sl-input>
+      <sl-button type="submit">Submit</sl-button>
+    </form>
+  `);
+
+  const button = form.querySelector('sl-button')!;
+  const submitHandler = sinon.spy((event: SubmitEvent) => {
+    formData = new FormData(form);
+    event.preventDefault();
+  });
+  let formData: FormData;
+
+  form.addEventListener('submit', submitHandler);
+  button.click();
+
+  await waitUntil(() => submitHandler.calledOnce);
+
+  expect(formData!.get('a')).to.equal('1');
+  expect(formData!.get('b')).to.equal('2');
+  expect(formData!.get('c')).to.equal('3');
+});
+
+it('should submit the correct form values when form controls are removed from the DOM', async () => {
+  const form = await fixture<HTMLFormElement>(html`
+    <form>
+      <sl-input name="a" value="1"></sl-input>
+      <sl-input name="b" value="2"></sl-input>
+      <sl-input name="c" value="3"></sl-input>
+      <sl-button type="submit">Submit</sl-button>
+    </form>
+  `);
+
+  const button = form.querySelector('sl-button')!;
+  const submitHandler = sinon.spy((event: SubmitEvent) => {
+    formData = new FormData(form);
+    event.preventDefault();
+  });
+  let formData: FormData;
+
+  form.addEventListener('submit', submitHandler);
+  form.querySelector('[name="b"]')!.remove();
+
+  button.click();
+
+  await waitUntil(() => submitHandler.calledOnce);
+
+  expect(formData!.get('a')).to.equal('1');
+  expect(formData!.get('b')).to.equal(null);
+  expect(formData!.get('c')).to.equal('3');
+});

src/internal/form.ts

@@ -217,7 +217,14 @@ export class FormControlController implements ReactiveController {
     // injecting the name/value on a temporary button, so we can just skip them here.
     const isButton = this.host.tagName.toLowerCase() === 'sl-button';
 
-    if (!disabled && !isButton && typeof name === 'string' && name.length > 0 && typeof value !== 'undefined') {
+    if (
+      this.host.isConnected &&
+      !disabled &&
+      !isButton &&
+      typeof name === 'string' &&
+      name.length > 0 &&
+      typeof value !== 'undefined'
+    ) {
       if (Array.isArray(value)) {
         (value as unknown[]).forEach(val => {
           event.formData.append(name, (val as string | number | boolean).toString());