fix issue with onBeforePageChange breaking the form, add test, update demos

stutrek · stutrek · commit 840758b56614 · 2024-10-08T11:51:14.000-04:00
diff --git a/docs/src/app/demos/building/sequence.tsx b/docs/src/app/demos/building/sequence.tsx
@@ -101,7 +101,7 @@ const environmentalImpactAssessmentPage: BuildingFormPage = {
 
 // Site Plan Submission Sequence
 const sitePlanSubmissionSequence: BuildingFormSequence = {
-    id: 'sitePlanSubmission',
+    id: 'sitePlanSubmissionSequence',
     pages: [sitePlanSubmissionPage, environmentalImpactAssessmentPage],
 };
 
diff --git a/docs/src/app/layout.tsx b/docs/src/app/layout.tsx
@@ -9,11 +9,13 @@ export default function RootLayout({
 }>) {
     return (
         <html lang="en">
-            <body className="antialiased">{children}</body>
-            <div className="container max-w-3xl mx-auto p-4 text-sm text-gray-400">
-                <hr className="mb-3" />
-                &copy; 2024 Stu Kabakoff
-            </div>
+            <body className="antialiased">
+                {children}
+                <div className="container max-w-3xl mx-auto p-4 text-sm text-gray-400">
+                    <hr className="mb-3" />
+                    &copy; 2024 Stu Kabakoff
+                </div>
+            </body>
         </html>
     );
 }
diff --git a/docs/src/components/FormLibrary.tsx b/docs/src/components/FormLibrary.tsx
@@ -57,7 +57,7 @@ export const FormFieldContainer = ({
             </>
         )}
 
-        {!hideError ? (
+        {error || !hideError ? (
             <div>
                 <ErrorText error={error} />
             </div>
@@ -172,7 +172,6 @@ export const FileInput = forwardRef<
     HTMLInputElement,
     React.HTMLProps<HTMLInputElement> &
         Omit<AdditionalProps, 'error'> & {
-            // biome-ignore lint/suspicious/noExplicitAny: <explanation>
             setValue: UseFormSetValue<any>;
             name: string;
             error?: Partial<{ [K in keyof DocumentReference]: FieldError }>;
@@ -190,15 +189,16 @@ export const FileInput = forwardRef<
             setValue(name, undefined);
         }
     };
+    const errorToUse = error
+        ? {
+              message: 'Please check the box to "upload" a file',
+              type: 'required',
+          }
+        : undefined;
     return (
         <Checkbox
             name={name}
-            error={
-                error?.fileName ||
-                error?.fileSize ||
-                error?.fileType ||
-                error?.fileUrl
-            }
+            error={errorToUse}
             {...rest}
             onChange={handleChange}
             ref={ref}
diff --git a/docs/src/components/SequenceVisualizer/index.tsx b/docs/src/components/SequenceVisualizer/index.tsx
@@ -1,15 +1,12 @@
 import classNames from 'classnames';
 
 import styles from './SequenceVisualizer.module.css';
-import type { SequenceChild } from '../../../../src/types';
+import type { DeepPartial, SequenceChild } from '../../../../src/types';
 
 type SequenceVisualizerProps<T> = {
-    data: T;
-    // biome-ignore lint/suspicious/noExplicitAny: <explanation>
+    data: DeepPartial<T>;
     currentPage: SequenceChild<T, any, any>;
-    // biome-ignore lint/suspicious/noExplicitAny: <explanation>
     pages: SequenceChild<T, any, any>[];
-    idPrefix?: string;
     goToPage: (page: string) => void;
 };
 
@@ -18,7 +15,6 @@ export function SequenceVisualizer<T>({
     currentPage,
     pages,
     goToPage,
-    idPrefix = '',
 }: SequenceVisualizerProps<T>) {
     return (
         <div className={styles.visualizer}>
@@ -28,13 +24,14 @@ export function SequenceVisualizer<T>({
                     isRequired && 'isComplete' in page && page.isComplete(data);
                 const isForm = 'Component' in page;
                 const clickHandler = isForm
-                    ? () => goToPage(`${idPrefix}${page.id}`)
+                    ? () => goToPage(page.id)
                     : undefined;
                 return (
                     <div
                         className={classNames(
                             styles.page,
-                            `${idPrefix}${page.id}` === currentPage.id &&
+                            !('pages' in page) &&
+                                page.id === currentPage.id &&
                                 styles.currentPage,
                             isRequired && isComplete && styles.complete,
                             !isRequired && styles.notNeeded,
@@ -55,7 +52,6 @@ export function SequenceVisualizer<T>({
                                 data={data}
                                 currentPage={currentPage}
                                 pages={page.pages}
-                                idPrefix={`${idPrefix}${page.id}.`}
                                 goToPage={goToPage}
                             />
                         )}
diff --git a/src/__tests__/callbacks.spec.tsx b/src/__tests__/callbacks.spec.tsx
@@ -1,3 +1,4 @@
+import React from 'react';
 import { renderHook, act } from '@testing-library/react';
 import { useMultiPageFormBase } from '../base';
 
@@ -137,4 +138,37 @@ describe('useMultiPageFormBase - Callback Invocation Tests', () => {
         expect(onValidationError).toHaveBeenCalledWith(errorList);
         expect(result.current.currentPage.id).toBe('page1');
     });
+
+    it('can advance when onBeforePageChange is "fixed"', async () => {
+        // Initially, onBeforePageChange returns false
+        let allowNavigation = false;
+        const onBeforePageChange = jest.fn(() => allowNavigation);
+
+        const { result } = renderHook(() =>
+            useMultiPageFormBase({
+                getCurrentData,
+                pages,
+                onBeforePageChange,
+            }),
+        );
+
+        // First attempt to advance; should be prevented
+        await act(async () => {
+            await result.current.advance();
+        });
+
+        expect(onBeforePageChange).toHaveBeenCalledTimes(1);
+        expect(result.current.currentPage.id).toBe('page1');
+
+        // Now, "fix" onBeforePageChange to return true
+        allowNavigation = true;
+
+        // Second attempt to advance; should proceed
+        await act(async () => {
+            await result.current.advance();
+        });
+
+        expect(onBeforePageChange).toHaveBeenCalledTimes(2);
+        expect(result.current.currentPage.id).toBe('page2');
+    });
 });
diff --git a/src/base.ts b/src/base.ts
@@ -178,12 +178,14 @@ export function useMultiPageFormBase<DataT, ComponentProps, ErrorList>({
                 pagesMap[pages[currentPageIndex].id],
             );
             if (errorList === false) {
+                advanceAndNavState.current.navigating = false;
                 return;
             }
             if (errorList !== true && errorList) {
                 if (onValidationError) {
                     onValidationError(errorList);
                 }
+                advanceAndNavState.current.navigating = false;
                 return;
             }
         }

Original file line number	Diff line number	Diff line change
`@@ -178,12 +178,14 @@ export function useMultiPageFormBase<DataT, ComponentProps, ErrorList>({`
`178`	`178`	`pagesMap[pages[currentPageIndex].id],`
`179`	`179`	`);`
`180`	`180`	`if (errorList === false) {`
	`181`	`+ advanceAndNavState.current.navigating = false;`
`181`	`182`	`return;`
`182`	`183`	`}`
`183`	`184`	`if (errorList !== true && errorList) {`
`184`	`185`	`if (onValidationError) {`
`185`	`186`	`onValidationError(errorList);`
`186`	`187`	`}`
	`188`	`+ advanceAndNavState.current.navigating = false;`
`187`	`189`	`return;`
`188`	`190`	`}`
`189`	`191`	`}`