Fix the edges of the long text limit and height

CHANGELOG.md

@@ -36,6 +36,7 @@ Lines marked **Upgrade step** need you to act after updating.
 
 ### Fixed
 
+-   A long text question with a limit of 100 characters now accepts exactly 100 characters and warns from 101. "Max characters" and "Height in lines" refuse 0, negative and fractional numbers and say why; leave them empty for no limit or the default height. An empty height stays empty when you open the question again. (#292)
 -   Browser Back now leaves a form's Settings page. Before, every tab you opened added a history step, so Back only went through the tabs. A link to a Settings tab still opens that tab. (#287)
 -   A new long text question now shows in your form and in the editor preview, also before you set any options for it. Before, it failed to show. (#286)
 -   A long text question with an empty "Max characters" now has no limit, also when you open it again. Before, the editor showed 500 and saved it back. New long text questions still start at 500. (#291)

resources/js/components/Factory/Main/ConfigureTextarea.test.ts

@@ -10,7 +10,11 @@ const saved = vi.hoisted(() => ({
 
 vi.mock("../utils/useInteractionsUtils", () => ({
     useInteractionsUtils: () => ({
-        findOrCreate: async () => ({ id: 7, label: "", options: saved.options }),
+        findOrCreate: async () => ({
+            id: 7,
+            label: "",
+            options: saved.options,
+        }),
     }),
 }));
 
@@ -40,8 +44,10 @@ describe("long text max characters", () => {
         const { wrapper, save } = await reopen({ rows: 5, max_chars: null });
 
         expect(
-            (wrapper.find("input#textarea-max-chars").element as HTMLInputElement)
-                .value,
+            (
+                wrapper.find("input#textarea-max-chars")
+                    .element as HTMLInputElement
+            ).value,
         ).toBe("");
         expect(save).not.toHaveBeenCalled();
     });
@@ -50,8 +56,10 @@ describe("long text max characters", () => {
         const { wrapper } = await reopen({ rows: 5, max_chars: 120 });
 
         expect(
-            (wrapper.find("input#textarea-max-chars").element as HTMLInputElement)
-                .value,
+            (
+                wrapper.find("input#textarea-max-chars")
+                    .element as HTMLInputElement
+            ).value,
         ).toBe("120");
     });
 
@@ -65,3 +73,110 @@ describe("long text max characters", () => {
         expect(save.mock.lastCall?.[0].options.max_chars).toBeNull();
     });
 });
+
+describe("long text max characters, bad values", () => {
+    const maxInput = (wrapper: Awaited<ReturnType<typeof reopen>>["wrapper"]) =>
+        wrapper.find("input#textarea-max-chars");
+
+    it.each(["0", "-5", "2.5"])(
+        "never saves %s and says why",
+        async (value) => {
+            const { wrapper, save } = await reopen({ rows: 5, max_chars: 500 });
+
+            await maxInput(wrapper).setValue(value);
+            await flushPromises();
+
+            expect(save).not.toHaveBeenCalled();
+            expect(wrapper.find("#textarea-max-chars-error").exists()).toBe(
+                true,
+            );
+        },
+    );
+
+    it.each([0, -3])("shows a saved %s as empty", async (saved) => {
+        const { wrapper } = await reopen({ rows: 5, max_chars: saved });
+
+        expect((maxInput(wrapper).element as HTMLInputElement).value).toBe("");
+    });
+});
+
+describe("long text height", () => {
+    const rowsInput = (
+        wrapper: Awaited<ReturnType<typeof reopen>>["wrapper"],
+    ) => wrapper.find("input#textarea-rows");
+
+    it("saves no height when the owner clears it, and keeps it empty on reopen", async () => {
+        const { wrapper, save } = await reopen({ rows: 5, max_chars: 500 });
+
+        await rowsInput(wrapper).setValue("");
+        await flushPromises();
+
+        expect(save.mock.lastCall?.[0].options.rows).toBeNull();
+
+        const reopened = await reopen({ rows: null, max_chars: 500 });
+        expect(
+            (rowsInput(reopened.wrapper).element as HTMLInputElement).value,
+        ).toBe("");
+        expect(reopened.save).not.toHaveBeenCalled();
+    });
+
+    it.each(["0", "-1", "2.5", "11"])(
+        "never saves a height of %s",
+        async (value) => {
+            const { wrapper, save } = await reopen({ rows: 5, max_chars: 500 });
+
+            await rowsInput(wrapper).setValue(value);
+            await flushPromises();
+
+            expect(save).not.toHaveBeenCalled();
+            expect(wrapper.find("#textarea-rows-error").exists()).toBe(true);
+        },
+    );
+});
+
+describe("half-typed numbers", () => {
+    // the browser reads a half-typed "-" or "2." as an empty value and flags it
+    const typeHalf = async (selector: string, wrapper: any) => {
+        const input = wrapper.find(selector);
+        Object.defineProperty(input.element, "validity", {
+            value: { badInput: true },
+            configurable: true,
+        });
+        await input.setValue("");
+        await flushPromises();
+    };
+
+    it.each([
+        [
+            "max characters",
+            "input#textarea-max-chars",
+            "#textarea-max-chars-error",
+        ],
+        ["height", "input#textarea-rows", "#textarea-rows-error"],
+    ])(
+        "does not save %s as empty while it holds a bad number",
+        async (_, input, error) => {
+            const { wrapper, save } = await reopen({ rows: 5, max_chars: 500 });
+
+            await typeHalf(input, wrapper);
+
+            expect(save).not.toHaveBeenCalled();
+            expect(wrapper.find(error).exists()).toBe(true);
+        },
+    );
+
+    it("saves no limit once the owner deletes the half-typed sign", async () => {
+        const { wrapper, save } = await reopen({ rows: 5, max_chars: 500 });
+        await typeHalf("input#textarea-max-chars", wrapper);
+
+        const input = wrapper.find("input#textarea-max-chars");
+        Object.defineProperty(input.element, "validity", {
+            value: { badInput: false },
+            configurable: true,
+        });
+        await input.setValue("");
+        await flushPromises();
+
+        expect(save.mock.lastCall?.[0].options.max_chars).toBeNull();
+    });
+});

resources/js/components/Factory/Main/ConfigureTextarea.vue

@@ -20,12 +20,21 @@
         id="textarea-rows"
         placeholder="e.g. 4"
         type="number"
-        :min="1"
-        :max="10"
         step="1"
         block
         v-model="rows"
+        @input="rowsBad = isBadInput($event)"
+        :aria-invalid="!rowsValid"
+        aria-describedby="textarea-rows-error"
       />
+      <p
+        v-if="!rowsValid"
+        id="textarea-rows-error"
+        role="alert"
+        class="mt-1 text-xs text-red-600"
+      >
+        Use a whole number from 1 to 10, or leave it empty.
+      </p>
     </div>
     <div class="mb-4">
       <D9Label id="textarea-max-chars" label="Max characters" />
@@ -35,25 +44,53 @@
         type="number"
         block
         v-model="maxChars"
+        @input="maxCharsBad = isBadInput($event)"
+        :aria-invalid="!maxCharsValid"
+        aria-describedby="textarea-max-chars-error"
       />
+      <p
+        v-if="!maxCharsValid"
+        id="textarea-max-chars-error"
+        role="alert"
+        class="mt-1 text-xs text-red-600"
+      >
+        Use a whole number from 1, or leave it empty for no limit.
+      </p>
     </div>
   </div>
 </template>
 
 <script setup lang="ts">
 import { useWorkbench } from "@/stores";
 import { D9Label, D9Input } from "@deck9/ui";
-import { Ref, ref, onMounted } from "vue";
+import { Ref, computed, ref, onMounted } from "vue";
 import { useInteractionsUtils } from "../utils/useInteractionsUtils";
 import { useSettingsWatch } from "../utils/useSettingsWatch";
 
 const workbench = useWorkbench();
 const { findOrCreate } = useInteractionsUtils();
 
 const label = ref<FormBlockInteractionModel["label"]>("");
-const rows = ref<number>(5);
+// empty means the default height
+const rows = ref<number | "">(5);
 // empty means no limit; a new question starts at 500
 const maxChars = ref<number | "">(500);
+// a whole number from 1, or empty; anything else is never saved
+const isWhole = (value: number | string | null | undefined) =>
+  value === "" ||
+  value == null ||
+  (Number.isInteger(Number(value)) && Number(value) >= 1);
+// a half-typed "-" or "2." reads as an empty value, the browser flags it
+const rowsBad = ref(false);
+const maxCharsBad = ref(false);
+const isBadInput = (event: Event) =>
+  (event.target as HTMLInputElement).validity.badInput;
+const rowsValid = computed(
+  () => !rowsBad.value && isWhole(rows.value) && Number(rows.value) <= 10,
+);
+const maxCharsValid = computed(
+  () => !maxCharsBad.value && isWhole(maxChars.value),
+);
 const interaction = ref(null) as unknown as Ref<FormBlockInteractionModel>;
 
 onMounted(async () => {
@@ -65,22 +102,30 @@ onMounted(async () => {
     interaction.value = await findOrCreate("textarea", workbench);
 
     label.value = interaction.value.label;
-    rows.value = interaction.value.options?.rows ?? 5;
-    maxChars.value = isNew ? 500 : (interaction.value.options?.max_chars ?? "");
+    // a saved 0 or negative number means no limit on the public form
+    const savedRows = interaction.value.options?.rows;
+    const savedMax = interaction.value.options?.max_chars;
+    rows.value = isNew ? 5 : isWhole(savedRows) ? (savedRows ?? "") : "";
+    maxChars.value = isNew ? 500 : isWhole(savedMax) ? (savedMax ?? "") : "";
     await settled(isNew);
   }
 });
 
-const settled = useSettingsWatch([label, rows, maxChars], (newValues) => {
-  const update = {
-    id: interaction.value.id,
-    label: newValues[0],
-    options: {
-      rows: Math.max(1, Math.min(10, parseInt(newValues[1]))),
-      max_chars: parseInt(newValues[2]) || null,
-    },
-  };
+const settled = useSettingsWatch(
+  [label, rows, maxChars, rowsBad, maxCharsBad],
+  (newValues) => {
+    if (!rowsValid.value || !maxCharsValid.value) return;
 
-  workbench.updateInteraction(update);
-});
+    const update = {
+      id: interaction.value.id,
+      label: newValues[0],
+      options: {
+        rows: newValues[1] === "" ? null : Number(newValues[1]),
+        max_chars: newValues[2] === "" ? null : Number(newValues[2]),
+      },
+    };
+
+    workbench.updateInteraction(update);
+  },
+);
 </script>

resources/js/forms/classic/interactions/useTextareaAction.test.ts

@@ -0,0 +1,30 @@
+import { mount } from "@vue/test-utils";
+import { describe, expect, it, vi } from "vitest";
+import CharCount from "@/components/CharCount.vue";
+import { useTextareaAction } from "./useTextareaAction";
+
+vi.mock("vue-i18n", () => ({ useI18n: () => ({ t: (key: string) => key }) }));
+
+const limitOf = (max: number) =>
+    useTextareaAction({
+        type: "input-long",
+        is_required: false,
+        interactions: [{ options: { max_chars: max } }],
+    } as unknown as PublicFormBlockModel).validator;
+
+describe("long text limit edge", () => {
+    it("accepts exactly N characters and refuses N + 1", () => {
+        const validate = limitOf(5);
+
+        expect(validate({ payload: "a".repeat(5) }).valid).toBe(true);
+        expect(validate({ payload: "a".repeat(6) }).valid).toBe(false);
+    });
+
+    it("shows the counter in the warning color only above N", () => {
+        const counter = (text: string) =>
+            mount(CharCount, { props: { text, maxChars: 5 } }).find("span");
+
+        expect(counter("a".repeat(5)).classes()).not.toContain("text-red-600");
+        expect(counter("a".repeat(6)).classes()).toContain("text-red-600");
+    });
+});

resources/js/forms/classic/interactions/useTextareaAction.ts

@@ -20,7 +20,7 @@ export function useTextareaAction(block: PublicFormBlockModel) {
             action.options?.max_chars &&
             action.options?.max_chars > 0
         ) {
-            if (input?.payload?.length >= action.options.max_chars) {
+            if (input?.payload?.length > action.options.max_chars) {
                 return {
                     valid: false,
                     message:

resources/js/types/models.d.ts

@@ -166,7 +166,7 @@ type FormBlockInteractionType =
     | "date";
 
 type FormBlockInteractionSettings = {
-    rows?: number;
+    rows?: number | null;
     max_chars?: number | null;
     minDate?: string;
     maxDate?: string;