-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rewriting form data storage #1712
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…an easily switch back and forth between implementations
…we read from the data model to use the new hook instead of the (soon to be old) redux state. I think I'm on the right path here, but there are still errors in my code right now that I don't have great solutions for. Passing the hook-state into every other saga function seems futile (I tried that), so I'm committing now to try some variations.
… or casting. Renaming `global.d.ts` to `modules.d.ts`, because using imports in this file breaks TypeScript everywhere when importing module css files.
…bally on the window object. This lets me rewrite parts of the form data storage system without doing it all at once, and without passing the formData (and future variables) around into every other saga action that needs to read it.
…t should only be accessed directly from the relevant sagas
…r re-use in new functionality
…eed for it when it is switched on. Making a dummy.
…is left to implement. Committing them now to avoid having these huge commits in the future.
…ly everything re-renders and causes infinite loops. Need to rethink the structure here, because this became a hook-mess pretty quickly.
…changes. Aaaand we're back to where we started with Redux, I suppose. Hoping this works better to avoid the mess, and figuring out why I get an infinite loop of changes (although it isn't working quite yet).
…at making useStateDeepEqual to fix this was hacky, because the root problem is that some state seems to change and make everything fail for some reason, but haven't gotten to the bottom of it quite yet. Happy that it works better now! Using immer for the reducer as well, as this makes things much easier to work with.
…ave(), trying to get state changes down to a minimum
# Conflicts: # src/features/formData/submit/submitFormDataSagas.ts
…o 'freeze' debounced data right away after fetching the data
…nders by deep-comparing and memoizing
…so JSON formatting the output so that null actually shows up as null
…ype problems with introducing other types than strings
# Conflicts: # src/features/confirm/components/ConfirmButton.tsx # src/features/dataLists/fetchDataListsSaga.ts # src/features/expressions/index.ts # src/features/formData/submit/submitFormDataSagas.ts # src/features/formRules/checkRulesSagas.ts # src/features/layout/update/updateFormLayoutSagas.ts # src/features/options/fetch/fetchOptionsSagas.test.ts # src/features/options/fetch/fetchOptionsSagas.ts # src/features/party/selectPartySagas.ts # src/features/toggles.ts # src/global.d.ts # src/layout/Address/AddressComponent.tsx # src/layout/Datepicker/index.tsx # src/layout/GenericComponent.tsx # src/setupTests.ts # src/utils/validation/runClientSideValidation.ts # test/e2e/support/app-frontend.ts # test/e2e/support/index.ts # test/e2e/support/navigation.ts # test/tsconfig.json # yarn.lock
…ore, and it looks like it debounces immediately now, so I'll remove it again.
…, so it's not using doPutFormData any longer.
…s are not implemented now, and I'll take a chance nobody is really using this (at least our tests)
…lementing our own way to apply changes between models to work around the issue
|
This was referenced Jan 2, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
kind/breaking-change
Issue/pull request containing a breaking change
kind/other
Pull requests containing chores/repo structure/other changes
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The headliners:
FD
export, which is a toolbox providing methods for getting/setting form data state.useDelayedSaveState()
anymore. Instead of keeping a copy of the form data locally and waiting to push these changes up to the global form data model, we push changes up to the context immediately while the user is typing in a form field, and the globaldebounce()
method on the form data will run (by default after 400ms, but configurable for some components via thesaveWhileTyping
property). As soon as debouncing happens, rules and expressions will run, components may be hidden/shown, and we'll send a request to save the data to the backend. Automatic saving can still be disabled, and we may want to offer a configuration option later to allow to wait a bit longer after debouncing until we actually send a save request (especially for apps without a need to continually change the form data inProcessDataWrite
).saveWhileTyping
property now only allows numbers. All components will 'save' while typing into the form, and it is only a matter of time until that (locally saved) data is saved to the backend as well. It is assumed that the most common reason to setsaveWhileTyping: false
was to wait until the user more explictly navigates out of the form field, as a way to prevent expensive actions inProcessDataWrite
from running needlessly/repeatedly because of too early automatic saving. If you've setsaveWhileTyping: false
before, you might want to consider using theCustomButton
component instead, to make the user trigger expensive actions more explicitly. Components withsaveWhileTyping: false
will have the same effect as not setting the property at all.app-frontend-react
, not a flattened dot-notation variant of it. This also means that we have full support for empty objects, and we have everything needed to properly support non-string data types in the future. Most components still do store/save their data as strings for now, and we still assume the backend will convert numeric data into numeric data types when parsing the JSON through their strict data model definition.ProcessDataWrite
can potentially populate new rows with prefill data as soon as the new rows appear (they will appear as empty objects in the data model). Previously, adding a new row would not trigger a save operation until the user started filling out data into the row. Thus, visiting a repeating group withopenByDefault
may trigger a save operation as soon as the user navigates to the form page.openByDefault: true
, as we'll only open (and add) a new row the first time the user visits the page with the repeating group, but the next time the group row will already exist and will not be opened by default (as is correct, per the earlier behaviour of theopenByDefault
functionality). If you always want an existing row to be open when the user sees the repeating group, you might want to consider settingedit.openByDefault = "first"
,edit.openByDefault = "last"
, or maybe instead settingedit.mode = "showAll"
.Less major notes and changes:
saveWhileTyping
common property).handleDataChange
method fromGenericComponent
no longer exists, and all the sagas that triggered validation does not run anymore).repeatingGroups
redux state is no more. It has been replaced by two providers,RepeatingGroupProvider
(which keeps the state for which row is open for editing, along with functions and tools for adding new rows, deleting rows, and checking which rows are visible according tohiddenRow
, etc) andRepeatingGroupEditRowProvider
(which keeps state for theRepeatingGroupsEditContainer
, such as which multiPage each edit container is on). This now has the added benefit thatmultiPage
works along withedit.mode = "showAll"
, which before most likely made every open row show the same multiPage at the same time (now the navigation happens locally to the edit container, so each edit container has their own state and can navigate independently).NavigateToNodeProvider
, which allows any component implementation to register as a handler for how to navigate to a node. This also means that the original code has been spread out all over the place, and repeating groups will now helpNavigateToNodeProvider
by manipulating their own state so that the repeating group edit state can be changed and allow opening the correct row for editing where the target node is visible.multiPage
) in order to find the component you're wanting to edit.PresentationComponent
now rendersDevTools
. This ensures that our devtools will always be on the 'inside' of any providers.useLaxCtx()
on a context that is normallyrequired: true
, which means that you can use that context even when it may not be provided. This is useful, for example for DevTools, because we're not always inside a and even though most providers in there are required, DevTools cannot trust the providers to exist. I added a symbol calledContextNotProvided
that might be returned when callinguseLaxCtx()
, so that we don't have two create split-personality components that need to know beforehand which providers are present to know which hooks to call. This should make it easier to write clutter-free components in the critical path, also without breaking the rules of hooks.createZustandContext()
, which makes it easy to provide a zustand store as a context. This was very much needed in the form data context, as zustand enabled me to change leaf values in the form data very rapidly (i.e. while typing a single character into an input component) while also making sure to only re-render the components that used that deep leaf value. Those using the debounced copy of the data model will also not re-render until the debouncing runs.QueryContext
s will by default show the<Loader />
component when the query re-fetches data (i.e., when itisLoading
). That also happens when thequeryKey
changes, but for some queries we might want to skip showing the<Loader />
and only show it when the data is initially loaded. That is the case for text resources, as changing the language in the language selector should not show the<Loader />
for a brief moment. Flashing the<Loader />
while changing the language would also reset states in repeating groups, causing edit rows to be closed when the language changed. I addeduseQueryWithStaleData
to fix that problem.AttachmentsStoreProvider
andAllOptionsStoreProvider
is provided beforeNodesProvider
, but the stores are populated when nodes are generated the first time.Form.tsx
take care of redirecting to the first form page when none is present, I moved that to theFormFirstPage
component. This was in an effort to make the code easier to read, and prevent a blank form page from showing up when navigating to a form (before the first form page was found and loaded).validation-hooks
branch.layout-sets.json
file. I now added that global page setting as a default inLayoutSettingsContext
to make theauto-save-behaviour
tests work again. This looks like it also fixes Layout settings for receipt pages and instance selection #1686, as it is now possible to override these settings globally inlayout-sets.json
, not just per-page.ProcessNavigationContext.tsx
that should probably be altered a bit to work with the newer validation rewrite. (Note to self, and @bjosttveit)useEffect()
inuseGetOptions.ts
. React nagged me about these.PDFWrapper
now replaces the way PDF preview/printing was added toProcessWrapper
/Form
. This way the PDF functionality can properly read search params in the URL again.pdf.ts
test slightly to make that test use the same URL as the real PDF generator would..../Task_3
and the fact thatTask_3
is a confirm step should be read out from the app configuration instead of magic URL constants. When doing this I noticed that there was a route in place for stateless apps to render a receipt, but I removed it since it's not possible for a stateless app to 'send in' any data, and such it's also not possible to show the receipt (it would have to turn stateful in order to do that, which is possible, but at that point the route would no longer kick in).useAsRef()
as a useful tool when creating unchanging callbacks that still should read the most current state. This hook has a striking similiarity to theuseEffectEvent()
in thevalidation-hooks
branch, which I wish I would have read beforehand. I also used this ref trick to remove the cache key fromuseWaitForState()
.AddressComponent
has now been heavily cleaned up, and will be even cleaner invalidation-hooks
.src/layout/Group
, but my changes in them caused git to think I deletedGroupContainer*
and addedRepeatingGroupContainer*
. This may become a pain when merging, sorry about that.GenericComponent
to its own fileRenderLabel
,RenderDescription
andRenderLegend
out to be real components instead of components that are re-generated wheneverGenericComponent
re-renders. All in all, with reducing the complexity and amount of data passing throughGenericComponent
it will usually not re-render all of the time.queryClient
in thewindow
object, so that Cypress can inject data into the query cache incy.changeLayout()
. This should also be used for DevTools in some way.Related Issue(s)
Probably also fixes:
Verification/QA
kind/*
label to this PR for proper release notes grouping