fix form controls to read from property instead of attribute (#1707)

* fix form controls to read from properties and attributes

* update changelog

* prettier

* update changelog

* prettier

* small comment fix

docs/pages/resources/changelog.md

@@ -14,6 +14,7 @@ New versions of Shoelace are released as-needed and generally occur when a criti
 
 ## Next
 
+- Fixed a bug in form control components that used a `form` property, but not an attribute. [#1707]
 - Fixed a bug with bundled components using CDN builds not having translations on initial connect [#1696]
 - Fixed a bug where the `"sl-change"` event would always fire simultaneously with `"sl-input"` event in `<sl-color-picker>`. The `<sl-change>` event now only fires when a user stops dragging a slider or stops dragging on the color canvas. [#1689]
 - Updated the copy icon in the system library [#1702]

src/components/button/button.component.ts

@@ -45,20 +45,9 @@ export default class SlButton extends ShoelaceElement implements ShoelaceFormCon
   };
 
   private readonly formControlController = new FormControlController(this, {
-    form: input => {
-      // Buttons support a form attribute that points to an arbitrary form, so if this attribute is set we need to query
-      // the form from the same root using its id
-      if (input.hasAttribute('form')) {
-        const doc = input.getRootNode() as Document | ShadowRoot;
-        const formId = input.getAttribute('form')!;
-        return doc.getElementById(formId) as HTMLFormElement;
-      }
-
-      // Fall back to the closest containing form
-      return input.closest('form');
-    },
     assumeInteractionOn: ['click']
   });
+
   private readonly hasSlotController = new HasSlotController(this, '[default]', 'prefix', 'suffix');
   private readonly localize = new LocalizeController(this);
 

src/internal/form.ts

@@ -61,12 +61,16 @@ export class FormControlController implements ReactiveController {
     this.options = {
       form: input => {
         // If there's a form attribute, use it to find the target form by id
-        if (input.hasAttribute('form') && input.getAttribute('form') !== '') {
-          const root = input.getRootNode() as Document | ShadowRoot;
-          const formId = input.getAttribute('form');
+        // Controls may not always reflect the 'form' property. For example, `<sl-button>` doesn't reflect.
+        const formId = input.form;
 
-          if (formId) {
-            return root.getElementById(formId) as HTMLFormElement;
+        if (formId) {
+          const root = input.getRootNode() as Document | ShadowRoot;
+
+          const form = root.getElementById(formId);
+
+          if (form) {
+            return form as HTMLFormElement;
           }
         }
 

src/internal/test/form-control-base-tests.ts

@@ -44,6 +44,7 @@ export function runFormControlBaseTests<T extends ShoelaceFormControl = Shoelace
 //   - `.checkValidity()`
 //   - `.reportValidity()`
 //   - `.setCustomValidity(msg)`
+//   - `.getForm()`
 //
 function runAllValidityTests(
   tagName: string, //
@@ -124,6 +125,27 @@ function runAllValidityTests(
         const emittedEvents = checkEventEmissions(control, 'sl-invalid', () => control.reportValidity());
         expect(emittedEvents.length).to.equal(0);
       });
+
+      it('Should find the correct form when given a form property', async () => {
+        const formId = 'test-form';
+        const form = await fixture(`<form id='${formId}'></form>`);
+        const control = await createControl();
+        expect(control.getForm()).to.equal(null);
+        control.form = 'test-form';
+        await control.updateComplete;
+        expect(control.getForm()).to.equal(form);
+      });
+
+      it('Should find the correct form when given a form attribute', async () => {
+        const formId = 'test-form';
+        const form = await fixture(`<form id='${formId}'></form>`);
+        const control = await createControl();
+        expect(control.getForm()).to.equal(null);
+        control.setAttribute('form', 'test-form');
+
+        await control.updateComplete;
+        expect(control.getForm()).to.equal(form);
+      });
     }
 
     // Run special tests depending on component type