From 69ef65f1c980c7659ec6aa39a9a7f2340c1dfa63 Mon Sep 17 00:00:00 2001
From: Cory LaViska <cory@abeautifulsite.net>
Date: Thu, 18 Jan 2024 10:56:49 -0600
Subject: [PATCH] fixes #1823

---
 docs/pages/resources/changelog.md |  5 +--
 src/internal/form.test.ts         | 59 ++++++++++++++++++++++++++++++-
 src/internal/form.ts              |  9 ++++-
 3 files changed, 69 insertions(+), 4 deletions(-)

diff --git a/docs/pages/resources/changelog.md b/docs/pages/resources/changelog.md
index d1d03f4b..143af116 100644
--- a/docs/pages/resources/changelog.md
+++ b/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]
 - Improved the accessibility of `<sl-tooltip>` so they persist when hovering over the tooltip and dismiss when pressing [[Esc]] [#1734]
 
 ## 2.12.0
diff --git a/src/internal/form.test.ts b/src/internal/form.test.ts
index 3ce21e2e..1857df18 100644
--- a/src/internal/form.test.ts
+++ b/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');
+});
diff --git a/src/internal/form.ts b/src/internal/form.ts
index 1533f31e..46c5a44a 100644
--- a/src/internal/form.ts
+++ b/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());