From 35d2398233b723b0df64398194be5d0cf0c427ec Mon Sep 17 00:00:00 2001 From: Ole Martin Handeland Date: Thu, 13 Feb 2025 15:03:37 +0100 Subject: [PATCH 01/21] Fixing some low-hanging node traversal usages --- .../ExpressionPlayground.tsx | 11 +++-- src/features/pdf/PdfView2.tsx | 21 ++++++---- src/layout/Grid/GridSummary.tsx | 41 ++++++++++++++----- .../Summary/SubformSummaryComponent2.tsx | 25 +++++------ .../CommonSummaryComponents/EditButton.tsx | 2 +- .../SummaryComponent2/PageSummary.tsx | 7 ++-- src/utils/layout/NodesContext.tsx | 7 ++++ 7 files changed, 74 insertions(+), 40 deletions(-) diff --git a/src/features/devtools/components/ExpressionPlayground/ExpressionPlayground.tsx b/src/features/devtools/components/ExpressionPlayground/ExpressionPlayground.tsx index 43b0df9402..87c7378701 100644 --- a/src/features/devtools/components/ExpressionPlayground/ExpressionPlayground.tsx +++ b/src/features/devtools/components/ExpressionPlayground/ExpressionPlayground.tsx @@ -12,9 +12,9 @@ import { ExprVal } from 'src/features/expressions/types'; import { ExprValidation } from 'src/features/expressions/validation'; import { useNavigationParam } from 'src/features/routing/AppRoutingContext'; import comboboxClasses from 'src/styles/combobox.module.css'; -import { useNodes } from 'src/utils/layout/NodesContext'; +import { NodesInternal, useNodes } from 'src/utils/layout/NodesContext'; import { useExpressionDataSources } from 'src/utils/layout/useExpressionDataSources'; -import { useNodeTraversal, useNodeTraversalSelector } from 'src/utils/layout/useNodeTraversal'; +import { useNodeTraversalSelector } from 'src/utils/layout/useNodeTraversal'; import type { ExprConfig, Expression, ExprFunctionName } from 'src/features/expressions/types'; import type { LayoutNode } from 'src/utils/layout/LayoutNode'; import type { LayoutPage } from 'src/utils/layout/LayoutPage'; @@ -76,8 +76,11 @@ export const ExpressionPlayground = () => { const traversalSelector = useNodeTraversalSelector(); - const componentOptions = useNodeTraversal((t) => - t.allNodes().map((n) => ({ label: n.id, value: `${n.page.pageKey}|${n.id}` })), + const componentOptions = NodesInternal.useShallowSelector((state) => + Object.values(state.nodeData || {}).map((nodeData) => ({ + label: nodeData.layout.id, + value: `${nodeData.pageKey}|${nodeData.layout.id}`, + })), ); useEffect(() => { diff --git a/src/features/pdf/PdfView2.tsx b/src/features/pdf/PdfView2.tsx index 106623f860..c4e4c2a244 100644 --- a/src/features/pdf/PdfView2.tsx +++ b/src/features/pdf/PdfView2.tsx @@ -26,7 +26,7 @@ import { SubformSummaryComponent2 } from 'src/layout/Subform/Summary/SubformSumm import { SummaryComponent } from 'src/layout/Summary/SummaryComponent'; import { ComponentSummary } from 'src/layout/Summary2/SummaryComponent2/ComponentSummary'; import { SummaryComponent2 } from 'src/layout/Summary2/SummaryComponent2/SummaryComponent2'; -import { Hidden, NodesInternal } from 'src/utils/layout/NodesContext'; +import { Hidden, NodesInternal, useNode } from 'src/utils/layout/NodesContext'; import { useNodeItem } from 'src/utils/layout/useNodeItem'; import { useNodeTraversal } from 'src/utils/layout/useNodeTraversal'; import type { IPdfFormat } from 'src/features/pdf/types'; @@ -85,12 +85,16 @@ export const PDFView2 = () => { }; export function DataLoaderStoreInit({ children }: PropsWithChildren) { - const subforms = useNodeTraversal((t) => t.allNodes().filter((node) => node.isType('Subform'))); + const subformIds = NodesInternal.useShallowSelector((state) => + Object.values(state.nodeData || {}) + .filter((node) => node.layout.type === 'Subform') + .map((node) => node.layout.id), + ); const [loadingState, setLoadingState] = React.useState(() => { const initialLoadingState: Record = {}; - for (const subform of subforms) { - initialLoadingState[subform.id] = DataLoadingState.Loading; + for (const subformId of subformIds) { + initialLoadingState[subformId] = DataLoadingState.Loading; } return initialLoadingState; @@ -107,10 +111,10 @@ export function DataLoaderStoreInit({ children }: PropsWithChildren) { return ( <> - {subforms.map((subform, idx) => ( + {subformIds.map((subformId, idx) => ( ))} @@ -120,12 +124,13 @@ export function DataLoaderStoreInit({ children }: PropsWithChildren) { } function DataLoaderStoreInitWorker({ - node, + nodeId, initComplete, }: PropsWithChildren<{ - node: LayoutNode<'Subform'>; + nodeId: string; initComplete: (subformId: string) => void; }>): React.JSX.Element | null { + const node = useNode(nodeId) as LayoutNode<'Subform'>; const { layoutSet } = useNodeItem(node); const setDataLoaderElements = useDataLoadingStore((state) => state.setDataElements); const dataLoaderElements = useDataLoadingStore((state) => state.dataElements); diff --git a/src/layout/Grid/GridSummary.tsx b/src/layout/Grid/GridSummary.tsx index d34f58de54..5c70ada296 100644 --- a/src/layout/Grid/GridSummary.tsx +++ b/src/layout/Grid/GridSummary.tsx @@ -12,6 +12,7 @@ import { usePdfModeActive } from 'src/features/pdf/PDFWrapper'; import { useUnifiedValidationsForNode } from 'src/features/validation/selectors/unifiedValidationsForNode'; import { validationsOfSeverity } from 'src/features/validation/utils'; import { useIsMobile } from 'src/hooks/useDeviceWidths'; +import { getComponentDef } from 'src/layout'; import { CompCategory } from 'src/layout/common'; import { GenericComponent } from 'src/layout/GenericComponent'; import classes from 'src/layout/Grid/GridSummary.module.css'; @@ -19,9 +20,8 @@ import { isGridRowHidden } from 'src/layout/Grid/tools'; import { EditButton } from 'src/layout/Summary2/CommonSummaryComponents/EditButton'; import { ComponentSummary } from 'src/layout/Summary2/SummaryComponent2/ComponentSummary'; import { getColumnStyles } from 'src/utils/formComponentUtils'; -import { Hidden, useNode } from 'src/utils/layout/NodesContext'; +import { Hidden, NodesInternal, useNode } from 'src/utils/layout/NodesContext'; import { useNodeItem } from 'src/utils/layout/useNodeItem'; -import { useNodeTraversal } from 'src/utils/layout/useNodeTraversal'; import { typedBoolean } from 'src/utils/typing'; import type { DisplayDataProps } from 'src/features/displayData'; import type { @@ -32,6 +32,7 @@ import type { } from 'src/layout/common.generated'; import type { GridCellInternal, GridCellNode, GridRowInternal } from 'src/layout/Grid/types'; import type { ITextResourceBindings } from 'src/layout/layout'; +import type { EditButtonProps } from 'src/layout/Summary2/CommonSummaryComponents/EditButton'; import type { LayoutNode } from 'src/utils/layout/LayoutNode'; type GridSummaryProps = Readonly<{ @@ -145,7 +146,7 @@ export function GridRowRenderer(props: GridRowProps) { const isSmall = isMobile && !pdfModeActive; - const firstNode = useFirstFormNode(row); + const firstNodeId = useFirstFormNodeId(row); if (isGridRowHidden(row, isHiddenSelector)) { return null; @@ -171,9 +172,9 @@ export function GridRowRenderer(props: GridRowProps) { )} {!pdfModeActive && !row.header && !isSmall && ( - {firstNode && !row.readOnly && ( - )} @@ -183,13 +184,14 @@ export function GridRowRenderer(props: GridRowProps) { ); } -function useFirstFormNode(row: GridRowInternal) { - return useNodeTraversal((t) => { +function useFirstFormNodeId(row: GridRowInternal): string | undefined { + return NodesInternal.useSelector((state) => { for (const cell of row.cells) { if (cell && 'nodeId' in cell && cell.nodeId) { - const node = t.findById(cell.nodeId); - if (node && node.isCategory(CompCategory.Form)) { - return node; + const nodeData = state.nodeData?.[cell.nodeId]; + const def = nodeData && getComponentDef(nodeData.layout.type); + if (def && def.category === CompCategory.Form) { + return nodeData.layout.id; } } } @@ -197,6 +199,23 @@ function useFirstFormNode(row: GridRowInternal) { }); } +function WrappedEditButton({ + componentNodeId, + ...rest +}: { componentNodeId: string } & Omit) { + const node = useNode(componentNodeId); + if (!node) { + return null; + } + + return ( + + ); +} + type InternalRowProps = PropsWithChildren>; function InternalRow({ header, readOnly, children }: InternalRowProps) { diff --git a/src/layout/Subform/Summary/SubformSummaryComponent2.tsx b/src/layout/Subform/Summary/SubformSummaryComponent2.tsx index f8514853d9..c04b4298d7 100644 --- a/src/layout/Subform/Summary/SubformSummaryComponent2.tsx +++ b/src/layout/Subform/Summary/SubformSummaryComponent2.tsx @@ -14,11 +14,12 @@ import classes from 'src/layout/Subform/Summary/SubformSummaryComponent2.module. import { SubformSummaryTable } from 'src/layout/Subform/Summary/SubformSummaryTable'; import classes_singlevaluesummary from 'src/layout/Summary2/CommonSummaryComponents/SingleValueSummary.module.css'; import { LayoutSetSummary } from 'src/layout/Summary2/SummaryComponent2/LayoutSetSummary'; +import { NodesInternal, useNode } from 'src/utils/layout/NodesContext'; import { useNodeItem } from 'src/utils/layout/useNodeItem'; -import { useNodeTraversal } from 'src/utils/layout/useNodeTraversal'; import type { LayoutNode } from 'src/utils/layout/LayoutNode'; -export const SummarySubformWrapper = ({ node }: PropsWithChildren<{ node: LayoutNode<'Subform'> }>) => { +export const SummarySubformWrapper = ({ nodeId }: PropsWithChildren<{ nodeId: string }>) => { + const node = useNode(nodeId) as LayoutNode<'Subform'>; const { layoutSet, id, textResourceBindings } = useNodeItem(node); const dataType = useDataTypeFromLayoutSet(layoutSet); const dataElements = useStrictDataElements(dataType); @@ -121,16 +122,16 @@ export function SubformSummaryComponent2({ subformId?: string; componentNode?: LayoutNode<'Subform'>; }) { - const children = useNodeTraversal((t) => - t - .allNodes() - .filter((node) => node.isType('Subform')) - .filter((child) => { + const allOrOneSubformId = NodesInternal.useShallowSelector((state) => + Object.values(state.nodeData ?? {}) + .filter((data) => data.layout.type === 'Subform') + .filter((data) => { if (!subformId) { - return child; + return data; } - return child.id === subformId; - }), + return data.layout.id === subformId; + }) + .map((data) => data.layout.id), ); if (displayType === 'table' && componentNode) { @@ -139,10 +140,10 @@ export function SubformSummaryComponent2({ return ( <> - {children.map((child, idx) => ( + {allOrOneSubformId.map((childId, idx) => ( ))} diff --git a/src/layout/Summary2/CommonSummaryComponents/EditButton.tsx b/src/layout/Summary2/CommonSummaryComponents/EditButton.tsx index ccce850a00..214c5e8f93 100644 --- a/src/layout/Summary2/CommonSummaryComponents/EditButton.tsx +++ b/src/layout/Summary2/CommonSummaryComponents/EditButton.tsx @@ -15,7 +15,7 @@ import { useNodeItem } from 'src/utils/layout/useNodeItem'; import type { NavigationResult } from 'src/features/form/layout/NavigateToNode'; import type { LayoutNode } from 'src/utils/layout/LayoutNode'; -type EditButtonProps = { +export type EditButtonProps = { componentNode: LayoutNode; summaryComponentId?: string; navigationOverride?: (() => Promise | void) | null; diff --git a/src/layout/Summary2/SummaryComponent2/PageSummary.tsx b/src/layout/Summary2/SummaryComponent2/PageSummary.tsx index 1be97af398..3cc99cbd07 100644 --- a/src/layout/Summary2/SummaryComponent2/PageSummary.tsx +++ b/src/layout/Summary2/SummaryComponent2/PageSummary.tsx @@ -1,18 +1,17 @@ import React from 'react'; import { ComponentSummary } from 'src/layout/Summary2/SummaryComponent2/ComponentSummary'; -import { Hidden } from 'src/utils/layout/NodesContext'; +import { Hidden, useGetPage } from 'src/utils/layout/NodesContext'; import { useNodeTraversal } from 'src/utils/layout/useNodeTraversal'; import { typedBoolean } from 'src/utils/typing'; -import type { LayoutNode } from 'src/utils/layout/LayoutNode'; interface PageSummaryProps { pageId: string; } export function PageSummary({ pageId }: PageSummaryProps) { - const page = useNodeTraversal((dings) => dings.findPage(pageId)); - const children = useNodeTraversal((t) => t.children(), page) as (LayoutNode | undefined)[]; + const page = useGetPage(pageId); + const children = useNodeTraversal((t) => (page ? t.with(page).children() : [])); const isHiddenPage = Hidden.useIsHiddenPage(page); if (!page || !children) { diff --git a/src/utils/layout/NodesContext.tsx b/src/utils/layout/NodesContext.tsx index 74742f576a..861aedd8f7 100644 --- a/src/utils/layout/NodesContext.tsx +++ b/src/utils/layout/NodesContext.tsx @@ -52,6 +52,7 @@ import type { OptionsStorePluginConfig } from 'src/features/options/OptionsStore import type { ValidationsProcessedLast } from 'src/features/validation'; import type { ValidationStorePluginConfig } from 'src/features/validation/ValidationStorePlugin'; import type { DSProps, DSReturn, InnerSelectorMode, OnlyReRenderWhen } from 'src/hooks/delayedSelectors'; +import type { ObjectOrArray } from 'src/hooks/useShallowMemo'; import type { WaitForState } from 'src/hooks/useWaitForState'; import type { CompExternal, CompTypes, ILayouts } from 'src/layout/layout'; import type { LayoutComponent } from 'src/layout/LayoutComponent'; @@ -1250,6 +1251,12 @@ export const NodesInternal = { }), useHasErrors: () => Store.useSelector((s) => s.hasErrors), + // Raw selectors, used when there are no other hooks that match your needs + useSelector: (selector: (state: NodesContext) => T) => Store.useSelector(selector), + useShallowSelector: (selector: (state: NodesContext) => T) => + Store.useShallowSelector(selector), + useMemoSelector: (selector: (state: NodesContext) => T) => Store.useMemoSelector(selector), + useStore: () => Store.useStore(), useSetNodeProps: () => Store.useStaticSelector((s) => s.setNodeProps), useAddPage: () => Store.useStaticSelector((s) => s.addPage), From 70a77e505b2a78fd2a59eaa713b9dc2d87c1bf64 Mon Sep 17 00:00:00 2001 From: Ole Martin Handeland Date: Thu, 13 Feb 2025 15:27:42 +0100 Subject: [PATCH 02/21] Adding depth and parentId to node state, so that more traversal can be rewritten to be selectors --- src/codegen/ComponentConfig.ts | 2 ++ src/utils/layout/generator/NodeGenerator.tsx | 4 ++++ src/utils/layout/types.ts | 4 ++++ 3 files changed, 10 insertions(+) diff --git a/src/codegen/ComponentConfig.ts b/src/codegen/ComponentConfig.ts index 6f31b76209..e5d2c298a2 100644 --- a/src/codegen/ComponentConfig.ts +++ b/src/codegen/ComponentConfig.ts @@ -463,6 +463,8 @@ export class ComponentConfig { const baseState: ${BaseNodeData}<'${this.type}'> = { type: 'node', pageKey: props.pageKey, + parentId: props.parentId, + depth: props.depth, item: undefined, layout: props.item, hidden: undefined, diff --git a/src/utils/layout/generator/NodeGenerator.tsx b/src/utils/layout/generator/NodeGenerator.tsx index 3d1affcdc9..5716b423cc 100644 --- a/src/utils/layout/generator/NodeGenerator.tsx +++ b/src/utils/layout/generator/NodeGenerator.tsx @@ -22,6 +22,7 @@ import { } from 'src/utils/layout/generator/GeneratorStages'; import { useEvalExpressionInGenerator } from 'src/utils/layout/generator/useEvalExpression'; import { NodePropertiesValidation } from 'src/utils/layout/generator/validation/NodePropertiesValidation'; +import { BaseLayoutNode } from 'src/utils/layout/LayoutNode'; import { NodesInternal } from 'src/utils/layout/NodesContext'; import type { SimpleEval } from 'src/features/expressions'; import type { ExprConfig, ExprResolved, ExprValToActual, ExprValToActualOrExpr } from 'src/features/expressions/types'; @@ -110,6 +111,7 @@ function MarkAsHidden({ node, externalItem }: CommonProps({ node, intermediateItem }: CommonProps) { const parent = GeneratorInternal.useParent()!; + const depth = GeneratorInternal.useDepth(); const rowIndex = GeneratorInternal.useRowIndex(); const pageKey = GeneratorInternal.usePage()?.pageKey ?? ''; const idMutators = GeneratorInternal.useIdMutators() ?? []; @@ -118,6 +120,8 @@ function AddRemoveNode({ node, intermediateItem }: CommonPr const stateFactoryProps = { item: intermediateItem, parent, + parentId: parent instanceof BaseLayoutNode ? parent.id : undefined, + depth, rowIndex, pageKey, idMutators, diff --git a/src/utils/layout/types.ts b/src/utils/layout/types.ts index fa1970bfcb..0783600174 100644 --- a/src/utils/layout/types.ts +++ b/src/utils/layout/types.ts @@ -27,6 +27,8 @@ export interface StateFactoryProps { item: CompIntermediateExact; pageKey: string; parent: LayoutNode | LayoutPage; + parentId: string | undefined; + depth: number; rowIndex: number | undefined; idMutators: ChildIdMutator[]; layoutMap: Record; @@ -41,6 +43,8 @@ export interface GeneratorErrors { export interface BaseNodeData { type: 'node'; pageKey: string; + parentId: string | undefined; // String if parent is a node, undefined if parent is a page (on the top level) + depth: number; layout: CompIntermediate; item: CompInternal | undefined; hidden: boolean | undefined; From d1a8b0c57222239d9e294615c7976e2ce512b71f Mon Sep 17 00:00:00 2001 From: Ole Martin Handeland Date: Thu, 13 Feb 2025 16:32:02 +0100 Subject: [PATCH 03/21] Removing more traversal --- .../NodeInspector/NodeInspector.tsx | 12 +++++++----- src/features/pdf/PdfView2.tsx | 19 ++++++++++--------- .../Summary/LargeLikertSummaryContainer.tsx | 5 ++--- .../Summary/LargeGroupSummaryContainer.tsx | 5 ++--- 4 files changed, 21 insertions(+), 20 deletions(-) diff --git a/src/features/devtools/components/NodeInspector/NodeInspector.tsx b/src/features/devtools/components/NodeInspector/NodeInspector.tsx index 5c1ecb621f..17eb6383ab 100644 --- a/src/features/devtools/components/NodeInspector/NodeInspector.tsx +++ b/src/features/devtools/components/NodeInspector/NodeInspector.tsx @@ -13,15 +13,17 @@ import { SplitView } from 'src/features/devtools/components/SplitView/SplitView' import { useDevToolsStore } from 'src/features/devtools/data/DevToolsStore'; import { useCurrentView } from 'src/hooks/useNavigatePage'; import { implementsAnyValidation } from 'src/layout'; -import { useGetPage, useNode } from 'src/utils/layout/NodesContext'; -import { useNodeTraversal } from 'src/utils/layout/useNodeTraversal'; +import { NodesInternal, useNode } from 'src/utils/layout/NodesContext'; export const NodeInspector = () => { const pageKey = useCurrentView(); - const currentPage = useGetPage(pageKey); const selectedId = useDevToolsStore((state) => state.nodeInspector.selectedNodeId); const selectedNode = useNode(selectedId); - const children = useNodeTraversal((t) => (currentPage ? t.with(currentPage).children() : undefined)); + const children = NodesInternal.useShallowSelector((state) => + Object.values(state.nodeData || {}) + .filter((data) => data.pageKey === pageKey && data.parentId === undefined) // Find top-level nodes + .map((data) => data.layout.id), + ); const setSelected = useDevToolsStore((state) => state.actions.nodeInspectorSet); const focusLayoutInspector = useDevToolsStore((state) => state.actions.focusLayoutInspector); @@ -32,7 +34,7 @@ export const NodeInspector = () => { >
c.id) ?? []} + nodeIds={children?.map((id) => id) ?? []} selected={selectedId} onClick={setSelected} /> diff --git a/src/features/pdf/PdfView2.tsx b/src/features/pdf/PdfView2.tsx index c4e4c2a244..76ce6539c4 100644 --- a/src/features/pdf/PdfView2.tsx +++ b/src/features/pdf/PdfView2.tsx @@ -20,7 +20,7 @@ import classes from 'src/features/pdf/PDFView.module.css'; import { usePdfFormatQuery } from 'src/features/pdf/usePdfFormatQuery'; import { getFeature } from 'src/features/toggles'; import { usePageOrder } from 'src/hooks/useNavigatePage'; -import { GenericComponent } from 'src/layout/GenericComponent'; +import { GenericComponentById } from 'src/layout/GenericComponent'; import { InstanceInformation } from 'src/layout/InstanceInformation/InstanceInformationComponent'; import { SubformSummaryComponent2 } from 'src/layout/Subform/Summary/SubformSummaryComponent2'; import { SummaryComponent } from 'src/layout/Summary/SummaryComponent'; @@ -195,10 +195,11 @@ function PdfWrapping({ children }: PropsWithChildren) { } function PlainPage({ pageKey }: { pageKey: string }) { - const children = useNodeTraversal((t) => { - const page = t.findPage(pageKey); - return page ? t.with(page).children() : []; - }); + const children = NodesInternal.useShallowSelector((state) => + Object.values(state.nodeData || {}) + .filter((data) => data.pageKey === pageKey && data.parentId === undefined) // Find top-level nodes + .map((data) => data.layout.id), + ); return (
@@ -207,10 +208,10 @@ function PlainPage({ pageKey }: { pageKey: string }) { spacing={6} alignItems='flex-start' > - {children.map((node) => ( - ( + ))} diff --git a/src/layout/Likert/Summary/LargeLikertSummaryContainer.tsx b/src/layout/Likert/Summary/LargeLikertSummaryContainer.tsx index 931fb87b09..8dbf634861 100644 --- a/src/layout/Likert/Summary/LargeLikertSummaryContainer.tsx +++ b/src/layout/Likert/Summary/LargeLikertSummaryContainer.tsx @@ -6,9 +6,8 @@ import { Heading } from '@digdir/designsystemet-react'; import { Fieldset } from 'src/app-components/Label/Fieldset'; import { Lang } from 'src/features/language/Lang'; import classes from 'src/layout/Likert/Summary/LikertSummaryComponent.module.css'; -import { Hidden } from 'src/utils/layout/NodesContext'; +import { Hidden, NodesInternal } from 'src/utils/layout/NodesContext'; import { useNodeDirectChildren, useNodeItem } from 'src/utils/layout/useNodeItem'; -import { useNodeTraversal } from 'src/utils/layout/useNodeTraversal'; import type { HeadingLevel } from 'src/layout/common.generated'; import type { LayoutNode } from 'src/utils/layout/LayoutNode'; import type { TraversalRestriction } from 'src/utils/layout/useNodeTraversal'; @@ -39,7 +38,7 @@ export function LargeLikertSummaryContainer({ const container = useNodeItem(groupNode); const { title, summaryTitle } = container.textResourceBindings ?? {}; const isHidden = Hidden.useIsHidden(groupNode); - const depth = useNodeTraversal((t) => t.parents().length, groupNode); + const depth = NodesInternal.useSelector((state) => state.nodeData?.[groupNode.id]?.depth); const children = useNodeDirectChildren(groupNode, restriction); if (isHidden) { diff --git a/src/layout/RepeatingGroup/Summary/LargeGroupSummaryContainer.tsx b/src/layout/RepeatingGroup/Summary/LargeGroupSummaryContainer.tsx index 5e21328c8a..b25038f7c3 100644 --- a/src/layout/RepeatingGroup/Summary/LargeGroupSummaryContainer.tsx +++ b/src/layout/RepeatingGroup/Summary/LargeGroupSummaryContainer.tsx @@ -8,9 +8,8 @@ import { Lang } from 'src/features/language/Lang'; import classes from 'src/layout/RepeatingGroup/Summary/LargeGroupSummaryContainer.module.css'; import { pageBreakStyles } from 'src/utils/formComponentUtils'; import { BaseLayoutNode } from 'src/utils/layout/LayoutNode'; -import { Hidden } from 'src/utils/layout/NodesContext'; +import { Hidden, NodesInternal } from 'src/utils/layout/NodesContext'; import { useNodeDirectChildren, useNodeItem } from 'src/utils/layout/useNodeItem'; -import { useNodeTraversal } from 'src/utils/layout/useNodeTraversal'; import type { HeadingLevel } from 'src/layout/common.generated'; import type { LayoutNode } from 'src/utils/layout/LayoutNode'; import type { TraversalRestriction } from 'src/utils/layout/useNodeTraversal'; @@ -33,7 +32,7 @@ const headingSizes: { [k in HeadingLevel]: Parameters[0]['size'] export function LargeGroupSummaryContainer({ groupNode, id, restriction, renderLayoutNode }: IDisplayRepAsLargeGroup) { const item = useNodeItem(groupNode); const isHidden = Hidden.useIsHidden(groupNode); - const depth = useNodeTraversal((t) => t.parents().length, groupNode); + const depth = NodesInternal.useSelector((state) => state.nodeData?.[groupNode.id]?.depth); const children = useNodeDirectChildren(groupNode, restriction); if (isHidden) { return null; From bccc461f2c6d6f447cd1650c80416aead4832fc2 Mon Sep 17 00:00:00 2001 From: Ole Martin Handeland Date: Thu, 13 Feb 2025 16:45:53 +0100 Subject: [PATCH 04/21] Rewriting node traversal in Form.tsx --- src/components/form/Form.tsx | 104 +++++++++++++---------------------- src/utils/formLayout.ts | 24 -------- 2 files changed, 38 insertions(+), 90 deletions(-) diff --git a/src/components/form/Form.tsx b/src/components/form/Form.tsx index 080eed0e94..1f58762340 100644 --- a/src/components/form/Form.tsx +++ b/src/components/form/Form.tsx @@ -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'; @@ -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>[]; taskErrors: BaseValidation<'error'>[]; @@ -54,14 +53,7 @@ export function FormPage({ currentPageId }: { currentPageId: string | undefined const appName = useAppName(); const appOwner = useAppOwner(); const { langAsString } = useLanguage(); - const [formState, setFormState] = useState({ - hasRequired: false, - mainIds: undefined, - errorReportIds: [], - formErrors: [], - taskErrors: [], - }); - const { hasRequired, mainIds, errorReportIds, formErrors, taskErrors } = formState; + const { hasRequired, mainIds, errorReportIds, formErrors, taskErrors } = useFormState(); const requiredFieldsMissing = NodesInternal.usePageHasVisibleRequiredValidations(currentPageId); useRedirectToStoredPage(); @@ -86,15 +78,6 @@ export function FormPage({ currentPageId }: { currentPageId: string | undefined return ; } - if (mainIds === undefined) { - return ( - <> - - - - ); - } - const hasSetCurrentPageId = langAsString(currentPageId) !== currentPageId; if (!hasSetCurrentPageId) { @@ -114,7 +97,6 @@ export function FormPage({ currentPageId }: { currentPageId: string | undefined {`${getPageTitle(appName, hasSetCurrentPageId ? langAsString(currentPageId) : undefined, appOwner)}`} - {hasRequired && ( >; -} 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) { +function useFormState(): FormState { const currentPageId = useCurrentView(); - const page = useGetPage(currentPageId); - const traversalSelector = useNodeTraversalSelector(); + 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() { diff --git a/src/utils/formLayout.ts b/src/utils/formLayout.ts index 83a569d41c..8d666d6741 100644 --- a/src/utils/formLayout.ts +++ b/src/utils/formLayout.ts @@ -1,8 +1,6 @@ import { layoutSetIsDefault } from 'src/features/form/layoutSets/TypeGuards'; -import { getComponentCapabilities } from 'src/layout'; import type { ILayoutSets } from 'src/layout/common.generated'; import type { ILikertFilter } from 'src/layout/Likert/config.generated'; -import type { LayoutNode } from 'src/utils/layout/LayoutNode'; export const getLikertStartStopIndex = (lastIndex: number, filters: ILikertFilter = []) => { if (typeof lastIndex === 'undefined') { @@ -23,28 +21,6 @@ export const getLikertStartStopIndex = (lastIndex: number, filters: ILikertFilte return { startIndex, stopIndex: boundedStopIndex }; }; -/** - * Takes a layout and splits it into two return parts; the last will contain - * all the buttons on the bottom of the input layout, while the first returned - * value is the input layout except for these extracted components. - */ -export function extractBottomButtons(topLevelNodes: LayoutNode[]) { - const all = [...topLevelNodes]; - const toMainLayout: string[] = []; - const toErrorReport: string[] = []; - for (const node of all.reverse()) { - const capabilities = getComponentCapabilities(node.type); - const isButtonLike = node.isType('ButtonGroup') || (capabilities.renderInButtonGroup && !node.isType('Custom')); - if (isButtonLike && toMainLayout.length === 0) { - toErrorReport.push(node.id); - } else { - toMainLayout.push(node.id); - } - } - - return [toMainLayout.reverse(), toErrorReport.reverse()]; -} - /** * Some tasks other than data (for instance confirm, or other in the future) can be configured to behave like data steps * @param task the task From 2293143c5e885e6b1bfccd355102bcc184b8c10a Mon Sep 17 00:00:00 2001 From: Ole Martin Handeland Date: Fri, 14 Feb 2025 10:17:14 +0100 Subject: [PATCH 05/21] Rewriting more node traversal. This one is a bit risky, as nodes.allNodes() returned every node in pages in pageOrder, but nodeData included these implicitly hidden nodes. Removing generation of these nodes might break some use-cases, and I'll have to look into that more closely. --- src/codegen/ComponentConfig.ts | 5 +- src/components/form/Form.tsx | 4 +- .../ExpressionPlayground.tsx | 2 +- .../NodeInspector/NodeInspector.tsx | 2 +- src/features/pdf/PdfView2.tsx | 4 +- .../validation/ValidationStorePlugin.tsx | 103 ++++++++++-------- .../callbacks/onPageNavigationValidation.ts | 69 ++++++------ src/layout/Cards/CardsPlugin.tsx | 3 +- src/layout/Grid/GridRowsPlugin.tsx | 3 +- .../Likert/Generator/LikertRowsPlugin.tsx | 3 +- src/layout/RepeatingGroup/index.tsx | 12 +- .../Summary/SubformSummaryComponent2.tsx | 2 +- src/layout/Tabs/TabsPlugin.tsx | 3 +- src/utils/layout/NodesContext.tsx | 25 ++--- .../layout/generator/LayoutSetGenerator.tsx | 4 +- src/utils/layout/plugins/NodeDefPlugin.tsx | 3 +- .../plugins/NonRepeatingChildrenPlugin.tsx | 3 +- .../plugins/RepeatingChildrenPlugin.tsx | 3 +- 18 files changed, 127 insertions(+), 126 deletions(-) diff --git a/src/codegen/ComponentConfig.ts b/src/codegen/ComponentConfig.ts index e5d2c298a2..c9278f4125 100644 --- a/src/codegen/ComponentConfig.ts +++ b/src/codegen/ComponentConfig.ts @@ -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({ @@ -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( @@ -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); }`, ); diff --git a/src/components/form/Form.tsx b/src/components/form/Form.tsx index 1f58762340..d3e9b0c6f5 100644 --- a/src/components/form/Form.tsx +++ b/src/components/form/Form.tsx @@ -207,7 +207,7 @@ function useFormState(): FormState { const hasErrors = Boolean(formErrors.length) || Boolean(taskErrors.length); const [mainIds, errorReportIds] = NodesInternal.useMemoSelector((state) => { - const topLevelNodeData = Object.values(state.nodeData || {}).filter( + const topLevelNodeData = Object.values(state.nodeData).filter( (node) => node.pageKey === currentPageId && node.parentId === undefined, ); @@ -233,7 +233,7 @@ function useFormState(): FormState { }); const hasRequired = NodesInternal.useSelector((state) => - Object.values(state.nodeData || {}).some((node) => node.pageKey === currentPageId && nodeDataIsRequired(node)), + Object.values(state.nodeData).some((node) => node.pageKey === currentPageId && nodeDataIsRequired(node)), ); return { diff --git a/src/features/devtools/components/ExpressionPlayground/ExpressionPlayground.tsx b/src/features/devtools/components/ExpressionPlayground/ExpressionPlayground.tsx index 87c7378701..9968be5e32 100644 --- a/src/features/devtools/components/ExpressionPlayground/ExpressionPlayground.tsx +++ b/src/features/devtools/components/ExpressionPlayground/ExpressionPlayground.tsx @@ -77,7 +77,7 @@ export const ExpressionPlayground = () => { const traversalSelector = useNodeTraversalSelector(); const componentOptions = NodesInternal.useShallowSelector((state) => - Object.values(state.nodeData || {}).map((nodeData) => ({ + Object.values(state.nodeData).map((nodeData) => ({ label: nodeData.layout.id, value: `${nodeData.pageKey}|${nodeData.layout.id}`, })), diff --git a/src/features/devtools/components/NodeInspector/NodeInspector.tsx b/src/features/devtools/components/NodeInspector/NodeInspector.tsx index 17eb6383ab..98a5ae2284 100644 --- a/src/features/devtools/components/NodeInspector/NodeInspector.tsx +++ b/src/features/devtools/components/NodeInspector/NodeInspector.tsx @@ -20,7 +20,7 @@ export const NodeInspector = () => { const selectedId = useDevToolsStore((state) => state.nodeInspector.selectedNodeId); const selectedNode = useNode(selectedId); const children = NodesInternal.useShallowSelector((state) => - Object.values(state.nodeData || {}) + Object.values(state.nodeData) .filter((data) => data.pageKey === pageKey && data.parentId === undefined) // Find top-level nodes .map((data) => data.layout.id), ); diff --git a/src/features/pdf/PdfView2.tsx b/src/features/pdf/PdfView2.tsx index 76ce6539c4..b4889e4fbe 100644 --- a/src/features/pdf/PdfView2.tsx +++ b/src/features/pdf/PdfView2.tsx @@ -86,7 +86,7 @@ export const PDFView2 = () => { export function DataLoaderStoreInit({ children }: PropsWithChildren) { const subformIds = NodesInternal.useShallowSelector((state) => - Object.values(state.nodeData || {}) + Object.values(state.nodeData) .filter((node) => node.layout.type === 'Subform') .map((node) => node.layout.id), ); @@ -196,7 +196,7 @@ function PdfWrapping({ children }: PropsWithChildren) { function PlainPage({ pageKey }: { pageKey: string }) { const children = NodesInternal.useShallowSelector((state) => - Object.values(state.nodeData || {}) + Object.values(state.nodeData) .filter((data) => data.pageKey === pageKey && data.parentId === undefined) // Find top-level nodes .map((data) => data.layout.id), ); diff --git a/src/features/validation/ValidationStorePlugin.tsx b/src/features/validation/ValidationStorePlugin.tsx index 496c72f62c..ce604b6b8e 100644 --- a/src/features/validation/ValidationStorePlugin.tsx +++ b/src/features/validation/ValidationStorePlugin.tsx @@ -3,9 +3,8 @@ import { useCallback } from 'react'; import { ContextNotProvided } from 'src/core/contexts/context'; import { FrontendValidationSource, ValidationMask } from 'src/features/validation/index'; import { getInitialMaskFromNodeItem, selectValidations } from 'src/features/validation/utils'; -import { isHidden, nodesProduce, useNodesLax } from 'src/utils/layout/NodesContext'; +import { isHidden, nodesProduce } from 'src/utils/layout/NodesContext'; import { NodeDataPlugin } from 'src/utils/layout/plugins/NodeDataPlugin'; -import { TraversalTask } from 'src/utils/layout/useNodeTraversal'; import type { AnyValidation, AttachmentValidation, @@ -19,7 +18,7 @@ import type { NodeDataPluginSetState } from 'src/utils/layout/plugins/NodeDataPl import type { TraversalRestriction } from 'src/utils/layout/useNodeTraversal'; export type ValidationsSelector = ( - node: LayoutNode, + nodeOrId: LayoutNode | string, mask: NodeVisibility, severity?: ValidationSeverity, includeHidden?: boolean, // Defaults to false @@ -134,40 +133,49 @@ export class ValidationStorePlugin extends NodeDataPlugin store.useMemoSelector((state) => { if (!node) { return emptyArray; } - return getRecursiveValidations({ state, node, mask, severity, includeSelf, restriction }); + return getRecursiveValidations({ state, id: node.id, mask, severity, includeSelf, restriction }); }), useValidationsSelector: () => store.useDelayedSelector({ mode: 'simple', selector: - (node: LayoutNode, mask: NodeVisibility, severity?: ValidationSeverity, includeHidden: boolean = false) => + ( + nodeOrId: LayoutNode | string, + mask: NodeVisibility, + severity?: ValidationSeverity, + includeHidden: boolean = false, + ) => (state: NodesContext) => - getValidations({ state, node, mask, severity, includeHidden }), + getValidations({ + state, + id: typeof nodeOrId === 'string' ? nodeOrId : nodeOrId.id, + mask, + severity, + includeHidden, + }), }) satisfies ValidationsSelector, - useAllValidations: (mask, severity, includeHidden) => { - const nodes = useNodesLax(); - return store.useLaxMemoSelector((state) => { + useAllValidations: (mask, severity, includeHidden) => + store.useLaxMemoSelector((state) => { const out: NodeRefValidation[] = []; - for (const node of nodes?.allNodes() ?? []) { - const validations = getValidations({ state, node, mask, severity, includeHidden }); + for (const nodeData of Object.values(state.nodeData)) { + const id = nodeData.layout.id; + const validations = getValidations({ state, id, mask, severity, includeHidden }); for (const validation of validations) { - out.push({ ...validation, nodeId: node.id }); + out.push({ ...validation, nodeId: id }); } } return out; - }); - }, + }), useGetNodesWithErrors: () => { const zustand = store.useLaxStore(); - const nodes = useNodesLax(); return useCallback( (mask, severity, includeHidden = false) => { if (zustand === ContextNotProvided) { @@ -180,32 +188,31 @@ export class ValidationStorePlugin extends NodeDataPlugin 0) { - outNodes.push(node.id); + outNodes.push(id); outValidations.push(...validations); } } return [outNodes, outValidations]; }, - [nodes, zustand], + [zustand], ); }, - usePageHasVisibleRequiredValidations: (pageKey) => { - const nodes = useNodesLax(); - return store.useSelector((state) => { + usePageHasVisibleRequiredValidations: (pageKey) => + store.useSelector((state) => { if (!pageKey) { return false; } - for (const node of nodes?.allNodes() ?? []) { - const nodeData = state.nodeData[node.id]; + for (const nodeData of Object.values(state.nodeData)) { if (!nodeData || nodeData.pageKey !== pageKey) { continue; } - const validations = getValidations({ state, node, mask: 'visible', severity: 'error' }); + const id = nodeData.layout.id; + const validations = getValidations({ state, id, mask: 'visible', severity: 'error' }); for (const validation of validations) { if (validation.source === FrontendValidationSource.EmptyField) { return true; @@ -214,28 +221,26 @@ export class ValidationStorePlugin extends NodeDataPlugin nodeData.parentId === props.id && (!props.restriction || props.restriction === nodeData.rowIndex), + ) + .map((nodeData) => nodeData.layout.id); - // Restriction and includeSelf should only be applied to the first level (not recursively) - restriction: undefined, - includeSelf: true, - }), - ); - } + for (const id of children) { + out.push( + ...getRecursiveValidations({ + ...props, + id, + + // Restriction and includeSelf should only be applied to the first level (not recursively) + restriction: undefined, + includeSelf: true, + }), + ); } return out; diff --git a/src/features/validation/callbacks/onPageNavigationValidation.ts b/src/features/validation/callbacks/onPageNavigationValidation.ts index 8810fa4af6..78c5975d17 100644 --- a/src/features/validation/callbacks/onPageNavigationValidation.ts +++ b/src/features/validation/callbacks/onPageNavigationValidation.ts @@ -6,9 +6,7 @@ import { Validation } from 'src/features/validation/validationContext'; import { useEffectEvent } from 'src/hooks/useEffectEvent'; import { usePageOrder } from 'src/hooks/useNavigatePage'; import { NodesInternal } from 'src/utils/layout/NodesContext'; -import { useNodeTraversalSelector } from 'src/utils/layout/useNodeTraversal'; import type { PageValidation } from 'src/layout/common.generated'; -import type { LayoutNode } from 'src/utils/layout/LayoutNode'; import type { LayoutPage } from 'src/utils/layout/LayoutPage'; /** @@ -21,61 +19,64 @@ export function useOnPageNavigationValidation() { const getNodeValidations = NodesInternal.useValidationsSelector(); const validating = Validation.useValidating(); const pageOrder = usePageOrder(); - const traversalSelector = useNodeTraversalSelector(); + const nodeStore = NodesInternal.useStore(); const refetchInitialValidations = useRefetchInitialValidations(); /* Ensures the callback will have the latest state */ const callback = useEffectEvent(async (currentPage: LayoutPage, config: PageValidation): Promise => { const pageConfig = config.page ?? 'current'; const masks = config.show; - const mask = getVisibilityMask(masks); - let nodes: LayoutNode[] = []; - const currentIndex = pageOrder.indexOf(currentPage.pageKey); + if (!pageOrder || currentIndex === -1) { + return false; + } + const currentOrPreviousPages = new Set(); + for (const pageKey of pageOrder.slice(0, currentIndex + 1)) { + currentOrPreviousPages.add(pageKey); + } + + const state = nodeStore.getState(); + const nodeIds: string[] = []; + const nodesOnCurrentOrPreviousPages = new Set(); + let hasSubform = false; + + let shouldCheckPage: (pageKey: string) => boolean = () => true; // Defaults to all pages if (pageConfig === 'current') { - // Get nodes for current page - nodes = traversalSelector((t) => t.with(currentPage).flat(), [currentPage]); + shouldCheckPage = (pageKey: string) => pageKey === currentPage.pageKey; } else if (pageConfig === 'currentAndPrevious') { - // Get nodes for current and previous pages - if (!pageOrder || currentIndex === -1) { - return false; + shouldCheckPage = (pageKey: string) => currentOrPreviousPages.has(pageKey); + } + + for (const nodeData of Object.values(state.nodeData)) { + if (currentOrPreviousPages.has(nodeData.pageKey)) { + nodesOnCurrentOrPreviousPages.add(nodeData.layout.id); } - const pageKeysToCheck = pageOrder.slice(0, currentIndex + 1); - nodes = traversalSelector( - (t) => { - const out: LayoutNode[] = []; - for (const key of pageKeysToCheck) { - const page = t.findPage(key); - if (page) { - out.push(...t.with(page).flat()); - } - } - return out; - }, - [...pageKeysToCheck], - ); - } else { - // Get all nodes - nodes = traversalSelector((t) => t.flat(), []); + if (!shouldCheckPage(nodeData.pageKey)) { + continue; + } + if (nodeData.layout.type === 'Subform') { + hasSubform = true; + } + nodeIds.push(nodeData.layout.id); } // We need to get updated validations from backend to validate subform - if (nodes.some((n) => n.isType('Subform'))) { + if (hasSubform) { await refetchInitialValidations(); await validating(); } // Get nodes with errors along with their errors let onCurrentOrPreviousPage = false; - const nodeErrors = nodes - .map((n) => { - const validations = getNodeValidations(n, mask, 'error'); + const nodeErrors = nodeIds + .map((id) => { + const validations = getNodeValidations(id, mask, 'error'); if (validations.length > 0) { - onCurrentOrPreviousPage = onCurrentOrPreviousPage || pageOrder.indexOf(n.pageKey) <= currentIndex; + onCurrentOrPreviousPage = onCurrentOrPreviousPage || nodesOnCurrentOrPreviousPages.has(id); } - return [n, validations.length > 0] as const; + return [id, validations.length > 0] as const; }) .filter(([_, e]) => e); diff --git a/src/layout/Cards/CardsPlugin.tsx b/src/layout/Cards/CardsPlugin.tsx index 96bf22cd63..b5ac770cc6 100644 --- a/src/layout/Cards/CardsPlugin.tsx +++ b/src/layout/Cards/CardsPlugin.tsx @@ -4,7 +4,6 @@ import { NodeDefPlugin } from 'src/utils/layout/plugins/NodeDefPlugin'; import type { ComponentConfig } from 'src/codegen/ComponentConfig'; import type { CardConfigExternal } from 'src/layout/Cards/config.generated'; import type { CompTypes } from 'src/layout/layout'; -import type { LayoutNode } from 'src/utils/layout/LayoutNode'; import type { DefPluginChildClaimerProps, DefPluginExtraInItem, @@ -138,7 +137,7 @@ export class CardsPlugin return out; } - isChildHidden(_state: DefPluginState>, _childNode: LayoutNode): boolean { + isChildHidden(_state: DefPluginState>, _childId: string): boolean { return false; } } diff --git a/src/layout/Grid/GridRowsPlugin.tsx b/src/layout/Grid/GridRowsPlugin.tsx index 8443a2c3d3..e192b62aa9 100644 --- a/src/layout/Grid/GridRowsPlugin.tsx +++ b/src/layout/Grid/GridRowsPlugin.tsx @@ -5,7 +5,6 @@ import type { ComponentConfig } from 'src/codegen/ComponentConfig'; import type { GridRows } from 'src/layout/common.generated'; import type { GridCellInternal, GridRowsInternal } from 'src/layout/Grid/types'; import type { CompTypes, TypesFromCategory } from 'src/layout/layout'; -import type { LayoutNode } from 'src/utils/layout/LayoutNode'; import type { DefPluginChildClaimerProps, DefPluginExprResolver, @@ -188,7 +187,7 @@ export class GridRowsPlugin return out; } - isChildHidden(_state: DefPluginState>, _childNode: LayoutNode): boolean { + isChildHidden(_state: DefPluginState>, _childId: string): boolean { // There are no specific rules for hiding components in a Grid (yet). This should be implemented if we // add support for hiding a row or a cell (which should also hide the component inside) return false; diff --git a/src/layout/Likert/Generator/LikertRowsPlugin.tsx b/src/layout/Likert/Generator/LikertRowsPlugin.tsx index 653a98f3f3..d4ca122a89 100644 --- a/src/layout/Likert/Generator/LikertRowsPlugin.tsx +++ b/src/layout/Likert/Generator/LikertRowsPlugin.tsx @@ -3,7 +3,6 @@ import { NodeDefPlugin } from 'src/utils/layout/plugins/NodeDefPlugin'; import { typedBoolean } from 'src/utils/typing'; import type { ComponentConfig } from 'src/codegen/ComponentConfig'; import type { IDataModelBindingsLikert } from 'src/layout/common.generated'; -import type { LayoutNode } from 'src/utils/layout/LayoutNode'; import type { NodesContext } from 'src/utils/layout/NodesContext'; import type { DefPluginChildClaimerProps, @@ -79,7 +78,7 @@ export class LikertRowsPlugin extends NodeDefPlugin implements NodeDefCh return state.item?.rows.map((row) => row?.itemNodeId).filter(typedBoolean) ?? []; } - isChildHidden(_state: DefPluginState, _childNode: LayoutNode): boolean { + isChildHidden(_state: DefPluginState, _childId: string): boolean { return false; } diff --git a/src/layout/RepeatingGroup/index.tsx b/src/layout/RepeatingGroup/index.tsx index f17fa831db..9c81d439e0 100644 --- a/src/layout/RepeatingGroup/index.tsx +++ b/src/layout/RepeatingGroup/index.tsx @@ -10,6 +10,7 @@ import { RepeatingGroupProvider } from 'src/layout/RepeatingGroup/Providers/Repe import { RepeatingGroupsFocusProvider } from 'src/layout/RepeatingGroup/Providers/RepeatingGroupFocusContext'; import { SummaryRepeatingGroup } from 'src/layout/RepeatingGroup/Summary/SummaryRepeatingGroup'; import { RepeatingGroupSummary } from 'src/layout/RepeatingGroup/Summary2/RepeatingGroupSummary'; +import { splitDashedKey } from 'src/utils/splitDashedKey'; import type { LayoutValidationCtx } from 'src/features/devtools/layoutValidation/types'; import type { BaseValidation, ComponentValidation, ValidationDataSources } from 'src/features/validation'; import type { ExprResolver, SummaryRendererProps } from 'src/layout/LayoutComponent'; @@ -166,20 +167,21 @@ export class RepeatingGroup extends RepeatingGroupDef implements ValidateCompone return []; } - isChildHidden(state: NodeData<'RepeatingGroup'>, childNode: LayoutNode): boolean { - const hiddenByPlugins = super.isChildHidden(state, childNode); + isChildHidden(state: NodeData<'RepeatingGroup'>, childId: string): boolean { + const hiddenByPlugins = super.isChildHidden(state, childId); if (hiddenByPlugins) { return true; } - const row = childNode.rowIndex !== undefined ? state.item?.rows[childNode.rowIndex] : undefined; + const { baseComponentId, depth } = splitDashedKey(childId); + const rowIndex = depth.at(-1); + const row = rowIndex !== undefined ? state.item?.rows[rowIndex] : undefined; const rowHidden = row?.groupExpressions?.hiddenRow; if (rowHidden) { return true; } - const baseId = childNode.baseId; - const tableColSetup = state.item?.tableColumns?.[baseId]; + const tableColSetup = state.item?.tableColumns?.[baseComponentId]; const mode = state.item?.edit?.mode; // This specific configuration hides the component fully, without having set hidden=true on the component itself. diff --git a/src/layout/Subform/Summary/SubformSummaryComponent2.tsx b/src/layout/Subform/Summary/SubformSummaryComponent2.tsx index c04b4298d7..cecfc50e3e 100644 --- a/src/layout/Subform/Summary/SubformSummaryComponent2.tsx +++ b/src/layout/Subform/Summary/SubformSummaryComponent2.tsx @@ -123,7 +123,7 @@ export function SubformSummaryComponent2({ componentNode?: LayoutNode<'Subform'>; }) { const allOrOneSubformId = NodesInternal.useShallowSelector((state) => - Object.values(state.nodeData ?? {}) + Object.values(state.nodeData) .filter((data) => data.layout.type === 'Subform') .filter((data) => { if (!subformId) { diff --git a/src/layout/Tabs/TabsPlugin.tsx b/src/layout/Tabs/TabsPlugin.tsx index 4c71a64a5e..2037286dca 100644 --- a/src/layout/Tabs/TabsPlugin.tsx +++ b/src/layout/Tabs/TabsPlugin.tsx @@ -4,7 +4,6 @@ import { NodeDefPlugin } from 'src/utils/layout/plugins/NodeDefPlugin'; import type { ComponentConfig } from 'src/codegen/ComponentConfig'; import type { CompTypes } from 'src/layout/layout'; import type { TabConfig } from 'src/layout/Tabs/config.generated'; -import type { LayoutNode } from 'src/utils/layout/LayoutNode'; import type { DefPluginChildClaimerProps, DefPluginExtraInItem, @@ -117,7 +116,7 @@ export class TabsPlugin return out; } - isChildHidden(_state: DefPluginState>, _childNode: LayoutNode): boolean { + isChildHidden(_state: DefPluginState>, _childId: string): boolean { return false; } } diff --git a/src/utils/layout/NodesContext.tsx b/src/utils/layout/NodesContext.tsx index 861aedd8f7..2f9fa0b290 100644 --- a/src/utils/layout/NodesContext.tsx +++ b/src/utils/layout/NodesContext.tsx @@ -840,7 +840,6 @@ function isHiddenPage(state: NodesContext, page: LayoutPage | string | undefined export function isHidden( state: NodesContext, nodeOrId: LayoutNode | LayoutPage | undefined | string, - nodes: LayoutPages, _options?: IsHiddenOptions, ): boolean | undefined { if (!nodeOrId) { @@ -857,7 +856,6 @@ export function isHidden( } const id = typeof nodeOrId === 'string' ? nodeOrId : nodeOrId.id; - const node = nodes.findById(id); const hidden = state.nodeData[id]?.hidden; if (hidden === undefined) { return undefined; @@ -871,16 +869,18 @@ export function isHidden( return true; } - const parent = node?.parent; - if (parent && parent instanceof BaseLayoutNode && 'isChildHidden' in parent.def && state.nodeData[parent.id]) { + const parentId = state.nodeData[id]?.parentId; + const parent = parentId ? state.nodeData[parentId] : undefined; + const parentDef = parent ? getComponentDef(parent.layout.type) : undefined; + if (parent && parentDef && 'isChildHidden' in parentDef) { // eslint-disable-next-line @typescript-eslint/no-explicit-any - const childHidden = parent.def.isChildHidden(state.nodeData[parent.id] as any, node); + const childHidden = parentDef.isChildHidden(parent as any, id); if (childHidden) { return true; } } - return isHidden(state, parent, nodes, options); + return isHidden(state, parent?.layout.id, options); } function makeOptions(forcedVisibleByDevTools: boolean, options?: AccessibleIsHiddenOptions): IsHiddenOptions { @@ -894,8 +894,7 @@ export type IsHiddenSelector = ReturnType; export const Hidden = { useIsHidden(node: LayoutNode | LayoutPage | undefined, options?: AccessibleIsHiddenOptions) { const forcedVisibleByDevTools = GeneratorData.useIsForcedVisibleByDevTools(); - const nodes = useNodes(); - return WhenReady.useSelector((s) => isHidden(s, node, nodes, makeOptions(forcedVisibleByDevTools, options))); + return WhenReady.useSelector((s) => isHidden(s, node, makeOptions(forcedVisibleByDevTools, options))); }, useIsHiddenPage(page: LayoutPage | string | undefined, options?: AccessibleIsHiddenOptions) { const forcedVisibleByDevTools = GeneratorData.useIsForcedVisibleByDevTools(); @@ -921,26 +920,24 @@ export const Hidden = { }, useIsHiddenSelector() { const forcedVisibleByDevTools = GeneratorData.useIsForcedVisibleByDevTools(); - const nodes = useNodes(); return Store.useDelayedSelector( { mode: 'simple', selector: (node: LayoutNode | LayoutPage | string, options?: IsHiddenOptions) => (state) => - isHidden(state, node, nodes, makeOptions(forcedVisibleByDevTools, options)), + isHidden(state, node, makeOptions(forcedVisibleByDevTools, options)), }, - [forcedVisibleByDevTools, nodes], + [forcedVisibleByDevTools], ); }, useIsHiddenSelectorProps() { const forcedVisibleByDevTools = GeneratorData.useIsForcedVisibleByDevTools(); - const nodes = useNodes(); return Store.useDelayedSelectorProps( { mode: 'simple', selector: (node: LayoutNode | LayoutPage, options?: IsHiddenOptions) => (state) => - isHidden(state, node, nodes, makeOptions(forcedVisibleByDevTools, options)), + isHidden(state, node, makeOptions(forcedVisibleByDevTools, options)), }, - [forcedVisibleByDevTools, nodes], + [forcedVisibleByDevTools], ); }, diff --git a/src/utils/layout/generator/LayoutSetGenerator.tsx b/src/utils/layout/generator/LayoutSetGenerator.tsx index ce39fda425..57706a18c4 100644 --- a/src/utils/layout/generator/LayoutSetGenerator.tsx +++ b/src/utils/layout/generator/LayoutSetGenerator.tsx @@ -2,6 +2,7 @@ import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { ExprVal } from 'src/features/expressions/types'; import { useHiddenLayoutsExpressions } from 'src/features/form/layout/LayoutsContext'; +import { useLayoutSettings } from 'src/features/form/layoutSettings/LayoutSettingsContext'; import { getComponentCapabilities, getComponentDef } from 'src/layout'; import { ContainerComponent } from 'src/layout/LayoutComponent'; import { NodesStateQueue } from 'src/utils/layout/generator/CommitQueue'; @@ -43,6 +44,7 @@ interface ChildrenState { export function LayoutSetGenerator() { const layouts = GeneratorInternal.useLayouts(); + const pageOrder = useLayoutSettings().pages.order; const pages = useNodes(); const children = ( @@ -53,7 +55,7 @@ export function LayoutSetGenerator() { Object.keys(layouts).map((key) => { const layout = layouts[key]; - if (!layout) { + if (!layout || !pageOrder.includes(key)) { return null; } diff --git a/src/utils/layout/plugins/NodeDefPlugin.tsx b/src/utils/layout/plugins/NodeDefPlugin.tsx index 204ce6e63e..b172fe7bac 100644 --- a/src/utils/layout/plugins/NodeDefPlugin.tsx +++ b/src/utils/layout/plugins/NodeDefPlugin.tsx @@ -4,7 +4,6 @@ import type { GenerateImportedSymbol } from 'src/codegen/dataTypes/GenerateImpor import type { SerializableSetting } from 'src/codegen/SerializableSetting'; import type { CompInternal, CompTypes } from 'src/layout/layout'; import type { ChildClaimerProps, ExprResolver } from 'src/layout/LayoutComponent'; -import type { LayoutNode } from 'src/utils/layout/LayoutNode'; import type { NodesContext } from 'src/utils/layout/NodesContext'; import type { BaseNodeData, StateFactoryProps } from 'src/utils/layout/types'; import type { TraversalRestriction } from 'src/utils/layout/useNodeTraversal'; @@ -261,7 +260,7 @@ export abstract class NodeDefPlugin { export interface NodeDefChildrenPlugin { claimChildren(props: DefPluginChildClaimerProps): void; pickDirectChildren(state: DefPluginState, restriction?: TraversalRestriction): string[]; - isChildHidden(state: DefPluginState, childNode: LayoutNode): boolean; + isChildHidden(state: DefPluginState, childId: string): boolean; } // eslint-disable-next-line @typescript-eslint/no-explicit-any diff --git a/src/utils/layout/plugins/NonRepeatingChildrenPlugin.tsx b/src/utils/layout/plugins/NonRepeatingChildrenPlugin.tsx index c307a16d99..5d221d9584 100644 --- a/src/utils/layout/plugins/NonRepeatingChildrenPlugin.tsx +++ b/src/utils/layout/plugins/NonRepeatingChildrenPlugin.tsx @@ -4,7 +4,6 @@ import { NodeDefPlugin } from 'src/utils/layout/plugins/NodeDefPlugin'; import type { ComponentConfig } from 'src/codegen/ComponentConfig'; import type { CompCapabilities } from 'src/codegen/Config'; import type { TypesFromCategory } from 'src/layout/layout'; -import type { LayoutNode } from 'src/utils/layout/LayoutNode'; import type { DefPluginChildClaimerProps, DefPluginExtraInItem, @@ -158,7 +157,7 @@ export class NonRepeatingChildrenPlugin return state.item?.[this.settings.internalProp] || []; } - isChildHidden(_state: DefPluginState>, _childNode: LayoutNode): boolean { + isChildHidden(_state: DefPluginState>, _childId: string): boolean { return false; } } diff --git a/src/utils/layout/plugins/RepeatingChildrenPlugin.tsx b/src/utils/layout/plugins/RepeatingChildrenPlugin.tsx index cea7a1c7bb..7c457d074d 100644 --- a/src/utils/layout/plugins/RepeatingChildrenPlugin.tsx +++ b/src/utils/layout/plugins/RepeatingChildrenPlugin.tsx @@ -7,7 +7,6 @@ import type { ComponentConfig } from 'src/codegen/ComponentConfig'; import type { GenerateImportedSymbol } from 'src/codegen/dataTypes/GenerateImportedSymbol'; import type { TypesFromCategory } from 'src/layout/layout'; import type { ChildIdMutator } from 'src/utils/layout/generator/GeneratorContext'; -import type { LayoutNode } from 'src/utils/layout/LayoutNode'; import type { NodesContext } from 'src/utils/layout/NodesContext'; import type { DefPluginChildClaimerProps, @@ -236,7 +235,7 @@ export class RepeatingChildrenPlugin>, _childNode: LayoutNode): boolean { + isChildHidden(_state: DefPluginState>, _childId: string): boolean { // Repeating children plugins do not have any specific logic here, but beware that // the RepeatingGroup component does. return false; From 699e49bd6031db3c36cff5e888f7ede89987056a Mon Sep 17 00:00:00 2001 From: Ole Martin Handeland Date: Fri, 14 Feb 2025 13:24:00 +0100 Subject: [PATCH 06/21] Fixing some things I broke when checking for hidden components --- .../validation/ValidationStorePlugin.tsx | 2 +- src/utils/layout/NodesContext.tsx | 63 ++++++++++++------- 2 files changed, 41 insertions(+), 24 deletions(-) diff --git a/src/features/validation/ValidationStorePlugin.tsx b/src/features/validation/ValidationStorePlugin.tsx index ce604b6b8e..83811dc9e6 100644 --- a/src/features/validation/ValidationStorePlugin.tsx +++ b/src/features/validation/ValidationStorePlugin.tsx @@ -240,7 +240,7 @@ function getValidations({ state, id, mask, severity, includeHidden = false }: Ge return emptyArray; } - if (!includeHidden && isHidden(state, id, hiddenOptions)) { + if (!includeHidden && isHidden(state, 'node', id, hiddenOptions)) { return emptyArray; } diff --git a/src/utils/layout/NodesContext.tsx b/src/utils/layout/NodesContext.tsx index 2f9fa0b290..d72f149632 100644 --- a/src/utils/layout/NodesContext.tsx +++ b/src/utils/layout/NodesContext.tsx @@ -817,9 +817,9 @@ function withDefaults(options?: IsHiddenOptions): Required { return { respectDevTools, respectTracks, forcedVisibleByDevTools }; } -function isHiddenPage(state: NodesContext, page: LayoutPage | string | undefined, _options?: IsHiddenOptions) { +function isHiddenPage(state: NodesContext, pageKey: string | undefined, _options?: IsHiddenOptions) { const options = withDefaults(_options); - if (!page) { + if (!pageKey) { return true; } @@ -827,7 +827,6 @@ function isHiddenPage(state: NodesContext, page: LayoutPage | string | undefined return false; } - const pageKey = typeof page === 'string' ? page : page.pageKey; const pageState = state.pagesData.pages[pageKey]; const hidden = pageState?.hidden; if (hidden) { @@ -839,15 +838,21 @@ function isHiddenPage(state: NodesContext, page: LayoutPage | string | undefined export function isHidden( state: NodesContext, - nodeOrId: LayoutNode | LayoutPage | undefined | string, + type: 'page' | 'node', + id: string | undefined, _options?: IsHiddenOptions, ): boolean | undefined { - if (!nodeOrId) { + if (!id) { return undefined; } - if (nodeOrId instanceof LayoutPage) { - return isHiddenPage(state, nodeOrId, _options); + if (type === 'page') { + return isHiddenPage(state, id, _options); + } + + const pageKey = state.nodeData[id]?.pageKey; + if (pageKey && isHiddenPage(state, pageKey, _options)) { + return true; } const options = withDefaults(_options); @@ -855,14 +860,9 @@ export function isHidden( return false; } - const id = typeof nodeOrId === 'string' ? nodeOrId : nodeOrId.id; const hidden = state.nodeData[id]?.hidden; - if (hidden === undefined) { - return undefined; - } - - if (hidden) { - return true; + if (hidden === undefined || hidden === true) { + return hidden; } if (state.hiddenViaRules[id]) { @@ -880,7 +880,11 @@ export function isHidden( } } - return isHidden(state, parent?.layout.id, options); + if (parent) { + return isHidden(state, 'node', parent?.layout.id, options); + } + + return false; } function makeOptions(forcedVisibleByDevTools: boolean, options?: AccessibleIsHiddenOptions): IsHiddenOptions { @@ -894,19 +898,26 @@ export type IsHiddenSelector = ReturnType; export const Hidden = { useIsHidden(node: LayoutNode | LayoutPage | undefined, options?: AccessibleIsHiddenOptions) { const forcedVisibleByDevTools = GeneratorData.useIsForcedVisibleByDevTools(); - return WhenReady.useSelector((s) => isHidden(s, node, makeOptions(forcedVisibleByDevTools, options))); + const type = node instanceof LayoutPage ? ('page' as const) : ('node' as const); + const id = node instanceof LayoutPage ? node.pageKey : node?.id; + return WhenReady.useSelector((s) => isHidden(s, type, id, makeOptions(forcedVisibleByDevTools, options))); }, useIsHiddenPage(page: LayoutPage | string | undefined, options?: AccessibleIsHiddenOptions) { const forcedVisibleByDevTools = GeneratorData.useIsForcedVisibleByDevTools(); - return WhenReady.useSelector((s) => isHiddenPage(s, page, makeOptions(forcedVisibleByDevTools, options))); + return WhenReady.useSelector((s) => { + const pageKey = page instanceof LayoutPage ? page.pageKey : page; + return isHiddenPage(s, pageKey, makeOptions(forcedVisibleByDevTools, options)); + }); }, useIsHiddenPageSelector() { const forcedVisibleByDevTools = GeneratorData.useIsForcedVisibleByDevTools(); return Store.useDelayedSelector( { mode: 'simple', - selector: (page: LayoutPage | string) => (state) => - isHiddenPage(state, page, makeOptions(forcedVisibleByDevTools)), + selector: (page: LayoutPage | string) => (state) => { + const pageKey = page instanceof LayoutPage ? page.pageKey : page; + return isHiddenPage(state, pageKey, makeOptions(forcedVisibleByDevTools)); + }, }, [forcedVisibleByDevTools], ); @@ -923,8 +934,11 @@ export const Hidden = { return Store.useDelayedSelector( { mode: 'simple', - selector: (node: LayoutNode | LayoutPage | string, options?: IsHiddenOptions) => (state) => - isHidden(state, node, makeOptions(forcedVisibleByDevTools, options)), + selector: (node: LayoutNode | LayoutPage | string, options?: IsHiddenOptions) => (state) => { + const type = node instanceof LayoutPage ? ('page' as const) : ('node' as const); + const id = node instanceof LayoutPage ? node.pageKey : typeof node === 'string' ? node : node?.id; + return isHidden(state, type, id, makeOptions(forcedVisibleByDevTools, options)); + }, }, [forcedVisibleByDevTools], ); @@ -934,8 +948,11 @@ export const Hidden = { return Store.useDelayedSelectorProps( { mode: 'simple', - selector: (node: LayoutNode | LayoutPage, options?: IsHiddenOptions) => (state) => - isHidden(state, node, makeOptions(forcedVisibleByDevTools, options)), + selector: (node: LayoutNode | LayoutPage | string, options?: IsHiddenOptions) => (state) => { + const type = node instanceof LayoutPage ? ('page' as const) : ('node' as const); + const id = node instanceof LayoutPage ? node.pageKey : typeof node === 'string' ? node : node?.id; + return isHidden(state, type, id, makeOptions(forcedVisibleByDevTools, options)); + }, }, [forcedVisibleByDevTools], ); From 863f7bb1618fd7407531a6c5ca430018f1fcafb1 Mon Sep 17 00:00:00 2001 From: Ole Martin Handeland Date: Fri, 14 Feb 2025 17:10:20 +0100 Subject: [PATCH 07/21] Whoops, FormPage didn't really care about the page key it was given. This might have been a bug for a while! --- src/components/form/Form.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/components/form/Form.tsx b/src/components/form/Form.tsx index d3e9b0c6f5..908d6c5ff8 100644 --- a/src/components/form/Form.tsx +++ b/src/components/form/Form.tsx @@ -53,7 +53,7 @@ export function FormPage({ currentPageId }: { currentPageId: string | undefined const appName = useAppName(); const appOwner = useAppOwner(); const { langAsString } = useLanguage(); - const { hasRequired, mainIds, errorReportIds, formErrors, taskErrors } = useFormState(); + const { hasRequired, mainIds, errorReportIds, formErrors, taskErrors } = useFormState(currentPageId); const requiredFieldsMissing = NodesInternal.usePageHasVisibleRequiredValidations(currentPageId); useRedirectToStoredPage(); @@ -201,8 +201,7 @@ function nodeDataIsRequired(n: NodeData) { return !!(item && 'required' in item && item.required === true); } -function useFormState(): FormState { - const currentPageId = useCurrentView(); +function useFormState(currentPageId: string | undefined): FormState { const { formErrors, taskErrors } = useTaskErrors(); const hasErrors = Boolean(formErrors.length) || Boolean(taskErrors.length); From cec1bcfac0dd806b7649f61a009f3ef921d4b966 Mon Sep 17 00:00:00 2001 From: Ole Martin Handeland Date: Fri, 14 Feb 2025 17:11:01 +0100 Subject: [PATCH 08/21] No point in reading layout settings twice --- src/utils/layout/NodesContext.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/utils/layout/NodesContext.tsx b/src/utils/layout/NodesContext.tsx index d72f149632..3c30435ba9 100644 --- a/src/utils/layout/NodesContext.tsx +++ b/src/utils/layout/NodesContext.tsx @@ -16,7 +16,7 @@ import { AttachmentsStorePlugin } from 'src/features/attachments/AttachmentsStor import { UpdateAttachmentsForCypress } from 'src/features/attachments/UpdateAttachmentsForCypress'; import { HiddenComponentsProvider } from 'src/features/form/dynamics/HiddenComponentsProvider'; import { useLayouts } from 'src/features/form/layout/LayoutsContext'; -import { useLaxLayoutSettings, useLayoutSettings } from 'src/features/form/layoutSettings/LayoutSettingsContext'; +import { useLayoutSettings } from 'src/features/form/layoutSettings/LayoutSettingsContext'; import { OptionsStorePlugin } from 'src/features/options/OptionsStorePlugin'; import { useIsCurrentView } from 'src/features/routing/AppRoutingContext'; import { ExpressionValidation } from 'src/features/validation/expressionValidation/ExpressionValidation'; @@ -963,9 +963,8 @@ export const Hidden = { */ useIsPageInOrder(pageKey: string) { const isCurrentView = useIsCurrentView(pageKey); - const maybeLayoutSettings = useLaxLayoutSettings(); - const orderWithHidden = maybeLayoutSettings === ContextNotProvided ? [] : maybeLayoutSettings.pages.order; const layoutSettings = useLayoutSettings(); + const orderWithHidden = layoutSettings.pages.order; if (isCurrentView) { // If this is the current view, then it's never hidden. This avoids settings fields as hidden when From a4dd94dc4efa0053987acb0c4f77b844022df361 Mon Sep 17 00:00:00 2001 From: Ole Martin Handeland Date: Fri, 14 Feb 2025 17:15:58 +0100 Subject: [PATCH 09/21] A layout is also valid if it's the pdf layout (and nodes should be generated) --- src/utils/layout/generator/LayoutSetGenerator.tsx | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/utils/layout/generator/LayoutSetGenerator.tsx b/src/utils/layout/generator/LayoutSetGenerator.tsx index 57706a18c4..8a3704da58 100644 --- a/src/utils/layout/generator/LayoutSetGenerator.tsx +++ b/src/utils/layout/generator/LayoutSetGenerator.tsx @@ -44,7 +44,9 @@ interface ChildrenState { export function LayoutSetGenerator() { const layouts = GeneratorInternal.useLayouts(); - const pageOrder = useLayoutSettings().pages.order; + const layoutSettings = useLayoutSettings(); + const pageOrder = layoutSettings.pages.order; + const pdfPage = layoutSettings.pages.pdfLayoutName; const pages = useNodes(); const children = ( @@ -55,7 +57,12 @@ export function LayoutSetGenerator() { Object.keys(layouts).map((key) => { const layout = layouts[key]; - if (!layout || !pageOrder.includes(key)) { + if (!layout) { + return null; + } + + if (!pageOrder.includes(key) && key !== pdfPage) { + window.logErrorOnce(`Page '${key}' is not in the page order, and will be ignored`); return null; } From 8b148bb6cdf1f706d6dbdb9d476629c29b1f704f Mon Sep 17 00:00:00 2001 From: Ole Martin Handeland Date: Fri, 14 Feb 2025 18:51:58 +0100 Subject: [PATCH 10/21] Removing useEffectWhenReady() and replacing it with a simpler/more direct method. This failed hard _sometimes_ in the pdf.ts test, seemingly because the clearTimeout() that ran here failed hard when freezing clocks in cy.testPdf(). I have no idea what's going on, or why this suddenly happened now, but it seems like getting rid of more cruft fixes it. --- .../effects/EffectPreselectedOptionIndex.tsx | 15 ++++++--- .../effects/EffectRemoveStaleValues.tsx | 15 +++++++-- .../options/effects/EffectStoreLabel.tsx | 15 ++++++--- src/utils/layout/NodesContext.tsx | 31 +------------------ 4 files changed, 35 insertions(+), 41 deletions(-) diff --git a/src/features/options/effects/EffectPreselectedOptionIndex.tsx b/src/features/options/effects/EffectPreselectedOptionIndex.tsx index 279efd4634..f019907728 100644 --- a/src/features/options/effects/EffectPreselectedOptionIndex.tsx +++ b/src/features/options/effects/EffectPreselectedOptionIndex.tsx @@ -1,8 +1,8 @@ -import { useRef } from 'react'; +import { useEffect, useRef, useState } from 'react'; import { useSetOptions } from 'src/features/options/useGetOptions'; import { GeneratorInternal } from 'src/utils/layout/generator/GeneratorContext'; -import { Hidden, NodesInternal } from 'src/utils/layout/NodesContext'; +import { Hidden, NodesInternal, NodesReadiness } from 'src/utils/layout/NodesContext'; import type { IOptionInternal } from 'src/features/options/castOptionsToStrings'; import type { OptionsValueType } from 'src/features/options/useGetOptions'; import type { IDataModelBindingsOptionsSimple } from 'src/layout/common.generated'; @@ -21,6 +21,8 @@ interface Props { */ export function EffectPreselectedOptionIndex({ preselectedOption, valueType, options }: Props) { const node = GeneratorInternal.useParent() as LayoutNode>; + const nodeStore = NodesInternal.useStore(); + const [_, setForceUpdate] = useState(0); const isNodeHidden = Hidden.useIsHidden(node); const hasSelectedInitial = useRef(false); const item = GeneratorInternal.useIntermediateItem() as CompIntermediate>; @@ -30,12 +32,17 @@ export function EffectPreselectedOptionIndex({ preselectedOption, valueType, opt const shouldSelectOptionAutomatically = !hasValue && !hasSelectedInitial.current && preselectedOption !== undefined && isNodeHidden !== true; - NodesInternal.useEffectWhenReady(() => { + useEffect(() => { if (shouldSelectOptionAutomatically) { + if (nodeStore.getState().readiness !== NodesReadiness.Ready) { + requestAnimationFrame(() => setForceUpdate((prev) => prev + 1)); + return; + } + setData([preselectedOption.value]); hasSelectedInitial.current = true; } - }, [preselectedOption, shouldSelectOptionAutomatically, setData]); + }, [preselectedOption, shouldSelectOptionAutomatically, setData, nodeStore]); return null; } diff --git a/src/features/options/effects/EffectRemoveStaleValues.tsx b/src/features/options/effects/EffectRemoveStaleValues.tsx index 965877736e..86bfcecee5 100644 --- a/src/features/options/effects/EffectRemoveStaleValues.tsx +++ b/src/features/options/effects/EffectRemoveStaleValues.tsx @@ -1,9 +1,11 @@ +import { useEffect, useState } from 'react'; + import deepEqual from 'fast-deep-equal'; import { useSetOptions } from 'src/features/options/useGetOptions'; import { useAsRef } from 'src/hooks/useAsRef'; import { GeneratorInternal } from 'src/utils/layout/generator/GeneratorContext'; -import { Hidden, NodesInternal } from 'src/utils/layout/NodesContext'; +import { Hidden, NodesInternal, NodesReadiness } from 'src/utils/layout/NodesContext'; import type { IOptionInternal } from 'src/features/options/castOptionsToStrings'; import type { OptionsValueType } from 'src/features/options/useGetOptions'; import type { IDataModelBindingsOptionsSimple } from 'src/layout/common.generated'; @@ -24,6 +26,8 @@ interface Props { export function EffectRemoveStaleValues({ valueType, options }: Props) { const node = GeneratorInternal.useParent() as LayoutNode>; const isNodeHidden = Hidden.useIsHidden(node); + const nodeStore = NodesInternal.useStore(); + const [_, setForceUpdate] = useState(0); const item = GeneratorInternal.useIntermediateItem() as CompIntermediate>; const dataModelBindings = item.dataModelBindings as IDataModelBindingsOptionsSimple | undefined; @@ -32,7 +36,7 @@ export function EffectRemoveStaleValues({ valueType, options }: Props) { const optionsAsRef = useAsRef(options); const itemsToRemove = getItemsToRemove(options, setResult.unsafeSelectedValues); - NodesInternal.useEffectWhenReady(() => { + useEffect(() => { const { unsafeSelectedValues, setData } = setResultAsRef.current; const options = optionsAsRef.current; if (itemsToRemove.length === 0 || isNodeHidden || !options) { @@ -41,9 +45,14 @@ export function EffectRemoveStaleValues({ valueType, options }: Props) { const freshItemsToRemove = getItemsToRemove(optionsAsRef.current, unsafeSelectedValues); if (freshItemsToRemove.length > 0 && deepEqual(freshItemsToRemove, itemsToRemove)) { + if (nodeStore.getState().readiness !== NodesReadiness.Ready) { + requestAnimationFrame(() => setForceUpdate((prev) => prev + 1)); + return; + } + setData(unsafeSelectedValues.filter((v) => !itemsToRemove.includes(v))); } - }, [isNodeHidden, itemsToRemove, optionsAsRef, setResultAsRef]); + }, [isNodeHidden, itemsToRemove, nodeStore, optionsAsRef, setResultAsRef]); return null; } diff --git a/src/features/options/effects/EffectStoreLabel.tsx b/src/features/options/effects/EffectStoreLabel.tsx index 8d634d081f..13f4fde81b 100644 --- a/src/features/options/effects/EffectStoreLabel.tsx +++ b/src/features/options/effects/EffectStoreLabel.tsx @@ -1,4 +1,4 @@ -import { useMemo } from 'react'; +import { useEffect, useMemo, useState } from 'react'; import deepEqual from 'fast-deep-equal'; @@ -6,7 +6,7 @@ import { useDataModelBindings } from 'src/features/formData/useDataModelBindings import { useLanguage } from 'src/features/language/useLanguage'; import { useSetOptions } from 'src/features/options/useGetOptions'; import { GeneratorInternal } from 'src/utils/layout/generator/GeneratorContext'; -import { Hidden, NodesInternal } from 'src/utils/layout/NodesContext'; +import { Hidden, NodesInternal, NodesReadiness } from 'src/utils/layout/NodesContext'; import type { IOptionInternal } from 'src/features/options/castOptionsToStrings'; import type { OptionsValueType } from 'src/features/options/useGetOptions'; import type { IDataModelBindingsOptionsSimple } from 'src/layout/common.generated'; @@ -24,6 +24,8 @@ interface Props { export function EffectStoreLabel({ valueType, options }: Props) { const item = GeneratorInternal.useIntermediateItem() as CompIntermediate>; const node = GeneratorInternal.useParent() as LayoutNode>; + const nodeStore = NodesInternal.useStore(); + const [_, setForceUpdate] = useState(0); const isNodeHidden = Hidden.useIsHidden(node); const { langAsString } = useLanguage(); const dataModelBindings = item.dataModelBindings as IDataModelBindingsOptionsSimple | undefined; @@ -42,11 +44,16 @@ export function EffectStoreLabel({ valueType, options }: Props) { const labelsHaveChanged = !deepEqual(translatedLabels, 'label' in formData ? formData.label : undefined); const shouldSetData = labelsHaveChanged && !isNodeHidden && dataModelBindings && 'label' in dataModelBindings; - NodesInternal.useEffectWhenReady(() => { + useEffect(() => { if (!shouldSetData) { return; } + if (nodeStore.getState().readiness !== NodesReadiness.Ready) { + requestAnimationFrame(() => setForceUpdate((prev) => prev + 1)); + return; + } + if (!translatedLabels || translatedLabels.length === 0) { setValue('label', undefined); return; @@ -55,7 +62,7 @@ export function EffectStoreLabel({ valueType, options }: Props) { } else { setValue('label', translatedLabels); } - }, [setValue, shouldSetData, translatedLabels, valueType]); + }, [nodeStore, setValue, shouldSetData, translatedLabels, valueType]); return null; } diff --git a/src/utils/layout/NodesContext.tsx b/src/utils/layout/NodesContext.tsx index 3c30435ba9..2416e6a069 100644 --- a/src/utils/layout/NodesContext.tsx +++ b/src/utils/layout/NodesContext.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react'; +import React, { useCallback, useEffect, useLayoutEffect, useMemo, useRef } from 'react'; import type { MutableRefObject, PropsWithChildren } from 'react'; import deepEqual from 'fast-deep-equal'; @@ -1117,35 +1117,6 @@ export const NodesInternal = { [markReady], ); }, - /** - * Like a useEffect, but only runs the effect when the nodes context is ready. - */ - useEffectWhenReady(effect: Parameters[0], deps: Parameters[1]) { - const [force, setForceReRun] = useState(0); - const getAwaiting = useGetAwaitingCommits(); - const isReadyRef = NodesInternal.useIsReadyRef(); - const waitUntilReady = NodesInternal.useWaitUntilReady(); - - useEffect(() => { - const isReady = isReadyRef.current; - if (!isReady) { - waitUntilReady().then(() => { - // We need to force a rerender to run the effect. If we didn't, the effect would never run. - setForceReRun((v) => v + 1); - }); - return; - } - const awaiting = getAwaiting(); - if (awaiting) { - // If we are awaiting commits, we need to wait until they are done before we can run the effect. - const timeout = setTimeout(() => setForceReRun((v) => v + 1), 100); - return () => clearTimeout(timeout); - } - - return effect(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [force, ...(deps ?? [])]); - }, useFullErrorList() { return Store.useMemoSelector((s) => { From 2aedb79047ca9bba81598a1e5ad04f6de756a86e Mon Sep 17 00:00:00 2001 From: Ole Martin Handeland Date: Fri, 14 Feb 2025 18:52:40 +0100 Subject: [PATCH 11/21] Making sure these tests wait until preselectedOptionIndex etc is saved to the backend before it continues to test the pdf. --- test/e2e/integration/frontend-test/pdf.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/e2e/integration/frontend-test/pdf.ts b/test/e2e/integration/frontend-test/pdf.ts index 0cbc79339b..2f0439646c 100644 --- a/test/e2e/integration/frontend-test/pdf.ts +++ b/test/e2e/integration/frontend-test/pdf.ts @@ -308,6 +308,8 @@ describe('PDF', () => { }); cy.goto('changename'); + cy.get(appFrontend.changeOfName.newFirstName).should('be.visible'); + cy.waitUntilSaved(); cy.testPdf({ callback: () => { @@ -345,6 +347,8 @@ describe('PDF', () => { }); cy.goto('changename'); + cy.get(appFrontend.changeOfName.newFirstName).should('be.visible'); + cy.waitUntilSaved(); cy.testPdf({ callback: () => { From 40192f673367acc2e74e0e41e5b26dd12b3680b1 Mon Sep 17 00:00:00 2001 From: Ole Martin Handeland Date: Fri, 14 Feb 2025 19:00:22 +0100 Subject: [PATCH 12/21] Huh, there were more easy ones - I missed these. --- src/layout/Group/GroupComponent.tsx | 5 ++-- .../SummaryComponent2/PageSummary.tsx | 27 +++++++++++++------ 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/layout/Group/GroupComponent.tsx b/src/layout/Group/GroupComponent.tsx index 15e62ad696..dcc9796fe6 100644 --- a/src/layout/Group/GroupComponent.tsx +++ b/src/layout/Group/GroupComponent.tsx @@ -11,9 +11,8 @@ import { FullWidthWrapper } from 'src/components/form/FullWidthWrapper'; import { Lang } from 'src/features/language/Lang'; import classes from 'src/layout/Group/GroupComponent.module.css'; import { BaseLayoutNode } from 'src/utils/layout/LayoutNode'; -import { Hidden } from 'src/utils/layout/NodesContext'; +import { Hidden, NodesInternal } from 'src/utils/layout/NodesContext'; import { useNodeDirectChildren, useNodeItem } from 'src/utils/layout/useNodeItem'; -import { useNodeTraversal } from 'src/utils/layout/useNodeTraversal'; import type { HeadingLevel } from 'src/layout/common.generated'; import type { LayoutNode } from 'src/utils/layout/LayoutNode'; import type { TraversalRestriction } from 'src/utils/layout/useNodeTraversal'; @@ -48,7 +47,7 @@ export function GroupComponent({ const isHidden = Hidden.useIsHidden(groupNode); const children = useNodeDirectChildren(groupNode, restriction); - const depth = useNodeTraversal((traverser) => traverser.with(groupNode).parents().length); + const depth = NodesInternal.useSelector((state) => state.nodeData?.[groupNode.id]?.depth); if (isHidden) { return null; diff --git a/src/layout/Summary2/SummaryComponent2/PageSummary.tsx b/src/layout/Summary2/SummaryComponent2/PageSummary.tsx index 3cc99cbd07..a56926f680 100644 --- a/src/layout/Summary2/SummaryComponent2/PageSummary.tsx +++ b/src/layout/Summary2/SummaryComponent2/PageSummary.tsx @@ -1,9 +1,7 @@ import React from 'react'; import { ComponentSummary } from 'src/layout/Summary2/SummaryComponent2/ComponentSummary'; -import { Hidden, useGetPage } from 'src/utils/layout/NodesContext'; -import { useNodeTraversal } from 'src/utils/layout/useNodeTraversal'; -import { typedBoolean } from 'src/utils/typing'; +import { Hidden, NodesInternal, useGetPage, useNode } from 'src/utils/layout/NodesContext'; interface PageSummaryProps { pageId: string; @@ -11,7 +9,11 @@ interface PageSummaryProps { export function PageSummary({ pageId }: PageSummaryProps) { const page = useGetPage(pageId); - const children = useNodeTraversal((t) => (page ? t.with(page).children() : [])); + const children = NodesInternal.useShallowSelector((state) => + Object.values(state.nodeData) + .filter((nodeData) => nodeData.pageKey === pageId && nodeData.parentId === undefined) // Find top-level nodes + .map((nodeData) => nodeData.layout.id), + ); const isHiddenPage = Hidden.useIsHiddenPage(page); if (!page || !children) { @@ -22,10 +24,19 @@ export function PageSummary({ pageId }: PageSummaryProps) { return null; } - return children?.filter(typedBoolean).map((child, idx) => ( - ( + )); } + +function NodeSummary({ nodeId }: { nodeId: string }) { + const node = useNode(nodeId); + if (!node) { + return null; + } + + return ; +} From 91b17d941bf9780826af9cc9a112115044056605 Mon Sep 17 00:00:00 2001 From: Ole Martin Handeland Date: Fri, 14 Feb 2025 19:17:47 +0100 Subject: [PATCH 13/21] Oooh, sneaky! I'm so glad to have tests that catch me messing up a 0 !== undefined check --- src/features/validation/ValidationStorePlugin.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/features/validation/ValidationStorePlugin.tsx b/src/features/validation/ValidationStorePlugin.tsx index 83811dc9e6..6d18573ac5 100644 --- a/src/features/validation/ValidationStorePlugin.tsx +++ b/src/features/validation/ValidationStorePlugin.tsx @@ -273,7 +273,8 @@ function getRecursiveValidations(props: GetDeepValidationsProps): NodeRefValidat const children = Object.values(props.state.nodeData) .filter( - (nodeData) => nodeData.parentId === props.id && (!props.restriction || props.restriction === nodeData.rowIndex), + (nodeData) => + nodeData.parentId === props.id && (props.restriction === undefined || props.restriction === nodeData.rowIndex), ) .map((nodeData) => nodeData.layout.id); From 47308efad328aa364714d9f7c16b904219a7abd4 Mon Sep 17 00:00:00 2001 From: Ole Martin Handeland Date: Fri, 14 Feb 2025 19:44:11 +0100 Subject: [PATCH 14/21] Replacing a few more traversals --- src/features/pdf/PdfView2.tsx | 52 +++++++++++++++++------------ src/layout/LayoutComponent.tsx | 17 +++------- src/layout/Panel/PanelComponent.tsx | 24 +++++++++---- 3 files changed, 53 insertions(+), 40 deletions(-) diff --git a/src/features/pdf/PdfView2.tsx b/src/features/pdf/PdfView2.tsx index b4889e4fbe..3549803a67 100644 --- a/src/features/pdf/PdfView2.tsx +++ b/src/features/pdf/PdfView2.tsx @@ -20,17 +20,19 @@ import classes from 'src/features/pdf/PDFView.module.css'; import { usePdfFormatQuery } from 'src/features/pdf/usePdfFormatQuery'; import { getFeature } from 'src/features/toggles'; import { usePageOrder } from 'src/hooks/useNavigatePage'; +import { getComponentDef } from 'src/layout'; import { GenericComponentById } from 'src/layout/GenericComponent'; import { InstanceInformation } from 'src/layout/InstanceInformation/InstanceInformationComponent'; import { SubformSummaryComponent2 } from 'src/layout/Subform/Summary/SubformSummaryComponent2'; import { SummaryComponent } from 'src/layout/Summary/SummaryComponent'; import { ComponentSummary } from 'src/layout/Summary2/SummaryComponent2/ComponentSummary'; import { SummaryComponent2 } from 'src/layout/Summary2/SummaryComponent2/SummaryComponent2'; -import { Hidden, NodesInternal, useNode } from 'src/utils/layout/NodesContext'; +import { Hidden, isHidden, NodesInternal, useNode } from 'src/utils/layout/NodesContext'; import { useNodeItem } from 'src/utils/layout/useNodeItem'; -import { useNodeTraversal } from 'src/utils/layout/useNodeTraversal'; import type { IPdfFormat } from 'src/features/pdf/types'; +import type { CompTypes } from 'src/layout/layout'; import type { LayoutNode } from 'src/utils/layout/LayoutNode'; +import type { NodeData } from 'src/utils/layout/types'; export const PDFView2 = () => { const order = usePageOrder(); @@ -220,21 +222,23 @@ function PlainPage({ pageKey }: { pageKey: string }) { } function PdfForPage({ pageKey, pdfSettings }: { pageKey: string; pdfSettings: IPdfFormat | undefined }) { - const isHiddenSelector = Hidden.useIsHiddenSelector(); - const nodeDataSelector = NodesInternal.useNodeDataSelector(); - const children = useNodeTraversal((t) => { - const page = t.findPage(pageKey); - return page - ? t - .with(page) - .children() - .filter((node) => !node.isType('Subform')) - .filter((node) => !isHiddenSelector(node)) - .filter((node) => !pdfSettings?.excludedComponents.includes(node.id)) - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .filter((node) => node.def.shouldRenderInAutomaticPDF(node as any, nodeDataSelector)) - : []; - }); + const children = NodesInternal.useShallowSelector((state) => + Object.values(state.nodeData) + .filter( + (data) => + data.pageKey === pageKey && + data.parentId === undefined && + data.layout.type !== 'Subform' && + !isHidden(state, 'node', data.layout.id) && + !pdfSettings?.excludedComponents.includes(data.layout.id), + ) + .filter((data: NodeData) => { + const def = getComponentDef(data.layout.type); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return def.shouldRenderInAutomaticPDF(data as any); + }) + .map((data) => data.layout.id), + ); return (
@@ -243,10 +247,10 @@ function PdfForPage({ pageKey, pdfSettings }: { pageKey: string; pdfSettings: IP spacing={6} alignItems='flex-start' > - {children.map((node) => ( + {children.map((nodeId) => ( ))} @@ -254,8 +258,14 @@ function PdfForPage({ pageKey, pdfSettings }: { pageKey: string; pdfSettings: IP ); } -function PdfForNode({ node }: { node: LayoutNode }) { +function PdfForNode({ nodeId }: { nodeId: string }) { + const node = useNode(nodeId); const target = useNodeItem(node, (i) => (i.type === 'Summary2' ? i.target : undefined)); + + if (!node) { + return null; + } + if (node.isType('Summary2') && target?.taskId) { return ( { return false; } - shouldRenderInAutomaticPDF(node: LayoutNode, nodeDataSelector: NodeDataSelector): boolean { - const renderAsSummary = nodeDataSelector( - (picker) => { - const item = picker(node)?.item; - return item && 'renderAsSummary' in item ? item.renderAsSummary : false; - }, - [node], - ); - - return !renderAsSummary; + shouldRenderInAutomaticPDF(data: NodeData): boolean { + const item = data.item; + return !(item && 'renderAsSummary' in item ? item.renderAsSummary : false); } /** @@ -362,7 +355,7 @@ abstract class _FormComponent extends AnyComponent export abstract class ActionComponent extends AnyComponent { readonly category = CompCategory.Action; - shouldRenderInAutomaticPDF(_node: LayoutNode, _nodeDataSelector: NodeDataSelector): boolean { + shouldRenderInAutomaticPDF(_data: NodeData): boolean { return false; } } diff --git a/src/layout/Panel/PanelComponent.tsx b/src/layout/Panel/PanelComponent.tsx index 6a5bf6979d..5e65088ac5 100644 --- a/src/layout/Panel/PanelComponent.tsx +++ b/src/layout/Panel/PanelComponent.tsx @@ -7,8 +7,8 @@ import { FullWidthWrapper } from 'src/components/form/FullWidthWrapper'; import { Lang } from 'src/features/language/Lang'; import { ComponentStructureWrapper } from 'src/layout/ComponentStructureWrapper'; import { LayoutPage } from 'src/utils/layout/LayoutPage'; +import { NodesInternal } from 'src/utils/layout/NodesContext'; import { useNodeItem } from 'src/utils/layout/useNodeItem'; -import { useNodeTraversal } from 'src/utils/layout/useNodeTraversal'; import type { PropsFromGenericComponent } from 'src/layout'; type IPanelProps = PropsFromGenericComponent<'Panel'>; @@ -17,13 +17,23 @@ export const PanelComponent = ({ node }: IPanelProps) => { const { textResourceBindings, variant, showIcon, grid } = useNodeItem(node); const fullWidth = !grid && node.parent instanceof LayoutPage; - const { isOnBottom, isOnTop } = useNodeTraversal((t) => { - const parent = t.parents()[0]; - const children = t.with(parent).children(); - const isOnBottom = children.indexOf(node) === children.length - 1; - const isOnTop = children.indexOf(node) === 0; + const { isOnBottom, isOnTop } = NodesInternal.useShallowSelector((state) => { + let children: string[] = []; + if (node.parent instanceof LayoutPage) { + children = Object.values(state.nodeData) + .filter((n) => n.pageKey === node.parent.pageKey && n.parentId === undefined) + .map((n) => n.layout.id); + } else { + const parentId = node.parent.id; + children = Object.values(state.nodeData) + .filter((n) => n.parentId === parentId) + .map((n) => n.layout.id); + } + + const isOnBottom = children.indexOf(node.id) === children.length - 1; + const isOnTop = children.indexOf(node.id) === 0; return { isOnBottom, isOnTop }; - }, node); + }); if (!textResourceBindings?.body && !textResourceBindings?.title) { window.logWarn('Unable to render panel component: no text resource binding found.'); From 3c87105662ee687295ece24ce84ed8959e6043b7 Mon Sep 17 00:00:00 2001 From: Ole Martin Handeland Date: Sat, 15 Feb 2025 13:57:24 +0100 Subject: [PATCH 15/21] [no ci] Whoops, my simpler fix failed to re-run the useEffect, breaking preselectedOptionIndex among others --- src/features/options/effects/EffectPreselectedOptionIndex.tsx | 4 ++-- src/features/options/effects/EffectRemoveStaleValues.tsx | 4 ++-- src/features/options/effects/EffectStoreLabel.tsx | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/features/options/effects/EffectPreselectedOptionIndex.tsx b/src/features/options/effects/EffectPreselectedOptionIndex.tsx index f019907728..9d29e48675 100644 --- a/src/features/options/effects/EffectPreselectedOptionIndex.tsx +++ b/src/features/options/effects/EffectPreselectedOptionIndex.tsx @@ -22,7 +22,7 @@ interface Props { export function EffectPreselectedOptionIndex({ preselectedOption, valueType, options }: Props) { const node = GeneratorInternal.useParent() as LayoutNode>; const nodeStore = NodesInternal.useStore(); - const [_, setForceUpdate] = useState(0); + const [force, setForceUpdate] = useState(0); const isNodeHidden = Hidden.useIsHidden(node); const hasSelectedInitial = useRef(false); const item = GeneratorInternal.useIntermediateItem() as CompIntermediate>; @@ -42,7 +42,7 @@ export function EffectPreselectedOptionIndex({ preselectedOption, valueType, opt setData([preselectedOption.value]); hasSelectedInitial.current = true; } - }, [preselectedOption, shouldSelectOptionAutomatically, setData, nodeStore]); + }, [preselectedOption, shouldSelectOptionAutomatically, setData, nodeStore, node.id, force]); return null; } diff --git a/src/features/options/effects/EffectRemoveStaleValues.tsx b/src/features/options/effects/EffectRemoveStaleValues.tsx index 86bfcecee5..924361c3fe 100644 --- a/src/features/options/effects/EffectRemoveStaleValues.tsx +++ b/src/features/options/effects/EffectRemoveStaleValues.tsx @@ -27,7 +27,7 @@ export function EffectRemoveStaleValues({ valueType, options }: Props) { const node = GeneratorInternal.useParent() as LayoutNode>; const isNodeHidden = Hidden.useIsHidden(node); const nodeStore = NodesInternal.useStore(); - const [_, setForceUpdate] = useState(0); + const [force, setForceUpdate] = useState(0); const item = GeneratorInternal.useIntermediateItem() as CompIntermediate>; const dataModelBindings = item.dataModelBindings as IDataModelBindingsOptionsSimple | undefined; @@ -52,7 +52,7 @@ export function EffectRemoveStaleValues({ valueType, options }: Props) { setData(unsafeSelectedValues.filter((v) => !itemsToRemove.includes(v))); } - }, [isNodeHidden, itemsToRemove, nodeStore, optionsAsRef, setResultAsRef]); + }, [isNodeHidden, itemsToRemove, nodeStore, optionsAsRef, setResultAsRef, force]); return null; } diff --git a/src/features/options/effects/EffectStoreLabel.tsx b/src/features/options/effects/EffectStoreLabel.tsx index 13f4fde81b..2a15334964 100644 --- a/src/features/options/effects/EffectStoreLabel.tsx +++ b/src/features/options/effects/EffectStoreLabel.tsx @@ -25,7 +25,7 @@ export function EffectStoreLabel({ valueType, options }: Props) { const item = GeneratorInternal.useIntermediateItem() as CompIntermediate>; const node = GeneratorInternal.useParent() as LayoutNode>; const nodeStore = NodesInternal.useStore(); - const [_, setForceUpdate] = useState(0); + const [force, setForceUpdate] = useState(0); const isNodeHidden = Hidden.useIsHidden(node); const { langAsString } = useLanguage(); const dataModelBindings = item.dataModelBindings as IDataModelBindingsOptionsSimple | undefined; @@ -62,7 +62,7 @@ export function EffectStoreLabel({ valueType, options }: Props) { } else { setValue('label', translatedLabels); } - }, [nodeStore, setValue, shouldSetData, translatedLabels, valueType]); + }, [nodeStore, setValue, shouldSetData, translatedLabels, valueType, force]); return null; } From a8413f612e49185729e572e18160840a58ab2de5 Mon Sep 17 00:00:00 2001 From: Ole Martin Handeland Date: Mon, 17 Feb 2025 13:00:32 +0100 Subject: [PATCH 16/21] Instead of skipping node generation for implicitly hidden pages, mark them as 'isValid: false' instead, and use that info to not include them in validation selectors. --- src/codegen/ComponentConfig.ts | 1 + src/features/validation/ValidationStorePlugin.tsx | 2 +- src/utils/layout/generator/GeneratorContext.tsx | 4 +++- src/utils/layout/generator/LayoutSetGenerator.tsx | 14 ++++++-------- src/utils/layout/generator/NodeGenerator.tsx | 2 ++ src/utils/layout/types.ts | 2 ++ 6 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/codegen/ComponentConfig.ts b/src/codegen/ComponentConfig.ts index c9278f4125..e270e2df2d 100644 --- a/src/codegen/ComponentConfig.ts +++ b/src/codegen/ComponentConfig.ts @@ -464,6 +464,7 @@ export class ComponentConfig { pageKey: props.pageKey, parentId: props.parentId, depth: props.depth, + isValid: props.isValid, item: undefined, layout: props.item, hidden: undefined, diff --git a/src/features/validation/ValidationStorePlugin.tsx b/src/features/validation/ValidationStorePlugin.tsx index 6d18573ac5..6edc6fdc0e 100644 --- a/src/features/validation/ValidationStorePlugin.tsx +++ b/src/features/validation/ValidationStorePlugin.tsx @@ -236,7 +236,7 @@ interface GetValidationsProps { function getValidations({ state, id, mask, severity, includeHidden = false }: GetValidationsProps): AnyValidation[] { const nodeData = state.nodeData[id]; - if (!nodeData || !('validations' in nodeData) || !('validationVisibility' in nodeData)) { + if (!nodeData || !('validations' in nodeData) || !('validationVisibility' in nodeData) || !nodeData.isValid) { return emptyArray; } diff --git a/src/utils/layout/generator/GeneratorContext.tsx b/src/utils/layout/generator/GeneratorContext.tsx index 17dbdf6a3d..10a5f51c48 100644 --- a/src/utils/layout/generator/GeneratorContext.tsx +++ b/src/utils/layout/generator/GeneratorContext.tsx @@ -25,7 +25,7 @@ export interface ChildClaimsMap { type GlobalProviderProps = Pick; -type PageProviderProps = Pick & { +type PageProviderProps = Pick & { parent: LayoutPage; }; @@ -57,6 +57,7 @@ interface GeneratorContext { } | undefined; depth: number; // Depth is 1 for top level nodes, 2 for children of top level nodes, etc. + isValid?: boolean; // False when page is not in the page order, and not a pdf page (forwarded to nodes as well) } const { Provider, useCtx, useLaxCtx } = createContext({ @@ -188,4 +189,5 @@ export const GeneratorInternal = { useRowBinding: () => useCtx().row?.binding, useRowIndex: () => useCtx().row?.index, useIntermediateItem: () => useCtx().item, + useIsValid: () => useCtx().isValid ?? true, }; diff --git a/src/utils/layout/generator/LayoutSetGenerator.tsx b/src/utils/layout/generator/LayoutSetGenerator.tsx index 8a3704da58..a8d7a4bb08 100644 --- a/src/utils/layout/generator/LayoutSetGenerator.tsx +++ b/src/utils/layout/generator/LayoutSetGenerator.tsx @@ -44,9 +44,6 @@ interface ChildrenState { export function LayoutSetGenerator() { const layouts = GeneratorInternal.useLayouts(); - const layoutSettings = useLayoutSettings(); - const pageOrder = layoutSettings.pages.order; - const pdfPage = layoutSettings.pages.pdfLayoutName; const pages = useNodes(); const children = ( @@ -61,11 +58,6 @@ export function LayoutSetGenerator() { return null; } - if (!pageOrder.includes(key) && key !== pdfPage) { - window.logErrorOnce(`Page '${key}' is not in the page order, and will be ignored`); - return null; - } - return ( new LayoutPage(), []); useGeneratorErrorBoundaryNodeRef().current = page; + const layoutSettings = useLayoutSettings(); + const pageOrder = layoutSettings.pages.order; + const pdfPage = layoutSettings.pages.pdfLayoutName; + const isValid = pageOrder.includes(name) || name === pdfPage; + const getProto = useMemo(() => { const proto: { [id: string]: ComponentProto } = {}; @@ -227,6 +224,7 @@ function PageGenerator({ layout, name, layoutSet }: PageProps) { ({ node, intermediateItem }: CommonPr const pageKey = GeneratorInternal.usePage()?.pageKey ?? ''; const idMutators = GeneratorInternal.useIdMutators() ?? []; const layoutMap = GeneratorInternal.useLayoutMap(); + const isValid = GeneratorInternal.useIsValid(); const getCapabilities = (type: CompTypes) => getComponentCapabilities(type); const stateFactoryProps = { item: intermediateItem, @@ -127,6 +128,7 @@ function AddRemoveNode({ node, intermediateItem }: CommonPr idMutators, layoutMap, getCapabilities, + isValid, } satisfies StateFactoryProps; const isAdded = NodesInternal.useIsAdded(node); diff --git a/src/utils/layout/types.ts b/src/utils/layout/types.ts index 0783600174..6fff0643a9 100644 --- a/src/utils/layout/types.ts +++ b/src/utils/layout/types.ts @@ -33,6 +33,7 @@ export interface StateFactoryProps { idMutators: ChildIdMutator[]; layoutMap: Record; getCapabilities: (type: CompTypes) => CompCapabilities; + isValid: boolean; } export interface GeneratorErrors { @@ -44,6 +45,7 @@ export interface BaseNodeData { type: 'node'; pageKey: string; parentId: string | undefined; // String if parent is a node, undefined if parent is a page (on the top level) + isValid: boolean; // False when page is not in the page order, and not a pdf page depth: number; layout: CompIntermediate; item: CompInternal | undefined; From f4500fafaa0d0d0361902f2cdde01c2f35388bcc Mon Sep 17 00:00:00 2001 From: Ole Martin Handeland Date: Mon, 17 Feb 2025 14:57:10 +0100 Subject: [PATCH 17/21] Removing node traversal (and all the delayed selector usage!) from useAttachmentDeletionInRepGroups.ts --- .../attachments/AttachmentsStorePlugin.tsx | 101 ++++++++---------- src/features/attachments/hooks.ts | 4 +- .../useAttachmentDeletionInRepGroups.ts | 61 +++++++---- .../FileUpload/Error/FailedAttachments.tsx | 2 +- src/layout/FileUpload/FileUploadComponent.tsx | 4 +- .../FileUploadTable/FileTableButtons.tsx | 2 +- src/layout/FileUpload/index.tsx | 4 +- .../FileUploadWithTag/EditWindowComponent.tsx | 2 +- src/layout/FileUploadWithTag/index.tsx | 4 +- 9 files changed, 96 insertions(+), 88 deletions(-) diff --git a/src/features/attachments/AttachmentsStorePlugin.tsx b/src/features/attachments/AttachmentsStorePlugin.tsx index fc69dc9f97..2a9ab29878 100644 --- a/src/features/attachments/AttachmentsStorePlugin.tsx +++ b/src/features/attachments/AttachmentsStorePlugin.tsx @@ -25,11 +25,11 @@ import { backendValidationIssueGroupListToObject } from 'src/features/validation import { useWaitForState } from 'src/hooks/useWaitForState'; import { nodesProduce } from 'src/utils/layout/NodesContext'; import { NodeDataPlugin } from 'src/utils/layout/plugins/NodeDataPlugin'; +import { splitDashedKey } from 'src/utils/splitDashedKey'; import { isAtLeastVersion } from 'src/utils/versionCompare'; import type { ApplicationMetadata } from 'src/features/applicationMetadata/types'; import type { DataPostResponse, - FileUploaderNode, IAttachment, IAttachmentsMap, IFailedAttachment, @@ -42,7 +42,6 @@ import type { IDataModelBindingsList, IDataModelBindingsSimple } from 'src/layou import type { RejectedFileError } from 'src/layout/FileUpload/RejectedFileError'; import type { CompWithBehavior } from 'src/layout/layout'; import type { IData } from 'src/types/shared'; -import type { LayoutNode } from 'src/utils/layout/LayoutNode'; import type { NodesContext, NodesStoreFull } from 'src/utils/layout/NodesContext'; import type { NodeDataPluginSetState } from 'src/utils/layout/plugins/NodeDataPlugin'; import type { NodeData } from 'src/utils/layout/types'; @@ -74,29 +73,27 @@ export interface AttachmentActionUpload { temporaryId: string; file: File; }[]; - node: FileUploaderNode; + nodeId: string; dataModelBindings: IDataModelBindingsSimple | IDataModelBindingsList | undefined; } export interface AttachmentActionUpdate { tags: string[]; - node: FileUploaderNode; + nodeId: string; attachment: UploadedAttachment; } export interface AttachmentActionRemove { - node: FileUploaderNode; + nodeId: string; attachment: UploadedAttachment; dataModelBindings: IDataModelBindingsSimple | IDataModelBindingsList | undefined; } export interface AttachmentActionAddFailed { - node: FileUploaderNode; + nodeId: string; attachments: IFailedAttachment[]; } -export type AttachmentsSelector = (node: FileUploaderNode) => IAttachment[]; - export interface AttachmentsStorePluginConfig { extraFunctions: { attachmentUpload: (action: AttachmentActionUpload) => void; @@ -110,7 +107,7 @@ export interface AttachmentsStorePluginConfig { attachmentRemoveFulfilled: (action: AttachmentActionRemove) => void; attachmentRemoveRejected: (action: AttachmentActionRemove, error: AxiosError) => void; - deleteFailedAttachment: (node: FileUploaderNode, temporaryId: string) => void; + deleteFailedAttachment: (nodeId: string, temporaryId: string) => void; addFailedAttachments: (action: AttachmentActionAddFailed) => void; }; extraHooks: { @@ -121,14 +118,14 @@ export interface AttachmentsStorePluginConfig { ) => Promise; useAttachmentsUpdate: () => (action: AttachmentActionUpdate) => Promise; useAttachmentsRemove: () => (action: AttachmentActionRemove) => Promise; - useDeleteFailedAttachment: () => (node: FileUploaderNode, temporaryId: string) => void; - useAddRejectedAttachments: () => (node: FileUploaderNode, errors: RejectedFileError[]) => void; + useDeleteFailedAttachment: () => (nodeId: string, temporaryId: string) => void; + useAddRejectedAttachments: () => (nodeId: string, errors: RejectedFileError[]) => void; - useAttachments: (node: FileUploaderNode) => IAttachment[]; - useFailedAttachments: (node: FileUploaderNode) => IFailedAttachment[]; + useAttachments: (nodeId: string) => IAttachment[]; + useFailedAttachments: (nodeId: string) => IFailedAttachment[]; useAttachmentsSelector: () => AttachmentsSelector; useAttachmentsSelectorProps: () => DSPropsForSimpleSelector; - useWaitUntilUploaded: () => (node: FileUploaderNode, attachment: TemporaryAttachment) => Promise; + useWaitUntilUploaded: () => (nodeId: string, attachment: TemporaryAttachment) => Promise; useHasPendingAttachments: () => boolean; useAllAttachments: () => IAttachmentsMap; @@ -142,10 +139,10 @@ type ProperData = NodeData>; export class AttachmentsStorePlugin extends NodeDataPlugin { extraFunctions(set: NodeDataPluginSetState): AttachmentsStorePluginConfig['extraFunctions'] { return { - attachmentUpload: ({ files, node }) => { + attachmentUpload: ({ files, nodeId }) => { set( nodesProduce((draft) => { - const data = draft.nodeData[node.id] as ProperData; + const data = draft.nodeData[nodeId] as ProperData; for (const { file, temporaryId } of files) { data.attachments[temporaryId] = { uploaded: false, @@ -161,10 +158,10 @@ export class AttachmentsStorePlugin extends NodeDataPlugin { + attachmentUploadFinished: ({ nodeId }, results) => { set( nodesProduce((draft) => { - const nodeData = draft.nodeData[node.id] as ProperData; + const nodeData = draft.nodeData[nodeId] as ProperData; for (const result of results) { if (isAttachmentUploadSuccess(result)) { nodeData.attachments[result.newDataElementId] = nodeData.attachments[result.temporaryId]; @@ -179,10 +176,10 @@ export class AttachmentsStorePlugin extends NodeDataPlugin { + attachmentUpdate: ({ nodeId, attachment, tags }) => { set( nodesProduce((draft) => { - const nodeData = draft.nodeData[node.id] as ProperData; + const nodeData = draft.nodeData[nodeId] as ProperData; const attachmentData = nodeData.attachments[attachment.data.id]; if (isAttachmentUploaded(attachmentData)) { attachmentData.updating = true; @@ -193,10 +190,10 @@ export class AttachmentsStorePlugin extends NodeDataPlugin { + attachmentUpdateFulfilled: ({ nodeId, attachment }) => { set( nodesProduce((draft) => { - const nodeData = draft.nodeData[node.id] as ProperData; + const nodeData = draft.nodeData[nodeId] as ProperData; const attachmentData = nodeData.attachments[attachment.data.id]; if (isAttachmentUploaded(attachmentData)) { attachmentData.updating = false; @@ -206,10 +203,10 @@ export class AttachmentsStorePlugin extends NodeDataPlugin { + attachmentUpdateRejected: ({ nodeId, attachment }, error) => { set( nodesProduce((draft) => { - const nodeData = draft.nodeData[node.id] as ProperData; + const nodeData = draft.nodeData[nodeId] as ProperData; const attachmentData = nodeData.attachments[attachment.data.id]; if (isAttachmentUploaded(attachmentData)) { attachmentData.updating = false; @@ -220,10 +217,10 @@ export class AttachmentsStorePlugin extends NodeDataPlugin { + attachmentRemove: ({ nodeId, attachment }) => { set( nodesProduce((draft) => { - const nodeData = draft.nodeData[node.id] as ProperData; + const nodeData = draft.nodeData[nodeId] as ProperData; const attachmentData = nodeData.attachments[attachment.data.id]; if (isAttachmentUploaded(attachmentData)) { attachmentData.deleting = true; @@ -233,18 +230,18 @@ export class AttachmentsStorePlugin extends NodeDataPlugin { + attachmentRemoveFulfilled: ({ nodeId, attachment }) => { set( nodesProduce((draft) => { - const nodeData = draft.nodeData[node.id] as ProperData; + const nodeData = draft.nodeData[nodeId] as ProperData; delete nodeData.attachments[attachment.data.id]; }), ); }, - attachmentRemoveRejected: ({ node, attachment }, error) => { + attachmentRemoveRejected: ({ nodeId, attachment }, error) => { set( nodesProduce((draft) => { - const nodeData = draft.nodeData[node.id] as ProperData; + const nodeData = draft.nodeData[nodeId] as ProperData; const attachmentData = nodeData.attachments[attachment.data.id]; if (isAttachmentUploaded(attachmentData)) { attachmentData.deleting = false; @@ -255,19 +252,19 @@ export class AttachmentsStorePlugin extends NodeDataPlugin { + deleteFailedAttachment: (nodeId, temporaryId) => { set( nodesProduce((draft) => { - const nodeData = draft.nodeData[node.id] as ProperData; + const nodeData = draft.nodeData[nodeId] as ProperData; delete nodeData.attachmentsFailedToUpload[temporaryId]; }), ); }, - addFailedAttachments: ({ node, attachments }) => { + addFailedAttachments: ({ nodeId, attachments }) => { set( nodesProduce((draft) => { for (const { data, error } of attachments) { - const nodeData = draft.nodeData[node.id] as ProperData; + const nodeData = draft.nodeData[nodeId] as ProperData; nodeData.attachmentsFailedToUpload[data.temporaryId] = { data, error, @@ -309,8 +306,9 @@ export class AttachmentsStorePlugin extends NodeDataPlugin { @@ -337,10 +335,11 @@ export class AttachmentsStorePlugin extends NodeDataPlugin - uploadAttachmentOld({ dataTypeId: action.node.baseId, file }) + uploadAttachmentOld({ dataTypeId: baseComponentId, file }) .then((data) => { if (!data || !data.blobStoragePath) { return { temporaryId, error: new Error('Failed to upload attachment') }; @@ -454,13 +453,9 @@ export class AttachmentsStorePlugin extends NodeDataPlugin { - if (!node) { - return emptyArray; - } - - const nodeData = state.nodeData[node.id]; + const nodeData = state.nodeData[nodeId]; if (nodeData && 'attachments' in nodeData) { return Object.values(nodeData.attachments).sort(sortAttachmentsByName); } @@ -485,9 +480,9 @@ export class AttachmentsStorePlugin extends NodeDataPlugin(zustandStore); return useCallback( - (node, attachment) => + (nodeId, attachment) => waitFor((state, setReturnValue) => { - const nodeData = state.nodeData[node.id]; + const nodeData = state.nodeData[nodeId]; if (!nodeData || !('attachments' in nodeData) || !('attachmentsFailedToUpload' in nodeData)) { setReturnValue(false); return true; @@ -548,13 +543,9 @@ export class AttachmentsStorePlugin extends NodeDataPlugin { - if (!node) { - return emptyArray; - } - - const nodeData = state.nodeData[node.id]; + const nodeData = state.nodeData[nodeId]; if (nodeData && 'attachmentsFailedToUpload' in nodeData) { return Object.values(nodeData.attachmentsFailedToUpload).sort(sortAttachmentsByName); } @@ -568,7 +559,7 @@ export class AttachmentsStorePlugin extends NodeDataPlugin state.addFailedAttachments); return useCallback( - (node: FileUploaderNode, errors: RejectedFileError[]) => { + (nodeId: string, errors: RejectedFileError[]) => { const attachments: IFailedAttachment[] = errors.map((error) => ({ data: { temporaryId: uuidv4(), @@ -577,7 +568,7 @@ export class AttachmentsStorePlugin extends NodeDataPlugin (state: NodesContext) => { - const nodeData = state.nodeData[node.id]; + +export type AttachmentsSelector = (nodeId: string) => IAttachment[]; +export const attachmentSelector = (nodeId: string) => (state: NodesContext) => { + const nodeData = state.nodeData[nodeId]; if (!nodeData) { return emptyArray; } diff --git a/src/features/attachments/hooks.ts b/src/features/attachments/hooks.ts index d2d3a45af1..2cf7d7e191 100644 --- a/src/features/attachments/hooks.ts +++ b/src/features/attachments/hooks.ts @@ -8,8 +8,8 @@ export const useAttachmentsAwaiter = () => NodesInternal.useWaitUntilUploaded(); export const useAddRejectedAttachments = () => NodesInternal.useAddRejectedAttachments(); export const useDeleteFailedAttachment = () => NodesInternal.useDeleteFailedAttachment(); -export const useAttachmentsFor = (node: FileUploaderNode) => NodesInternal.useAttachments(node); -export const useFailedAttachmentsFor = (node: FileUploaderNode) => NodesInternal.useFailedAttachments(node); +export const useAttachmentsFor = (node: FileUploaderNode) => NodesInternal.useAttachments(node.id); +export const useFailedAttachmentsFor = (node: FileUploaderNode) => NodesInternal.useFailedAttachments(node.id); export const useAttachmentsSelector = () => NodesInternal.useAttachmentsSelector(); diff --git a/src/features/attachments/useAttachmentDeletionInRepGroups.ts b/src/features/attachments/useAttachmentDeletionInRepGroups.ts index 132bcfb5b8..67849bbc44 100644 --- a/src/features/attachments/useAttachmentDeletionInRepGroups.ts +++ b/src/features/attachments/useAttachmentDeletionInRepGroups.ts @@ -1,16 +1,17 @@ import { useCallback } from 'react'; import { AttachmentsPlugin } from 'src/features/attachments/AttachmentsPlugin'; -import { useAttachmentsAwaiter, useAttachmentsRemover, useAttachmentsSelector } from 'src/features/attachments/hooks'; +import { attachmentSelector } from 'src/features/attachments/AttachmentsStorePlugin'; +import { useAttachmentsAwaiter, useAttachmentsRemover } from 'src/features/attachments/hooks'; import { isAttachmentUploaded } from 'src/features/attachments/index'; import { useAsRef } from 'src/hooks/useAsRef'; +import { getComponentDef } from 'src/layout'; import { NodesInternal } from 'src/utils/layout/NodesContext'; -import { useNodeTraversalSelector } from 'src/utils/layout/useNodeTraversal'; +import type { CompWithPlugin, IDataModelBindings } from 'src/layout/layout'; import type { LayoutNode } from 'src/utils/layout/LayoutNode'; +import type { NodesContext } from 'src/utils/layout/NodesContext'; import type { TraversalRestriction } from 'src/utils/layout/useNodeTraversal'; -type UploaderNode = LayoutNode<'FileUpload' | 'FileUploadWithTag'>; - /** * When deleting a row in a repeating group, we need to find any attachments that are uploaded * in that row (or any of its children) and remove them from the instance. @@ -25,37 +26,43 @@ export function useAttachmentDeletionInRepGroups(node: LayoutNode<'RepeatingGrou const remove = useAsRef(useAttachmentsRemover()); const awaiter = useAttachmentsAwaiter(); const nodeRef = useAsRef(node); - const attachmentsSelector = useAttachmentsSelector(); - const traversalSelector = useNodeTraversalSelector(); - const nodeItemSelector = NodesInternal.useNodeDataSelector(); + const nodesStore = NodesInternal.useStore(); return useCallback( async (restriction: TraversalRestriction): Promise => { - const uploaders = traversalSelector( - (t) => - t - .with(nodeRef.current) - .flat(undefined, restriction) - .filter((n) => n.def.hasPlugin(AttachmentsPlugin)), - [nodeRef.current, restriction], - ) as UploaderNode[]; + const state = nodesStore.getState(); + const recursiveChildren = new Set(recursivelyFindChildren(nodeRef.current.id, state, restriction)); + const uploaderNodeIds = Object.values(state.nodeData) + .filter((n) => { + if (!recursiveChildren.has(n.layout.id)) { + return false; + } + const def = getComponentDef(n.layout.type); + return def && def.hasPlugin(AttachmentsPlugin); + }) + .map((n) => n.layout.id); // This code is intentionally not parallelized, as especially LocalTest can't handle parallel requests to // delete attachments. It might return a 500 if you try. To be safe, we do them one by one. - for (const uploader of uploaders) { - const files = attachmentsSelector(uploader); + for (const uploaderId of uploaderNodeIds) { + const nodeData = state.nodeData[uploaderId]; + const dataModelBindings = nodeData?.layout.dataModelBindings as IDataModelBindings< + CompWithPlugin + >; + + const files = attachmentSelector(uploaderId)(state); for (const file of files) { if (isAttachmentUploaded(file)) { const result = await remove.current({ attachment: file, - node: uploader, - dataModelBindings: nodeItemSelector((picker) => picker(uploader)?.layout.dataModelBindings, [uploader]), + nodeId: uploaderId, + dataModelBindings, }); if (!result) { return false; } } else { - const uploaded = await awaiter(uploader, file); + const uploaded = await awaiter(uploaderId, file); if (uploaded) { const result = await remove.current({ attachment: { @@ -64,8 +71,8 @@ export function useAttachmentDeletionInRepGroups(node: LayoutNode<'RepeatingGrou updating: false, data: uploaded, }, - node: uploader, - dataModelBindings: nodeItemSelector((picker) => picker(uploader)?.layout.dataModelBindings, [uploader]), + nodeId: uploaderId, + dataModelBindings, }); if (!result) { return false; @@ -80,6 +87,14 @@ export function useAttachmentDeletionInRepGroups(node: LayoutNode<'RepeatingGrou return true; }, - [traversalSelector, nodeRef, attachmentsSelector, remove, nodeItemSelector, awaiter], + [nodesStore, nodeRef, remove, awaiter], ); } + +function recursivelyFindChildren(parentId: string, state: NodesContext, restriction?: number): string[] { + const children = Object.values(state.nodeData) + .filter((n) => n.parentId === parentId && (restriction === undefined || n.rowIndex === restriction)) + .map((n) => n.layout.id); + + return [...children, ...children.flatMap((c) => recursivelyFindChildren(c, state, undefined))]; +} diff --git a/src/layout/FileUpload/Error/FailedAttachments.tsx b/src/layout/FileUpload/Error/FailedAttachments.tsx index a618063e6c..0cd842aeaf 100644 --- a/src/layout/FileUpload/Error/FailedAttachments.tsx +++ b/src/layout/FileUpload/Error/FailedAttachments.tsx @@ -23,7 +23,7 @@ export function FailedAttachments({ node }: { node: FileUploaderNode }) { deleteFailedAttachment(node, attachment.data.temporaryId)} + handleClose={() => deleteFailedAttachment(node.id, attachment.data.temporaryId)} /> ))}
diff --git a/src/layout/FileUpload/FileUploadComponent.tsx b/src/layout/FileUpload/FileUploadComponent.tsx index 90ae699d48..a5d37caf6c 100644 --- a/src/layout/FileUpload/FileUploadComponent.tsx +++ b/src/layout/FileUpload/FileUploadComponent.tsx @@ -79,7 +79,7 @@ export function FileUploadComponent({ node }: IFileUploadWithTagProps): React.JS return; } // we should upload all files, if any rejected files we should display an error - uploadAttachments({ files: acceptedFiles, node, dataModelBindings }); + uploadAttachments({ files: acceptedFiles, nodeId: node.id, dataModelBindings }); if (acceptedFiles.length > 0) { setShowFileUpload(displayMode === 'simple' ? false : attachments.length < maxNumberOfAttachments); @@ -87,7 +87,7 @@ export function FileUploadComponent({ node }: IFileUploadWithTagProps): React.JS const rejections = rejectedFiles.map((fileRejection) => new RejectedFileError(fileRejection, maxFileSizeInMB)); if (rejections?.length) { - addRejectedAttachments(node, rejections); + addRejectedAttachments(node.id, rejections); } }; diff --git a/src/layout/FileUpload/FileUploadTable/FileTableButtons.tsx b/src/layout/FileUpload/FileUploadTable/FileTableButtons.tsx index 117b0c400c..8c132a0f6b 100644 --- a/src/layout/FileUpload/FileUploadTable/FileTableButtons.tsx +++ b/src/layout/FileUpload/FileUploadTable/FileTableButtons.tsx @@ -45,7 +45,7 @@ export function FileTableButtons({ node, attachment, mobileView, editWindowIsOpe return; } - await removeAttachment({ attachment, node, dataModelBindings }); + await removeAttachment({ attachment, nodeId: node.id, dataModelBindings }); editWindowIsOpen && setEditIndex(-1); }; diff --git a/src/layout/FileUpload/index.tsx b/src/layout/FileUpload/index.tsx index 0fb992dddb..b0b3b42b95 100644 --- a/src/layout/FileUpload/index.tsx +++ b/src/layout/FileUpload/index.tsx @@ -29,7 +29,7 @@ export class FileUpload extends FileUploadDef implements ValidateComponent<'File } getDisplayData(node: LayoutNode<'FileUpload'>, { attachmentsSelector }: DisplayDataProps): string { - return attachmentsSelector(node) + return attachmentsSelector(node.id) .map((a) => a.data.filename) .join(', '); } @@ -63,7 +63,7 @@ export class FileUpload extends FileUploadDef implements ValidateComponent<'File const minNumberOfAttachments = nodeDataSelector((picker) => picker(node)?.item?.minNumberOfAttachments, [node]); // Validate minNumberOfAttachments - const attachments = attachmentsSelector(node); + const attachments = attachmentsSelector(node.id); if ( minNumberOfAttachments !== undefined && minNumberOfAttachments > 0 && diff --git a/src/layout/FileUploadWithTag/EditWindowComponent.tsx b/src/layout/FileUploadWithTag/EditWindowComponent.tsx index 3d04a3cb69..17fd657a78 100644 --- a/src/layout/FileUploadWithTag/EditWindowComponent.tsx +++ b/src/layout/FileUploadWithTag/EditWindowComponent.tsx @@ -75,7 +75,7 @@ export function EditWindowComponent({ await updateAttachment({ attachment, - node, + nodeId: node.id, tags, }); }; diff --git a/src/layout/FileUploadWithTag/index.tsx b/src/layout/FileUploadWithTag/index.tsx index acf19f3c30..ba6e10234b 100644 --- a/src/layout/FileUploadWithTag/index.tsx +++ b/src/layout/FileUploadWithTag/index.tsx @@ -29,7 +29,7 @@ export class FileUploadWithTag extends FileUploadWithTagDef implements ValidateC } getDisplayData(node: LayoutNode<'FileUploadWithTag'>, { attachmentsSelector }: DisplayDataProps): string { - return attachmentsSelector(node) + return attachmentsSelector(node.id) .map((a) => a.data.filename) .join(', '); } @@ -59,7 +59,7 @@ export class FileUploadWithTag extends FileUploadWithTagDef implements ValidateC const minNumberOfAttachments = nodeDataSelector((picker) => picker(node)?.item?.minNumberOfAttachments, [node]); // Validate minNumberOfAttachments - const attachments = attachmentsSelector(node); + const attachments = attachmentsSelector(node.id); if ( minNumberOfAttachments !== undefined && minNumberOfAttachments > 0 && From 03a45b6393fffabca0225fa675f38d1740270efb Mon Sep 17 00:00:00 2001 From: Ole Martin Handeland Date: Mon, 17 Feb 2025 14:59:20 +0100 Subject: [PATCH 18/21] Yay, an easy one! --- .../DevHiddenFunctionality/DevHiddenFunctionality.tsx | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/features/devtools/components/DevHiddenFunctionality/DevHiddenFunctionality.tsx b/src/features/devtools/components/DevHiddenFunctionality/DevHiddenFunctionality.tsx index 7afd9c10e3..2456a08253 100644 --- a/src/features/devtools/components/DevHiddenFunctionality/DevHiddenFunctionality.tsx +++ b/src/features/devtools/components/DevHiddenFunctionality/DevHiddenFunctionality.tsx @@ -6,7 +6,6 @@ import { useDevToolsStore } from 'src/features/devtools/data/DevToolsStore'; import { useComponentRefs } from 'src/features/devtools/hooks/useComponentRefs'; import { useIsInFormContext } from 'src/features/form/FormContext'; import { Hidden } from 'src/utils/layout/NodesContext'; -import { useNodeTraversalSelector } from 'src/utils/layout/useNodeTraversal'; import type { IDevToolsState } from 'src/features/devtools/data/types'; import type { IsHiddenOptions } from 'src/utils/layout/NodesContext'; @@ -47,15 +46,13 @@ const isHiddenOptions: IsHiddenOptions = { respectDevTools: false }; function MarkHiddenComponents() { const state = useDevToolsStore((state) => state.hiddenComponents); const isHiddenSelector = Hidden.useIsHiddenSelector(); - const traversalSelector = useNodeTraversalSelector(); useComponentRefs({ callback: (id, ref) => { if (ref.style.filter === pseudoHiddenCssFilter && state !== 'disabled') { ref.style.filter = ''; } else if (state === 'disabled') { - const node = traversalSelector((t) => t.findById(id), [id]); - const isHidden = node ? isHiddenSelector(node, isHiddenOptions) : true; + const isHidden = isHiddenSelector(id, isHiddenOptions); if (isHidden) { ref.style.filter = pseudoHiddenCssFilter; } From 7bcc40b760ce1818da0a112d9367ae93b2718a29 Mon Sep 17 00:00:00 2001 From: Ole Martin Handeland Date: Mon, 17 Feb 2025 16:21:30 +0100 Subject: [PATCH 19/21] There were not as simple, but I think the pagination one got considerably cheaper to run now --- .../Pagination/RepeatingGroupPagination.tsx | 51 ++++++------------- .../Providers/RepeatingGroupFocusContext.tsx | 33 ++++++------ src/layout/Tabs/Tabs.tsx | 22 ++++++-- 3 files changed, 49 insertions(+), 57 deletions(-) diff --git a/src/layout/RepeatingGroup/Pagination/RepeatingGroupPagination.tsx b/src/layout/RepeatingGroup/Pagination/RepeatingGroupPagination.tsx index 63e78b0985..221112d7f9 100644 --- a/src/layout/RepeatingGroup/Pagination/RepeatingGroupPagination.tsx +++ b/src/layout/RepeatingGroup/Pagination/RepeatingGroupPagination.tsx @@ -15,8 +15,7 @@ import { } from 'src/layout/RepeatingGroup/Providers/RepeatingGroupContext'; import { NodesInternal } from 'src/utils/layout/NodesContext'; import { useNodeItem } from 'src/utils/layout/useNodeItem'; -import { useNodeTraversalSelector } from 'src/utils/layout/useNodeTraversal'; -import type { RepGroupRow } from 'src/layout/RepeatingGroup/types'; +import { splitDashedKey } from 'src/utils/splitDashedKey'; import type { LayoutNode } from 'src/utils/layout/LayoutNode'; interface RepeatingGroupPaginationProps { @@ -210,49 +209,31 @@ function PaginationComponent({ /** * Returns a list of pagination pages containing errors */ -function usePagesWithErrors(rowsPerPage: number | undefined, node: LayoutNode<'RepeatingGroup'>) { - const rows = useNodeItem(node).rows; - const nodeValidationsSelector = NodesInternal.useValidationsSelector(); - const traversalSelector = useNodeTraversalSelector(); +function usePagesWithErrors(rowsPerPage: number | undefined, node: LayoutNode<'RepeatingGroup'>): number[] { + const rows = useNodeItem(node, (i) => i.rows); + const deepValidations = NodesInternal.useVisibleValidationsDeep(node, 'visible', false, undefined, 'error'); return useMemo(() => { if (typeof rowsPerPage !== 'number') { return []; } - const visibleRows: RepGroupRow[] = []; - for (const row of rows) { - if (row && !row.groupExpressions?.hiddenRow) { - visibleRows.push(row); - } - } - - const pagesWithErrors: number[] = []; + const { depth } = splitDashedKey(node.id); + const rowsWithErrors = new Set( + deepValidations.map((v) => splitDashedKey(v.nodeId).depth.slice(depth.length)[0]), + ); - for (let i = 0; i < visibleRows.length; i++) { - const pageNumber = Math.floor(i / rowsPerPage); - if (pagesWithErrors.includes(pageNumber)) { + const pagesWithErrors = new Set(); + for (const i of rowsWithErrors) { + const isHidden = rows[i]?.groupExpressions?.hiddenRow; + if (isHidden) { continue; } - const deepNodes = visibleRows[i].itemIds?.flatMap((nodeId) => - traversalSelector( - (t) => { - const node = t.findById(nodeId); - return node ? t.with(node).flat() : []; - }, - [nodeId], - ), - ); - for (const node of deepNodes || []) { - const validations = nodeValidationsSelector(node, 'visible', 'error'); - if (validations.length > 0) { - pagesWithErrors.push(pageNumber); - break; - } - } + const pageNumber = Math.floor(i / rowsPerPage); + pagesWithErrors.add(pageNumber); } - return pagesWithErrors; - }, [nodeValidationsSelector, rows, rowsPerPage, traversalSelector]); + return Array.from(pagesWithErrors); + }, [rowsPerPage, node.id, deepValidations, rows]); } diff --git a/src/layout/RepeatingGroup/Providers/RepeatingGroupFocusContext.tsx b/src/layout/RepeatingGroup/Providers/RepeatingGroupFocusContext.tsx index 87b0654f65..7dd0383a27 100644 --- a/src/layout/RepeatingGroup/Providers/RepeatingGroupFocusContext.tsx +++ b/src/layout/RepeatingGroup/Providers/RepeatingGroupFocusContext.tsx @@ -4,9 +4,10 @@ import type { PropsWithChildren } from 'react'; import { createContext } from 'src/core/contexts/context'; import { useRegisterNodeNavigationHandler } from 'src/features/form/layout/NavigateToNode'; import { useRepeatingGroup } from 'src/layout/RepeatingGroup/Providers/RepeatingGroupContext'; +import { BaseLayoutNode } from 'src/utils/layout/LayoutNode'; import { NodesInternal } from 'src/utils/layout/NodesContext'; -import { useNodeTraversalSelector } from 'src/utils/layout/useNodeTraversal'; import type { LayoutNode } from 'src/utils/layout/LayoutNode'; +import type { LayoutPage } from 'src/utils/layout/LayoutPage'; type FocusableHTMLElement = | HTMLButtonElement @@ -37,29 +38,25 @@ export const useRepeatingGroupsFocusContext = () => useCtx(); export function RepeatingGroupsFocusProvider({ children }: PropsWithChildren) { const elementRefs = useMemo(() => new Map(), []); const waitingForFocus = useRef(null); - const traversal = useNodeTraversalSelector(); const { node, openForEditing, changePageToRow } = useRepeatingGroup(); const getNodeItem = NodesInternal.useGetNodeData(node, (d) => d.item); useRegisterNodeNavigationHandler(async (targetNode) => { // Figure out if we are a parent of the target component, setting the targetChild to the target // component (or a nested repeating group containing the target component). - const targetChild = traversal( - (t) => { - if (targetNode.parent === node) { - // Direct child - return targetNode; - } - const parents = t.with(targetNode).parents(); - for (const parent of parents) { - if (parent.parent === node) { - return parent as LayoutNode; - } - } - return undefined; - }, - [targetNode, node], - ); + let targetChild: LayoutNode | undefined; + let subject: LayoutNode | LayoutPage | undefined = targetNode; + + while (subject) { + if (!(subject instanceof BaseLayoutNode)) { + break; + } + if (subject.parent === node) { + targetChild = subject; + break; + } + subject = subject.parent; + } if (!targetChild) { // We don't have any relation to the target diff --git a/src/layout/Tabs/Tabs.tsx b/src/layout/Tabs/Tabs.tsx index bb3f2c1423..e83c3f55f3 100644 --- a/src/layout/Tabs/Tabs.tsx +++ b/src/layout/Tabs/Tabs.tsx @@ -9,10 +9,11 @@ import { useLanguage } from 'src/features/language/useLanguage'; import { ComponentStructureWrapper } from 'src/layout/ComponentStructureWrapper'; import { GenericComponentById } from 'src/layout/GenericComponent'; import classes from 'src/layout/Tabs/Tabs.module.css'; +import { BaseLayoutNode } from 'src/utils/layout/LayoutNode'; import { useNodeItem } from 'src/utils/layout/useNodeItem'; -import { useNodeTraversalSelector } from 'src/utils/layout/useNodeTraversal'; import { typedBoolean } from 'src/utils/typing'; import type { PropsFromGenericComponent } from 'src/layout'; +import type { LayoutNode } from 'src/utils/layout/LayoutNode'; export const Tabs = ({ node }: PropsFromGenericComponent<'Tabs'>) => { const size = useNodeItem(node, (i) => i.size); @@ -20,10 +21,9 @@ export const Tabs = ({ node }: PropsFromGenericComponent<'Tabs'>) => { const tabs = useNodeItem(node, (i) => i.tabsInternal); const [activeTab, setActiveTab] = useState(defaultTab ?? tabs.at(0)?.id); - const traversalSelector = useNodeTraversalSelector(); useRegisterNodeNavigationHandler(async (targetNode) => { - const parents = traversalSelector((t) => t.with(targetNode).parents(), [targetNode]); - for (const parent of parents ?? []) { + const parents = parentNodes(targetNode); + for (const parent of parents) { if (parent === node) { const targetTabId = tabs.find((tab) => tab.childIds.some((childId) => childId === targetNode.id))?.id; if (targetTabId) { @@ -125,3 +125,17 @@ function TabHeader({ ); } + +function parentNodes(node: LayoutNode): LayoutNode[] { + const parents: LayoutNode[] = []; + let parent = node.parent; + while (parent) { + if (!(parent instanceof BaseLayoutNode)) { + break; + } + parents.push(parent); + parent = parent.parent; + } + + return parents; +} From 368aa1550a62383748e5b198120d0f0952094114 Mon Sep 17 00:00:00 2001 From: Ole Martin Handeland Date: Mon, 17 Feb 2025 17:13:39 +0100 Subject: [PATCH 20/21] Rewriting the remaining usages of node traversal that I found fairly easy to rewrite --- .../expressions/shared-context.test.tsx | 31 +++++++-------- .../dynamics/HiddenComponentsProvider.tsx | 3 +- .../validation/ValidationStorePlugin.tsx | 2 +- .../callbacks/onGroupCloseValidation.ts | 26 ++++++------- .../RepeatingGroupEditContext.tsx | 38 ++++++++----------- 5 files changed, 43 insertions(+), 57 deletions(-) diff --git a/src/features/expressions/shared-context.test.tsx b/src/features/expressions/shared-context.test.tsx index a02c1f372f..34ad1fbc47 100644 --- a/src/features/expressions/shared-context.test.tsx +++ b/src/features/expressions/shared-context.test.tsx @@ -8,11 +8,10 @@ import { getInstanceDataMock } from 'src/__mocks__/getInstanceDataMock'; import { getSharedTests } from 'src/features/expressions/shared'; import { fetchApplicationMetadata } from 'src/queries/queries'; import { renderWithInstanceAndLayout } from 'src/test/renderWithProviders'; -import { useNodeTraversal } from 'src/utils/layout/useNodeTraversal'; +import { NodesInternal } from 'src/utils/layout/NodesContext'; import { splitDashedKey } from 'src/utils/splitDashedKey'; import type { SharedTestContext, SharedTestContextList } from 'src/features/expressions/shared'; -import type { LayoutNode } from 'src/utils/layout/LayoutNode'; -import type { NodeTraversalFromRoot } from 'src/utils/layout/useNodeTraversal'; +import type { NodesContext } from 'src/utils/layout/NodesContext'; function contextSorter(a: SharedTestContext, b: SharedTestContext): -1 | 0 | 1 { if (a.component === b.component) { @@ -22,16 +21,15 @@ function contextSorter(a: SharedTestContext, b: SharedTestContext): -1 | 0 | 1 { return a.component > b.component ? 1 : -1; } -function recurse(t: NodeTraversalFromRoot, node: LayoutNode, key: string): SharedTestContextList { - const splitKey = splitDashedKey(node.id); +function recurse(state: NodesContext, nodeId: string, pageKey: string): SharedTestContextList { + const splitKey = splitDashedKey(nodeId); const context: SharedTestContextList = { component: splitKey.baseComponentId, - currentLayout: key, + currentLayout: pageKey, }; - const children = t - .with(node) - .children() - .map((child) => recurse(t, child, key)); + const children = Object.values(state.nodeData) + .filter((n) => n.parentId === nodeId) + .map((n) => recurse(state, n.layout.id, pageKey)); if (children.length) { context.children = children; } @@ -43,18 +41,15 @@ function recurse(t: NodeTraversalFromRoot, node: LayoutNode, key: string): Share } function TestContexts() { - const contexts = useNodeTraversal((t) => { + const contexts = NodesInternal.useMemoSelector((state) => { const contexts: SharedTestContextList[] = []; - const pages = t.children(); - for (const page of pages) { - const layout = t - .with(page) - .children() - .map((child) => recurse(t, child, page.pageKey)); + for (const page of Object.values(state.pagesData.pages)) { contexts.push({ component: page.pageKey, currentLayout: page.pageKey, - children: layout, + children: Object.values(state.nodeData) + .filter((n) => n.pageKey === page.pageKey && n.parentId === undefined) + .map((n) => recurse(state, n.layout.id, page.pageKey)), }); } diff --git a/src/features/form/dynamics/HiddenComponentsProvider.tsx b/src/features/form/dynamics/HiddenComponentsProvider.tsx index 642b392f2d..ff1fe37ab3 100644 --- a/src/features/form/dynamics/HiddenComponentsProvider.tsx +++ b/src/features/form/dynamics/HiddenComponentsProvider.tsx @@ -48,7 +48,6 @@ function useLegacyHiddenComponents() { } const props = [defaultDataType, hiddenNodes, formDataSelector, transposeSelector] as const; - const topLevelNode = traversalSelector((t) => t.allNodes()[0], []); for (const key of Object.keys(rules)) { if (!key) { continue; @@ -77,7 +76,7 @@ function useLegacyHiddenComponents() { } } } else { - runConditionalRenderingRule(rule, topLevelNode, ...props); + runConditionalRenderingRule(rule, undefined, ...props); } } diff --git a/src/features/validation/ValidationStorePlugin.tsx b/src/features/validation/ValidationStorePlugin.tsx index 6edc6fdc0e..1a68101fd4 100644 --- a/src/features/validation/ValidationStorePlugin.tsx +++ b/src/features/validation/ValidationStorePlugin.tsx @@ -261,7 +261,7 @@ interface GetDeepValidationsProps extends GetValidationsProps { restriction?: TraversalRestriction; } -function getRecursiveValidations(props: GetDeepValidationsProps): NodeRefValidation[] { +export function getRecursiveValidations(props: GetDeepValidationsProps): NodeRefValidation[] { const out: NodeRefValidation[] = []; if (props.includeSelf) { diff --git a/src/features/validation/callbacks/onGroupCloseValidation.ts b/src/features/validation/callbacks/onGroupCloseValidation.ts index 9c3f52864d..da6e1e162f 100644 --- a/src/features/validation/callbacks/onGroupCloseValidation.ts +++ b/src/features/validation/callbacks/onGroupCloseValidation.ts @@ -2,9 +2,9 @@ import { useCallback } from 'react'; import { getVisibilityMask } from 'src/features/validation/utils'; import { Validation } from 'src/features/validation/validationContext'; +import { getRecursiveValidations } from 'src/features/validation/ValidationStorePlugin'; import { useEffectEvent } from 'src/hooks/useEffectEvent'; import { NodesInternal } from 'src/utils/layout/NodesContext'; -import { useNodeTraversalSelector } from 'src/utils/layout/useNodeTraversal'; import type { AllowedValidationMasks } from 'src/layout/common.generated'; import type { LayoutNode } from 'src/utils/layout/LayoutNode'; import type { TraversalRestriction } from 'src/utils/layout/useNodeTraversal'; @@ -15,25 +15,23 @@ import type { TraversalRestriction } from 'src/utils/layout/useNodeTraversal'; */ export function useOnGroupCloseValidation() { const setNodeVisibility = NodesInternal.useSetNodeVisibility(); - const nodeValidationSelector = NodesInternal.useValidationsSelector(); const validating = Validation.useValidating(); - const traversalSelector = useNodeTraversalSelector(); + const nodeStore = NodesInternal.useStore(); /* Ensures the callback will have the latest state */ const callback = useEffectEvent( (node: LayoutNode, restriction: TraversalRestriction, masks: AllowedValidationMasks): boolean => { const mask = getVisibilityMask(masks); - - const nodesWithErrors = traversalSelector( - (t) => - t - .with(node) - .children(undefined, restriction) - .map((child) => t.with(child).flat()) - .flat() - .filter((n) => nodeValidationSelector(n, mask, 'error').length > 0), - [node, restriction, mask, nodeValidationSelector], - ); + const state = nodeStore.getState(); + const nodesWithErrors = getRecursiveValidations({ + id: node.id, + includeHidden: false, + includeSelf: false, + severity: 'error', + restriction, + mask, + state, + }).map((v) => v.nodeId); if (nodesWithErrors.length > 0) { setNodeVisibility(nodesWithErrors, mask); diff --git a/src/layout/RepeatingGroup/EditContainer/RepeatingGroupEditContext.tsx b/src/layout/RepeatingGroup/EditContainer/RepeatingGroupEditContext.tsx index 2e2b0c4260..d5c798d247 100644 --- a/src/layout/RepeatingGroup/EditContainer/RepeatingGroupEditContext.tsx +++ b/src/layout/RepeatingGroup/EditContainer/RepeatingGroupEditContext.tsx @@ -4,9 +4,9 @@ import type { PropsWithChildren } from 'react'; import { createContext } from 'src/core/contexts/context'; import { useRegisterNodeNavigationHandler } from 'src/features/form/layout/NavigateToNode'; import { useRepeatingGroup } from 'src/layout/RepeatingGroup/Providers/RepeatingGroupContext'; +import { BaseLayoutNode } from 'src/utils/layout/LayoutNode'; import { LayoutPage } from 'src/utils/layout/LayoutPage'; import { useNodeItem } from 'src/utils/layout/useNodeItem'; -import { useNodeTraversalSelector } from 'src/utils/layout/useNodeTraversal'; import type { LayoutNode } from 'src/utils/layout/LayoutNode'; interface RepeatingGroupEditRowContext { @@ -53,27 +53,27 @@ function useRepeatingGroupEditRowState( export function RepeatingGroupEditRowProvider({ children }: PropsWithChildren) { const { node } = useRepeatingGroup(); const { setMultiPageIndex, ...state } = useRepeatingGroupEditRowState(node); - const traversal = useNodeTraversalSelector(); useRegisterNodeNavigationHandler(async (targetNode) => { if (!state.multiPageEnabled) { // Nothing to do here. Other navigation handlers will make sure this row is opened for editing. return false; } - const ourChildRecursively = traversal( - (t) => - t - .with(node) - .flat() - .find((n) => n === targetNode), - [node, targetNode], - ); - if (!ourChildRecursively) { + let isOurChildRecursively = false; + let subject: LayoutNode | LayoutPage | undefined = targetNode; + while (subject instanceof BaseLayoutNode) { + if (subject.parent === node) { + isOurChildRecursively = true; + break; + } + subject = subject.parent; + } + + if (!isOurChildRecursively) { return false; } - const ourDirectChildren = traversal((t) => t.with(node).children(), [node]); - const ourChildDirectly = ourDirectChildren.find((n) => n === targetNode); - if (ourChildDirectly) { + const isOurChildDirectly = targetNode.parent === node; + if (isOurChildDirectly) { const targetMultiPageIndex = targetNode.multiPageIndex ?? 0; if (targetMultiPageIndex !== state.multiPageIndex) { setMultiPageIndex(targetMultiPageIndex); @@ -83,14 +83,8 @@ export function RepeatingGroupEditRowProvider({ children }: PropsWithChildren) { // It's our child, but not directly. We need to figure out which of our children contains the target node, // and navigate there. Then it's a problem that can be forwarded there. - const ourChildrenIds = new Set(ourDirectChildren.map((n) => n.id)); - const childWeAreLookingFor = traversal( - (t) => t.with(targetNode).parents((i) => i.type === 'node' && ourChildrenIds.has(i.layout.id)), - [targetNode], - )[0]; - - if (childWeAreLookingFor && !(childWeAreLookingFor instanceof LayoutPage)) { - const targetMultiPageIndex = childWeAreLookingFor.multiPageIndex ?? 0; + if (subject && !(subject instanceof LayoutPage)) { + const targetMultiPageIndex = subject.multiPageIndex ?? 0; if (targetMultiPageIndex !== state.multiPageIndex) { setMultiPageIndex(targetMultiPageIndex); } From 92477380db150bc2bd56f8fa1f14df465ff574f7 Mon Sep 17 00:00:00 2001 From: Ole Martin Handeland Date: Mon, 17 Feb 2025 18:17:46 +0100 Subject: [PATCH 21/21] Moving attachmentSelector to fix import error when running RepeatingGroupTable.test.tsx --- .../attachments/AttachmentsStorePlugin.tsx | 20 ++-------------- src/features/attachments/tools.ts | 23 +++++++++++++++++++ .../useAttachmentDeletionInRepGroups.ts | 2 +- src/features/displayData/index.ts | 2 +- src/features/validation/index.ts | 2 +- src/utils/layout/useExpressionDataSources.ts | 2 +- 6 files changed, 29 insertions(+), 22 deletions(-) create mode 100644 src/features/attachments/tools.ts diff --git a/src/features/attachments/AttachmentsStorePlugin.tsx b/src/features/attachments/AttachmentsStorePlugin.tsx index 2a9ab29878..0533c244a5 100644 --- a/src/features/attachments/AttachmentsStorePlugin.tsx +++ b/src/features/attachments/AttachmentsStorePlugin.tsx @@ -11,6 +11,7 @@ import { ContextNotProvided } from 'src/core/contexts/context'; import { useApplicationMetadata } from 'src/features/applicationMetadata/ApplicationMetadataProvider'; import { isAttachmentUploaded, isDataPostError } from 'src/features/attachments/index'; import { sortAttachmentsByName } from 'src/features/attachments/sortAttachments'; +import { appSupportsNewAttachmentAPI, attachmentSelector } from 'src/features/attachments/tools'; import { FD } from 'src/features/formData/FormDataWrite'; import { dataModelPairsToObject } from 'src/features/formData/types'; import { @@ -26,8 +27,6 @@ import { useWaitForState } from 'src/hooks/useWaitForState'; import { nodesProduce } from 'src/utils/layout/NodesContext'; import { NodeDataPlugin } from 'src/utils/layout/plugins/NodeDataPlugin'; import { splitDashedKey } from 'src/utils/splitDashedKey'; -import { isAtLeastVersion } from 'src/utils/versionCompare'; -import type { ApplicationMetadata } from 'src/features/applicationMetadata/types'; import type { DataPostResponse, IAttachment, @@ -36,6 +35,7 @@ import type { TemporaryAttachment, UploadedAttachment, } from 'src/features/attachments/index'; +import type { AttachmentsSelector } from 'src/features/attachments/tools'; import type { FDActionResult } from 'src/features/formData/FormDataWriteStateMachine'; import type { DSPropsForSimpleSelector } from 'src/hooks/delayedSelectors'; import type { IDataModelBindingsList, IDataModelBindingsSimple } from 'src/layout/common.generated'; @@ -664,18 +664,6 @@ export function useAttachmentsUploadMutation() { return useMutation(options); } -export type AttachmentsSelector = (nodeId: string) => IAttachment[]; -export const attachmentSelector = (nodeId: string) => (state: NodesContext) => { - const nodeData = state.nodeData[nodeId]; - if (!nodeData) { - return emptyArray; - } - if (nodeData && 'attachments' in nodeData) { - return Object.values(nodeData.attachments).sort(sortAttachmentsByName); - } - return emptyArray; -}; - function useAttachmentsAddTagMutation() { const { doAttachmentAddTag } = useAppMutations(); const instanceId = useLaxInstanceId(); @@ -730,7 +718,3 @@ function useAttachmentsRemoveMutation() { }, }); } - -export function appSupportsNewAttachmentAPI({ altinnNugetVersion }: ApplicationMetadata) { - return !altinnNugetVersion || isAtLeastVersion({ actualVersion: altinnNugetVersion, minimumVersion: '8.5.0.153' }); -} diff --git a/src/features/attachments/tools.ts b/src/features/attachments/tools.ts new file mode 100644 index 0000000000..20f5eed400 --- /dev/null +++ b/src/features/attachments/tools.ts @@ -0,0 +1,23 @@ +import { sortAttachmentsByName } from 'src/features/attachments/sortAttachments'; +import { isAtLeastVersion } from 'src/utils/versionCompare'; +import type { ApplicationMetadata } from 'src/features/applicationMetadata/types'; +import type { IAttachment } from 'src/features/attachments/index'; +import type { NodesContext } from 'src/utils/layout/NodesContext'; + +const emptyArray = []; + +export type AttachmentsSelector = (nodeId: string) => IAttachment[]; +export const attachmentSelector = (nodeId: string) => (state: NodesContext) => { + const nodeData = state.nodeData[nodeId]; + if (!nodeData) { + return emptyArray; + } + if (nodeData && 'attachments' in nodeData) { + return Object.values(nodeData.attachments).sort(sortAttachmentsByName); + } + return emptyArray; +}; + +export function appSupportsNewAttachmentAPI({ altinnNugetVersion }: ApplicationMetadata) { + return !altinnNugetVersion || isAtLeastVersion({ actualVersion: altinnNugetVersion, minimumVersion: '8.5.0.153' }); +} diff --git a/src/features/attachments/useAttachmentDeletionInRepGroups.ts b/src/features/attachments/useAttachmentDeletionInRepGroups.ts index 67849bbc44..57c01aa544 100644 --- a/src/features/attachments/useAttachmentDeletionInRepGroups.ts +++ b/src/features/attachments/useAttachmentDeletionInRepGroups.ts @@ -1,9 +1,9 @@ import { useCallback } from 'react'; import { AttachmentsPlugin } from 'src/features/attachments/AttachmentsPlugin'; -import { attachmentSelector } from 'src/features/attachments/AttachmentsStorePlugin'; import { useAttachmentsAwaiter, useAttachmentsRemover } from 'src/features/attachments/hooks'; import { isAttachmentUploaded } from 'src/features/attachments/index'; +import { attachmentSelector } from 'src/features/attachments/tools'; import { useAsRef } from 'src/hooks/useAsRef'; import { getComponentDef } from 'src/layout'; import { NodesInternal } from 'src/utils/layout/NodesContext'; diff --git a/src/features/displayData/index.ts b/src/features/displayData/index.ts index 789382dc1a..013025b81f 100644 --- a/src/features/displayData/index.ts +++ b/src/features/displayData/index.ts @@ -1,4 +1,4 @@ -import type { AttachmentsSelector } from 'src/features/attachments/AttachmentsStorePlugin'; +import type { AttachmentsSelector } from 'src/features/attachments/tools'; import type { IUseLanguage } from 'src/features/language/useLanguage'; import type { NodeOptionsSelector } from 'src/features/options/OptionsStorePlugin'; import type { FormDataSelector } from 'src/layout'; diff --git a/src/features/validation/index.ts b/src/features/validation/index.ts index 39794a58f7..497f3581ca 100644 --- a/src/features/validation/index.ts +++ b/src/features/validation/index.ts @@ -1,5 +1,5 @@ import type { ApplicationMetadata } from 'src/features/applicationMetadata/types'; -import type { AttachmentsSelector } from 'src/features/attachments/AttachmentsStorePlugin'; +import type { AttachmentsSelector } from 'src/features/attachments/tools'; import type { Expression, ExprValToActual } from 'src/features/expressions/types'; import type { DataElementSelector } from 'src/features/instance/InstanceContext'; import type { TextReference, ValidLangParam } from 'src/features/language/useLanguage'; diff --git a/src/utils/layout/useExpressionDataSources.ts b/src/utils/layout/useExpressionDataSources.ts index 3f78bf4311..85d7c75819 100644 --- a/src/utils/layout/useExpressionDataSources.ts +++ b/src/utils/layout/useExpressionDataSources.ts @@ -16,7 +16,7 @@ import { Hidden, NodesInternal, useNodes } from 'src/utils/layout/NodesContext'; import { useInnerDataModelBindingTranspose } from 'src/utils/layout/useDataModelBindingTranspose'; import { useInnerNodeFormDataSelector } from 'src/utils/layout/useNodeItem'; import { useInnerNodeTraversalSelector } from 'src/utils/layout/useNodeTraversal'; -import type { AttachmentsSelector } from 'src/features/attachments/AttachmentsStorePlugin'; +import type { AttachmentsSelector } from 'src/features/attachments/tools'; import type { ExternalApisResult } from 'src/features/externalApi/useExternalApi'; import type { DataElementSelector } from 'src/features/instance/InstanceContext'; import type { IUseLanguage } from 'src/features/language/useLanguage';