Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removing most node traversal usages #3009

Merged
merged 25 commits into from
Feb 19, 2025
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
35d2398
Fixing some low-hanging node traversal usages
Feb 13, 2025
70a77e5
Adding depth and parentId to node state, so that more traversal can b…
Feb 13, 2025
d1a8b0c
Removing more traversal
Feb 13, 2025
bccc461
Rewriting node traversal in Form.tsx
Feb 13, 2025
2293143
Rewriting more node traversal. This one is a bit risky, as nodes.allN…
Feb 14, 2025
699e49b
Fixing some things I broke when checking for hidden components
Feb 14, 2025
863f7bb
Whoops, FormPage didn't really care about the page key it was given. …
Feb 14, 2025
cec1bcf
No point in reading layout settings twice
Feb 14, 2025
a4dd94d
A layout is also valid if it's the pdf layout (and nodes should be ge…
Feb 14, 2025
8b148bb
Removing useEffectWhenReady() and replacing it with a simpler/more di…
Feb 14, 2025
2aedb79
Making sure these tests wait until preselectedOptionIndex etc is save…
Feb 14, 2025
40192f6
Huh, there were more easy ones - I missed these.
Feb 14, 2025
91b17d9
Oooh, sneaky! I'm so glad to have tests that catch me messing up a 0 …
Feb 14, 2025
c85a0bb
Merge branch 'main' into chore/removing-more-traversal
Feb 14, 2025
47308ef
Replacing a few more traversals
Feb 14, 2025
3c87105
[no ci] Whoops, my simpler fix failed to re-run the useEffect, breaki…
Feb 15, 2025
a8413f6
Instead of skipping node generation for implicitly hidden pages, mark…
Feb 17, 2025
f4500fa
Removing node traversal (and all the delayed selector usage!) from us…
Feb 17, 2025
03a45b6
Yay, an easy one!
Feb 17, 2025
7bcc40b
There were not as simple, but I think the pagination one got consider…
Feb 17, 2025
368aa15
Rewriting the remaining usages of node traversal that I found fairly …
Feb 17, 2025
9247738
Moving attachmentSelector to fix import error when running RepeatingG…
Feb 17, 2025
3ca1307
Fixing two minor issues discovered in review
Feb 19, 2025
3301867
Bringing back useEffectWhenReady()
Feb 19, 2025
06ebb26
Merge branch 'main' into chore/removing-more-traversal
Feb 19, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/codegen/ComponentConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,6 @@ export class ComponentConfig {
import: 'TraversalRestriction',
from: 'src/utils/layout/useNodeTraversal',
});
const LayoutNode = new CG.import({ import: 'LayoutNode', from: 'src/utils/layout/LayoutNode' });

const claimChildrenBody = childrenPlugins.map((plugin) =>
`${pluginRef(plugin)}.claimChildren({
Expand All @@ -429,7 +428,7 @@ export class ComponentConfig {
);

const isChildHiddenBody = childrenPlugins.map(
(plugin) => `${pluginRef(plugin)}.isChildHidden(state as any, childNode)`,
(plugin) => `${pluginRef(plugin)}.isChildHidden(state as any, childId)`,
);

additionalMethods.push(
Expand All @@ -439,7 +438,7 @@ export class ComponentConfig {
`pickDirectChildren(state: ${NodeData}<'${this.type}'>, restriction?: ${TraversalRestriction}) {
return [${pickDirectChildrenBody.join(', ')}];
}`,
`isChildHidden(state: ${NodeData}<'${this.type}'>, childNode: ${LayoutNode}) {
`isChildHidden(state: ${NodeData}<'${this.type}'>, childId: string) {
return [${isChildHiddenBody.join(', ')}].some((h) => h);
}`,
);
Expand All @@ -463,6 +462,9 @@ export class ComponentConfig {
const baseState: ${BaseNodeData}<'${this.type}'> = {
type: 'node',
pageKey: props.pageKey,
parentId: props.parentId,
depth: props.depth,
isValid: props.isValid,
item: undefined,
layout: props.item,
hidden: undefined,
Expand Down
105 changes: 38 additions & 67 deletions src/components/form/Form.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useEffect, useState } from 'react';
import React, { useEffect } from 'react';
import { Helmet } from 'react-helmet-async';

import { Flex } from 'src/app-components/Flex/Flex';
Expand Down Expand Up @@ -26,18 +26,17 @@ import {
import { useOnFormSubmitValidation } from 'src/features/validation/callbacks/onFormSubmitValidation';
import { useTaskErrors } from 'src/features/validation/selectors/taskErrors';
import { useCurrentView, useNavigatePage, useStartUrl } from 'src/hooks/useNavigatePage';
import { getComponentCapabilities } from 'src/layout';
import { GenericComponentById } from 'src/layout/GenericComponent';
import { extractBottomButtons } from 'src/utils/formLayout';
import { getPageTitle } from 'src/utils/getPageTitle';
import { NodesInternal, useGetPage, useNode } from 'src/utils/layout/NodesContext';
import { useNodeTraversalSelector } from 'src/utils/layout/useNodeTraversal';
import { NodesInternal, useNode } from 'src/utils/layout/NodesContext';
import type { NavigateToNodeOptions } from 'src/features/form/layout/NavigateToNode';
import type { AnyValidation, BaseValidation, NodeRefValidation } from 'src/features/validation';
import type { NodeData } from 'src/utils/layout/types';

interface FormState {
hasRequired: boolean;
mainIds: string[] | undefined;
mainIds: string[];
errorReportIds: string[];
formErrors: NodeRefValidation<AnyValidation<'error'>>[];
taskErrors: BaseValidation<'error'>[];
Expand All @@ -54,14 +53,7 @@ export function FormPage({ currentPageId }: { currentPageId: string | undefined
const appName = useAppName();
const appOwner = useAppOwner();
const { langAsString } = useLanguage();
const [formState, setFormState] = useState<FormState>({
hasRequired: false,
mainIds: undefined,
errorReportIds: [],
formErrors: [],
taskErrors: [],
});
const { hasRequired, mainIds, errorReportIds, formErrors, taskErrors } = formState;
const { hasRequired, mainIds, errorReportIds, formErrors, taskErrors } = useFormState(currentPageId);
const requiredFieldsMissing = NodesInternal.usePageHasVisibleRequiredValidations(currentPageId);

useRedirectToStoredPage();
Expand All @@ -86,15 +78,6 @@ export function FormPage({ currentPageId }: { currentPageId: string | undefined
return <FormFirstPage />;
}

if (mainIds === undefined) {
return (
<>
<ErrorProcessing setFormState={setFormState} />
<Loader reason='form-ids' />
</>
);
}

const hasSetCurrentPageId = langAsString(currentPageId) !== currentPageId;

if (!hasSetCurrentPageId) {
Expand All @@ -114,7 +97,6 @@ export function FormPage({ currentPageId }: { currentPageId: string | undefined
<Helmet>
<title>{`${getPageTitle(appName, hasSetCurrentPageId ? langAsString(currentPageId) : undefined, appOwner)}`}</title>
</Helmet>
<ErrorProcessing setFormState={setFormState} />
{hasRequired && (
<MessageBanner
error={requiredFieldsMissing}
Expand Down Expand Up @@ -213,64 +195,53 @@ function useSetExpandedWidth() {
}

const emptyArray = [];
interface ErrorProcessingProps {
setFormState: React.Dispatch<React.SetStateAction<FormState>>;
}

function nodeDataIsRequired(n: NodeData) {
const item = n.item;
return !!(item && 'required' in item && item.required === true);
}

/**
* Instead of re-rendering the entire Form component when any of this changes, we just report the
* state to the parent component.
*/
function ErrorProcessing({ setFormState }: ErrorProcessingProps) {
const currentPageId = useCurrentView();
const page = useGetPage(currentPageId);
const traversalSelector = useNodeTraversalSelector();
function useFormState(currentPageId: string | undefined): FormState {
const { formErrors, taskErrors } = useTaskErrors();
const hasErrors = Boolean(formErrors.length) || Boolean(taskErrors.length);

const topLevelNodeIds = traversalSelector(
(traverser) => {
if (!page) {
return emptyArray;
}
const [mainIds, errorReportIds] = NodesInternal.useMemoSelector((state) => {
const topLevelNodeData = Object.values(state.nodeData).filter(
(node) => node.pageKey === currentPageId && node.parentId === undefined,
);

const all = traverser.with(page).children();
return all.map((n) => n.id);
},
[currentPageId],
);
if (!hasErrors) {
return [topLevelNodeData.map((node) => node.layout.id), emptyArray];
}

const hasRequired = traversalSelector(
(traverser) => {
if (!page) {
return false;
const toMainLayout: string[] = [];
const toErrorReport: string[] = [];

for (const node of topLevelNodeData.reverse()) {
const capabilities = getComponentCapabilities(node.layout.type);
const isButtonLike =
node.layout.type === 'ButtonGroup' || (capabilities.renderInButtonGroup && node.layout.type !== 'Custom');
if (isButtonLike && toMainLayout.length === 0) {
toErrorReport.push(node.layout.id);
} else {
toMainLayout.push(node.layout.id);
}
return traverser.with(page).flat((n) => n.type === 'node' && nodeDataIsRequired(n)).length > 0;
},
[currentPageId],
);
}

const { formErrors, taskErrors } = useTaskErrors();
const hasErrors = Boolean(formErrors.length) || Boolean(taskErrors.length);
return [toMainLayout.reverse(), toErrorReport.reverse()];
});

const [mainIds, errorReportIds] = traversalSelector(
(traverser) => {
if (!hasErrors || !page) {
return [topLevelNodeIds, emptyArray];
}
return extractBottomButtons(traverser.with(page).children());
},
[currentPageId, hasErrors],
const hasRequired = NodesInternal.useSelector((state) =>
Object.values(state.nodeData).some((node) => node.pageKey === currentPageId && nodeDataIsRequired(node)),
);

useEffect(() => {
setFormState({ hasRequired, mainIds, errorReportIds, formErrors, taskErrors });
}, [setFormState, hasRequired, mainIds, errorReportIds, formErrors, taskErrors]);

return null;
return {
hasRequired,
mainIds,
errorReportIds,
formErrors,
taskErrors,
};
}

function HandleNavigationFocusComponent() {
Expand Down
Loading
Loading