A lot of comments about forms.

web/src/elements/forms/Form.ts

@@ -39,6 +39,41 @@ export interface KeyUnknown {
     [key: string]: unknown;
 }
 
+/**
+ * Form
+ *
+ * The base form element for interacting with user inputs.
+ *
+ * All forms either[1] inherit from this class and implement the `renderInlineForm()` method to
+ * produce the actual form, or include the form in-line as a slotted element. Bizarrely, this form
+ * will not render at all if it's not actually in the viewport?[2]
+ *
+ * @element ak-form
+ *
+ * @slot - Where the form goes if `renderInlineForm()` returns undefined.
+ * @fires eventname - description
+ *
+ * @csspart partname - description
+ */
+
+/* TODO:
+ *
+ * 1. Specialization: Separate this component into three different classes:
+ *    - The base class
+ *    - The "use `renderInlineForm` class
+ *    - The slotted class.
+ * 2. Ask why the form class won't render anything if it's not in the viewport.
+ * 3. Ask why there's so much slug management code.
+ * 4. There is already specialization-by-type throughout all of our code.
+ *    Consider refactoring serializeForm() so that the conversions are on
+ *    the input types, rather than here. (i.e. "Polymorphism is better than
+ *    switch.")
+ * 
+ * 
+ */
+  
+   
+
 @customElement("ak-form")
 export abstract class Form<T> extends AKElement {
     abstract send(data: T): Promise<unknown>;
@@ -69,6 +104,10 @@ export abstract class Form<T> extends AKElement {
         ];
     }
 
+    /**
+     * Called by the render function. Blocks rendering the form if the form is not within the
+     * viewport [2]
+     */
     get isInViewport(): boolean {
         const rect = this.getBoundingClientRect();
         return !(rect.x + rect.y + rect.width + rect.height === 0);
@@ -78,6 +117,11 @@ export abstract class Form<T> extends AKElement {
         return this.successMessage;
     }
 
+    /**
+     * After rendering the form, if there is both a `name` and `slug` element within the form,
+     * events the `name` element so that the slug will always have a slugified version of the `name.`.  This duplicates
+     * functionality within ak-form-element-horizontal. [3]
+     */
     updated(): void {
         this.shadowRoot
             ?.querySelectorAll("ak-form-element-horizontal[name=name]")
@@ -111,6 +155,12 @@ export abstract class Form<T> extends AKElement {
         form?.reset();
     }
 
+    /**
+     * Return the form elements that may contain filenames. Not sure why this is quite so
+     * convoluted. There is exactly one case where this is used:
+     * `./flow/stages/prompt/PromptStage: 147: case PromptTypeEnum.File.`
+     * Consider moving this functionality to there.
+     */
     getFormFiles(): { [key: string]: File } {
         const files: { [key: string]: File } = {};
         const elements =
@@ -134,6 +184,10 @@ export abstract class Form<T> extends AKElement {
         return files;
     }
 
+    /**
+     * Convert the elements of the form to JSON.[4]
+     *
+     */
     serializeForm(): T | undefined {
         const elements =
             this.shadowRoot?.querySelectorAll<HorizontalFormElement>(
@@ -199,6 +253,9 @@ export abstract class Form<T> extends AKElement {
         return json as unknown as T;
     }
 
+    /**
+     * As far as anyone can remember, this isn't being used.
+     */
     private serializeFieldRecursive(
         element: HTMLInputElement,
         value: unknown,
@@ -219,6 +276,12 @@ export abstract class Form<T> extends AKElement {
         parent[nameElements[nameElements.length - 1]] = value;
     }
 
+    /**
+     * Serialize and send the form to the destination. The `send()` method must be overridden for
+     * this to work. If processing the data results in an error, we catch the error, distribute
+     * field-levels errors to the fields, and send the rest of them to the Notifications.
+     *
+     */
     async submit(ev: Event): Promise<unknown | undefined> {
         ev.preventDefault();
         try {

web/src/elements/forms/FormElement.ts

@@ -9,6 +9,12 @@ import PFFormControl from "@patternfly/patternfly/components/FormControl/form-co
 
 import { ErrorDetail } from "@goauthentik/api";
 
+/**
+ * This is only used in two places, and in both cases is used primarily to display
+ * content, not take input.  It displays the TOPT QR code, and the static recovery
+ * tokens.
+ */
+
 @customElement("ak-form-element")
 export class FormElement extends AKElement {
     static get styles(): CSSResult[] {

web/src/elements/forms/FormGroup.ts

@@ -8,9 +8,21 @@ import PFForm from "@patternfly/patternfly/components/Form/form.css";
 import PFFormControl from "@patternfly/patternfly/components/FormControl/form-control.css";
 import PFBase from "@patternfly/patternfly/patternfly-base.css";
 
+/**
+ * Form Group
+ *
+ * Mostly visual effects, with a single interaction for opening/closing the view.
+ *
+ */
+
+/**
+ * TODO: Listen for custom events from its children about 'invalidation' events, and
+ * trigger the `expanded` property as needed.
+ */
+
 @customElement("ak-form-group")
 export class FormGroup extends AKElement {
-    @property({ type: Boolean })
+    @property({ type: Boolean, reflect: true })
     expanded = false;
 
     static get styles(): CSSResult[] {

web/src/elements/forms/HorizontalFormElement.ts

@@ -12,9 +12,30 @@ import PFFormControl from "@patternfly/patternfly/components/FormControl/form-co
 import PFBase from "@patternfly/patternfly/patternfly-base.css";
 
 /**
+ *
+ * Horizontal Form Element Container.
+ *
+ * This element provides the interface between elements of our forms and the
+ * form itself.  
  * @custom-element ak-form-element-horizontal
  */
 
+/* TODO
+
+ * 1. Replace the "probe upward for a parent object to event" with an event handler on the parent
+ *    group.
+ * 2. Updated() has a lot of that slug code again. Really, all you want is for the slug input object
+ *    to update itself if its content seems to have been tracking some other key element.
+ * 3. Updated() pushes the `name` field down to the children, as if that were necessary; why isn't
+ *    it being written on-demand when the child is written? Because it's slotted... despite there
+ *    being very few unique uses.
+ * 4. There is some very specific use-case around the `writeOnly` boolean; this seems to be a case
+ *    where the field isn't available for the user to view unless they explicitly request to be able
+ *    to see the content; otherwise, a dead password field is shown. There are 10 uses of this
+ *    feature.
+ * 
+ */
+
 @customElement("ak-form-element-horizontal")
 export class HorizontalFormElement extends AKElement {
     static get styles(): CSSResult[] {
@@ -56,6 +77,9 @@ export class HorizontalFormElement extends AKElement {
 
     _invalid = false;
 
+    /* If this property changes, we want to make sure the parent control is "opened" so
+     * that users can see the change.[1]
+     */ 
     @property({ type: Boolean })
     set invalid(v: boolean) {
         this._invalid = v;

web/src/elements/forms/ModelForm.ts

@@ -5,6 +5,13 @@ import { Form } from "@goauthentik/elements/forms/Form";
 import { TemplateResult, html } from "lit";
 import { property } from "lit/decorators.js";
 
+/**
+ * Model form
+ *
+ * A base form that automatically tracks the server-side object (instance)
+ * that we're interested in.  Handles loading and tracking of the instance.
+ */
+
 export abstract class ModelForm<T, PKT extends string | number> extends Form<T> {
     abstract loadInstance(pk: PKT): Promise<T>;