feat: add Page Break field type for multistep forms (#150)

* feat: add Page Break display-only field type

* test: integration test for Page Break form save and DocType sync

* chore: add Page Break to auto-generated TypeScript types

* refactor: map Page Break to Tab Break instead of HTML

Tab Break is a native Frappe layout field that renders proper tabs in
Desk view, replacing the raw HTML <hr> workaround.

* docs: add section navigation bar implementation plan

* Revert "docs: add section navigation bar implementation plan"

This reverts commit b34cec1.

* chore: gitignore screenshot PNGs

* feat: register Page Break field type in frontend

Add Page Break to fieldTypes registry, FieldRenderer, form_fields utils,
and hide it from the sidebar. Filter it out in submission field display.

* feat: multi-section form builder with section navigation

Add section navigation bar for multi-page forms. Sections are derived
from Page Break fields. Supports renaming (double-click), adding, and
removing sections with field merge or deletion options.

* feat: multi-step submission form with step navigation

Add step indicator and Next/Back navigation for multi-page form
submissions. Validates current step fields before advancing.

* feat: polish multi-step submission UI and lift step state into store

- Move step state (useFormSteps) into submissionForm store; FormRenderer
  reads steps/navigation off the store instead of the composable
- Add stepDirection (forward/backward) to drive slide animations
- Redesign StepIndicator as numbered-circle stepper with animated check
  and progress-fill connectors
- Compact step header on step 2+, full header on step 1
- Footer progress pills + Back/Next/Submit/Draft buttons; success fade-up
- Rename builder terminology page -> Step (addSection -> addStep)

* refactor: use frappe-ui semantic color tokens in stepper UI

Replace hardcoded Tailwind grays with surface/ink/outline tokens so the
multi-step submission UI respects theming. Kept the translucent halo ring
on the active step (no opaque token equivalent).

* fix: make multi-step stepper responsive on small screens

Drop the fixed percentage stepper width that starved nodes of space and
collapsed connectors. Use a scrollable w-max/mx-auto row with fixed-width
connectors, bounded nodes, and truncated labels so the stepper centers
when it fits and scrolls cleanly when many steps overflow narrow screens.

* refactor: rename "Section" to "Step" in form editor UI

User-facing labels and dialog text only. Default page-break labels now
read "Step N", and the remove-step dialog and nav aria-labels follow.
Internal symbols and section_* fieldnames unchanged.

* refactor: extract step logic into pure form_steps util

Step grouping was duplicated between editForm store and useFormSteps
composable and had drifted (label fallback, empty-form behavior).
Both now consume one parameterized groupFieldsIntoSteps core.

- form_steps.ts: grouping, page-break index helpers, step mutations
- form_fields.ts: compact/lastRowIndex/scrubFieldname (now take fields
  param instead of reading the store resource)
- editForm store keeps state, persistence, and thin action wrappers;
  step actions renamed removeSection* -> removeStep*, renameSection ->
  renameStep to finish the Section->Step migration

* test: add e2e coverage for multi-step builder and submission

Add Playwright specs for step navigation in the builder (add/rename/
remove/persist) and the public multi-step submission flow (next/back,
value retention, done/current/todo stepper, conditional across steps,
guest path).

Supporting changes to make flows testable + correct:
- SectionNavBar: fix Escape during rename leaking into the next save;
  add aria-label "Remove step" and "Add Step" labels
- StepIndicator: emit data-step-* hooks for stable step assertions
- submission helper: resolve field inputs via label sibling

Two known limitations pinned as test.fixme: getActiveStepEndIndex
leading-PageBreak offset, and REST seeding of empty-label Page Breaks.

* fix: make step remove control keyboard-accessible and harden rename

Address review findings on the multi-step builder:
- SectionNavBar: replace the nested <X role="button"> (mouse-only,
  interactive element inside <Button>) with a real frappe-ui ghost
  icon Button rendered as a sibling of the step tab. Now focusable and
  Enter/Space activatable, and no longer invalid nested-interactive HTML.
- SectionNavBar: reset cancelRename in startRename so an Escape whose
  @blur never fires (DOM removal doesn't reliably blur) can't leak into
  and silently drop the next rename.
- e2e: seed helper now asserts the Form PUT succeeded, failing loudly
  instead of as a downstream locator timeout.
- e2e: scope the remove control via the new [data-step-tab] group since
  it is no longer a child of the tab button.

* refactor: nest step remove control back into the tab button

Keep the remove "X" inside the step tab's suffix slot so it shares the
tab's pill background instead of floating beside it. Keyboard access is
handled on the icon itself: tabindex, Enter/Space activation, and a
focus-visible ring.

Nesting changes the tab's accessible name to "<label> Remove step", so
the e2e stepTab locator now matches the label with that suffix optional.

* fix: render Submit button on fieldless published forms

groupFieldsIntoSteps returns no steps for an empty field list, so
isLastStep was false on step 0 and FormRenderer showed Next instead
of Submit. Pass alwaysIncludeTrailing so there is always >= 1 step.

* fix: add Page Break to FormField fieldtype literal

Runtime map already supported it; the TYPE_CHECKING union lagged behind
the doctype JSON options.

* fix(frontend): keep row_index unique when inserting fields into steps

Field insertion computed row_index from the active section only, so an
empty or middle section produced rows colliding with other sections —
moveField and compact() treat row_index as global. Centralize insertion
in insertFieldAtStepEnd, which numbers the new row from everything
before the insertion point and shifts later rows.

Also fixed along the way:
- addFieldFromDoctype appended to the form end instead of the active
  section; it now uses the same helper
- getActiveStepEndIndex was off by one step when the form has a leading
  Page Break (un-fixmes the palette e2e test)
- renameStep step 0 inserted a leading Page Break at row 0 without
  shifting existing fields off that row

Pure grid helpers moved to form_layout.ts so vitest can import them
without the component registry; form_fields.ts re-exports them.

Author: harshtandiya

.gitignore

@@ -76,4 +76,7 @@ frontend/e2e/playwright-report/
 
 skills-lock.json
 
-semgrep-rules/
+semgrep-rules/
+
+# Screenshots
+*.png

forms_pro/api/submission/test_submission_validation.py

@@ -243,3 +243,41 @@ def test_required_data_field_still_validated_when_heading_present(self):
     def test_display_only_fieldtypes_contains_all_heading_levels(self):
         for fieldtype in ("Heading 1", "Heading 2", "Heading 3"):
             self.assertIn(fieldtype, _DISPLAY_ONLY_FIELDTYPES)
+
+
+class TestPageBreakValidation(unittest.TestCase):
+    def test_page_break_in_display_only_fieldtypes(self):
+        self.assertIn("Page Break", _DISPLAY_ONLY_FIELDTYPES)
+
+    def test_page_break_skipped_even_when_required(self):
+        """reqd=1 Page Break must never trigger a validation error."""
+        form = _form(_field("pb", fieldtype="Page Break", reqd=1))
+        _validate_form_response(form, {})  # must not raise
+
+    def test_required_data_field_still_validated_when_page_break_present(self):
+        """Page Break being skipped must not suppress validation of adjacent required fields."""
+        import frappe
+
+        form = _form(
+            _field("step_two", fieldtype="Page Break"),
+            _field("full_name", fieldtype="Data", reqd=1),
+        )
+        with self.assertRaises(frappe.ValidationError) as ctx:
+            _validate_form_response(form, {})
+        self.assertIn("Full Name", str(ctx.exception))
+
+
+class TestPageBreakProperties(unittest.TestCase):
+    def test_page_break_stores_value_is_false(self):
+        """Page Break maps to Tab Break which is a no_value_field — stores_value must be False."""
+        from frappe.model import no_value_fields
+
+        from forms_pro.forms_pro.doctype.form_field.form_field import FORM_TO_FRAPPE_FIELDTYPE
+
+        mapped = FORM_TO_FRAPPE_FIELDTYPE["Page Break"]
+        self.assertIn(mapped["fieldtype"], no_value_fields)
+
+    def test_page_break_frappe_fieldtype_is_tab_break(self):
+        from forms_pro.forms_pro.doctype.form_field.form_field import FORM_TO_FRAPPE_FIELDTYPE
+
+        self.assertEqual(FORM_TO_FRAPPE_FIELDTYPE["Page Break"]["fieldtype"], "Tab Break")

forms_pro/forms_pro/doctype/form_field/form_field.json

@@ -40,7 +40,7 @@
    "fieldtype": "Select",
    "in_list_view": 1,
    "label": "Fieldtype",
-   "options": "Attach\nData\nNumber\nEmail\nDate\nDate Time\nDate Range\nTime Picker\nPassword\nSelect\nSwitch\nTextarea\nText Editor\nLink\nCheckbox\nRating\nPhone\nTable\nMultiselect\nHeading 1\nHeading 2\nHeading 3",
+   "options": "Attach\nData\nNumber\nEmail\nDate\nDate Time\nDate Range\nTime Picker\nPassword\nSelect\nSwitch\nTextarea\nText Editor\nLink\nCheckbox\nRating\nPhone\nTable\nMultiselect\nHeading 1\nHeading 2\nHeading 3\nPage Break",
    "reqd": 1
   },
   {

forms_pro/forms_pro/doctype/form_field/form_field.py

@@ -34,10 +34,11 @@
     "Heading 1": {"fieldtype": "HTML"},
     "Heading 2": {"fieldtype": "HTML"},
     "Heading 3": {"fieldtype": "HTML"},
+    "Page Break": {"fieldtype": "Tab Break"},
 }
 
 
-_DISPLAY_ONLY_FIELDTYPES = {"Heading 1", "Heading 2", "Heading 3"}
+_DISPLAY_ONLY_FIELDTYPES = {"Heading 1", "Heading 2", "Heading 3", "Page Break"}
 
 
 class FormField(Document):
@@ -78,6 +79,7 @@ class FormField(Document):
             "Heading 1",
             "Heading 2",
             "Heading 3",
+            "Page Break",
         ]
         hidden: DF.Check
         label: DF.Data
@@ -114,13 +116,12 @@ def to_frappe_field(self) -> dict:
         }
 
     def get_options(self) -> str | None:
-        if self.fieldtype in _DISPLAY_ONLY_FIELDTYPES:
-            HEADING_MAP = {
-                "Heading 1": "h1",
-                "Heading 2": "h2",
-                "Heading 3": "h3",
-            }
-            tag = HEADING_MAP.get(self.fieldtype, "h2")
+        HEADING_MAP = {
+            "Heading 1": "h1",
+            "Heading 2": "h2",
+            "Heading 3": "h3",
+        }
+        if tag := HEADING_MAP.get(self.fieldtype):
             return f"<{tag}>{escape_html(self.label or '')}</{tag}>"
 
         return self.options

forms_pro/tests/test_page_break.py

@@ -0,0 +1,71 @@
+import frappe
+from frappe.tests import IntegrationTestCase
+
+from forms_pro.tests.factories.form_factory import FormFactory
+
+
+class TestPageBreakIntegration(IntegrationTestCase):
+    def test_form_with_page_break_saves(self):
+        form = FormFactory.create()
+        form.append(
+            "fields",
+            {
+                "label": "Your Name",
+                "fieldname": "your_name",
+                "fieldtype": "Data",
+                "reqd": 0,
+                "row_index": 0,
+                "column_index": 0,
+                "cell_index": 0,
+            },
+        )
+        form.append(
+            "fields",
+            {
+                "label": "Step 2",
+                "fieldname": "step_2",
+                "fieldtype": "Page Break",
+                "reqd": 0,
+                "row_index": 1,
+                "column_index": 0,
+                "cell_index": 0,
+            },
+        )
+        form.append(
+            "fields",
+            {
+                "label": "Your Email",
+                "fieldname": "your_email",
+                "fieldtype": "Email",
+                "reqd": 0,
+                "row_index": 2,
+                "column_index": 0,
+                "cell_index": 0,
+            },
+        )
+        form.save()
+
+        form.reload()
+        fieldtypes = [f.fieldtype for f in form.fields]
+        self.assertIn("Page Break", fieldtypes)
+
+    def test_page_break_syncs_as_tab_break_to_linked_doctype(self):
+        form = FormFactory.create()
+        form.append(
+            "fields",
+            {
+                "label": "Step 2",
+                "fieldname": "step_2",
+                "fieldtype": "Page Break",
+                "reqd": 0,
+                "row_index": 0,
+                "column_index": 0,
+                "cell_index": 0,
+            },
+        )
+        form.save()
+
+        doctype_doc = frappe.get_doc("DocType", form.linked_doctype)
+        synced = {f.fieldname: f for f in doctype_doc.fields}
+        self.assertIn("step_2", synced)
+        self.assertEqual(synced["step_2"].fieldtype, "Tab Break")

frontend/e2e/helpers/form-builder.ts

@@ -304,4 +304,69 @@ export class FormBuilderPage {
   publishButton() {
     return this.page.getByRole("button", { name: /^publish$/i });
   }
+
+  // ---- Step navigation (SectionNavBar) ----
+
+  stepNav(): Locator {
+    return this.page.locator('nav[aria-label="Form steps"]');
+  }
+
+  // The remove "X" is nested inside the tab button, so tabs after the first
+  // have the accessible name "<label> Remove step" — match the label exactly,
+  // with that suffix optional.
+  stepTab(label: string): Locator {
+    const escaped = label.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
+    return this.stepNav().getByRole("button", {
+      name: new RegExp(`^${escaped}( Remove step)?$`),
+    });
+  }
+
+  removeStepControl(label: string): Locator {
+    return this.stepTab(label).getByLabel("Remove step");
+  }
+
+  activeStepTab(): Locator {
+    return this.stepNav().locator('button[aria-current="true"]');
+  }
+
+  async addStep() {
+    await this.stepNav().getByRole("button", { name: "Add Step" }).click();
+  }
+
+  async switchToStep(label: string) {
+    await this.stepTab(label).click();
+  }
+
+  async renameStep(oldLabel: string, newLabel: string) {
+    await this.stepTab(oldLabel).dblclick();
+    const input = this.stepNav().getByRole("textbox");
+    await input.waitFor({ state: "visible" });
+    await input.fill(newLabel);
+    await input.press("Enter");
+  }
+
+  // mode "keep"  -> "Move fields to previous step"
+  // mode "delete"-> "Remove step and fields"
+  // omit mode for empty steps (no dialog appears)
+  async removeStep(label: string, mode?: "keep" | "delete") {
+    await this.removeStepControl(label).click();
+    if (mode === "keep") {
+      await this.page
+        .getByRole("button", { name: "Move fields to previous step" })
+        .click();
+    } else if (mode === "delete") {
+      await this.page
+        .getByRole("button", { name: "Remove step and fields" })
+        .click();
+    }
+  }
+
+  async save() {
+    const saveBtn = this.page.getByRole("button", {
+      name: "Save",
+      exact: true,
+    });
+    await saveBtn.click();
+    await saveBtn.waitFor({ state: "hidden", timeout: 30000 });
+  }
 }

frontend/e2e/helpers/form-fields.ts

@@ -0,0 +1,82 @@
+import { expect, type APIRequestContext } from "@playwright/test";
+
+export type SeedField = {
+  label: string;
+  fieldtype?: string;
+  fieldname?: string;
+  reqd?: 0 | 1;
+  hidden?: 0 | 1;
+  conditional_logic?: string;
+  row_index?: number;
+  column_index?: number;
+  cell_index?: number;
+};
+
+function defaultFieldname(label: string): string {
+  return label
+    .toLowerCase()
+    .replace(/[^a-z0-9]+/g, "_")
+    .replace(/^_+|_+$/g, "");
+}
+
+// Replace a form's full field list via REST so the builder/public page load a
+// known layout. row_index defaults to the array position, which keeps every
+// field on its own row — fine for step tests, which don't care about columns.
+export async function setFormFields(
+  apiContext: APIRequestContext,
+  formId: string,
+  fields: SeedField[]
+) {
+  const res = await apiContext.put(`/api/resource/Form/${formId}`, {
+    data: {
+      fields: fields.map((f, i) => ({
+        idx: i + 1,
+        fieldtype: f.fieldtype ?? "Data",
+        label: f.label,
+        fieldname: f.fieldname ?? defaultFieldname(f.label),
+        reqd: f.reqd ?? 0,
+        hidden: f.hidden ?? 0,
+        conditional_logic: f.conditional_logic ?? "",
+        row_index: f.row_index ?? i,
+        column_index: f.column_index ?? 0,
+        cell_index: f.cell_index ?? 0,
+      })),
+    },
+  });
+  // Fail loudly here rather than as a confusing locator timeout downstream.
+  expect(
+    res.ok(),
+    `setFormFields PUT failed: ${res.status()} ${await res.text()}`
+  ).toBeTruthy();
+}
+
+export function pageBreak(label = "", fieldname?: string): SeedField {
+  return {
+    label,
+    fieldtype: "Page Break",
+    fieldname: fieldname ?? (label ? defaultFieldname(label) : undefined),
+  };
+}
+
+// 3 steps: Basics(Full Name) / Contact(Work Email) / Final(Notes).
+// Leading Page Break labels step 1.
+export function threeStepFields(): SeedField[] {
+  return [
+    pageBreak("Basics", "pb_basics"),
+    { label: "Full Name" },
+    pageBreak("Contact", "pb_contact"),
+    { label: "Work Email" },
+    pageBreak("Final", "pb_final"),
+    { label: "Notes" },
+  ];
+}
+
+// 2 steps with NO leading Page Break: step 1 is unlabeled in data
+// (builder shows fallback "Step 1"), Page Break labels step 2.
+export function twoStepFieldsNoLeadingPB(): SeedField[] {
+  return [
+    { label: "Full Name" },
+    pageBreak("Details", "pb_details"),
+    { label: "Notes" },
+  ];
+}

frontend/e2e/helpers/submission.ts

@@ -1,4 +1,4 @@
-import type { Page } from "@playwright/test";
+import type { Locator, Page } from "@playwright/test";
 
 export class SubmissionPage {
   constructor(private page: Page) {}
@@ -7,8 +7,16 @@ export class SubmissionPage {
     await this.page.goto(`/forms/p/${route}`);
   }
 
+  // Submission fields render a visible <label> sibling, not a for/id association.
+  fieldInput(label: string): Locator {
+    return this.page
+      .locator("div.flex.flex-col.gap-2")
+      .filter({ has: this.page.getByText(label, { exact: true }) })
+      .getByRole("textbox");
+  }
+
   async fillField(label: string, value: string) {
-    await this.page.getByLabel(label).fill(value);
+    await this.fieldInput(label).fill(value);
   }
 
   async submit() {
@@ -19,4 +27,36 @@ export class SubmissionPage {
   successMessage() {
     return this.page.getByText(/thank you for submitting/i);
   }
+
+  // ---- Multi-step navigation ----
+
+  stepIndicator(): Locator {
+    return this.page.locator('nav[aria-label="Form steps"]');
+  }
+
+  stepNode(index: number): Locator {
+    return this.page.locator(
+      `[data-step-component="step-node"][data-step-index="${index}"]`
+    );
+  }
+
+  nextButton(): Locator {
+    return this.page.getByRole("button", { name: "Next" });
+  }
+
+  backButton(): Locator {
+    return this.page.getByRole("button", { name: "Back" });
+  }
+
+  submitButton(): Locator {
+    return this.page.getByRole("button", { name: "Submit" });
+  }
+
+  async next() {
+    await this.nextButton().click();
+  }
+
+  async back() {
+    await this.backButton().click();
+  }
 }