Skip to content
New issue

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

Sign up for GitHub

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

Jump to bottom

fix(applicant): add more logic to disable/enable save and close button #2918

Merged
merged 2 commits into from
Apr 11, 2024
Merged

fix(applicant): add more logic to disable/enable save and close button #2918

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fdacf70
fix(applicant): add more logic to disable / enable save and close button
sirtawast Apr 5, 2024
c229774
refactor: typing as per @EmiliaMakelaVincit; find instead of some
sirtawast Apr 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions ...it/applicant/src/components/applications/forms/application/step1/ApplicationFormStep1.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import DeMinimisAidForm from '../deMinimisAid/DeMinimisAidForm';
import DeMinimisAidsList from '../deMinimisAid/list/DeMinimisAidsList';
import { useDeminimisAidsList } from '../deMinimisAid/list/useDeminimisAidsList';
import StepperActions from '../stepperActions/StepperActions';
import { getTouchedAndInvalidFields } from '../utils';
import { findIntersectionOfTouchedAndErroredFields } from '../utils';
import CompanyInfo from './companyInfo/CompanyInfo';
import { useApplicationFormStep1 } from './useApplicationFormStep1';

Expand Down Expand Up @@ -314,8 +314,10 @@ const ApplicationFormStep1: React.FC<DynamicFormStepComponentProps> = ({
disabledNext={deMinimisTotal() > MAX_DEMINIMIS_AID_TOTAL_AMOUNT}
handleSubmit={handleSubmit}
handleSave={
getTouchedAndInvalidFields(formik.touched, formik.errors).length ===
0 && !isUnfinishedDeMinimisAid
findIntersectionOfTouchedAndErroredFields(
formik.touched,
formik.errors
).length === 0 && !isUnfinishedDeMinimisAid
? handleSave
: undefined
}
Expand Down
7 changes: 5 additions & 2 deletions ...it/applicant/src/components/applications/forms/application/step2/ApplicationFormStep2.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { useTheme } from 'styled-components';

import { $SubFieldContainer } from '../Application.sc';
import StepperActions from '../stepperActions/StepperActions';
import { getTouchedAndInvalidFields } from '../utils';
import { findIntersectionOfTouchedAndErroredFields } from '../utils';
import { useApplicationFormStep2 } from './useApplicationFormStep2';

const ApplicationFormStep2: React.FC<DynamicFormStepComponentProps> = ({
Expand Down Expand Up @@ -542,7 +542,10 @@ const ApplicationFormStep2: React.FC<DynamicFormStepComponentProps> = ({
<StepperActions
handleSubmit={handleSubmit}
handleSave={
getTouchedAndInvalidFields(formik.touched, formik.errors).length === 0
findIntersectionOfTouchedAndErroredFields(
formik.touched,
formik.errors
).length === 0
? handleSave
: undefined
}
Expand Down
38 changes: 36 additions & 2 deletions frontend/benefit/applicant/src/components/applications/forms/application/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,42 @@
import { Application } from 'benefit-shared/types/application';
import { FormikErrors, FormikTouched } from 'formik';

export const getTouchedAndInvalidFields = (
type FlattenedObject = Record<string, string>;

function flattenObject(
obj: FlattenedObject,
parentKey = '',
result: FlattenedObject = {}
): FlattenedObject {
const appendResult = result;
Object.keys(obj).forEach((key) => {
const newKey = `${parentKey}${parentKey ? '.' : ''}${key}`;
if (
typeof obj[key] === 'object' &&
obj[key] !== null &&
!Array.isArray(obj[key])
) {
flattenObject(obj[key] as unknown as FlattenedObject, newKey, result);
} else {
appendResult[newKey] = obj[key];
}
});
return result;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation is sound but the typing is iffy, yeah. The input is already assigned to FlattenedObject – assumed to be an object with only string values – which doesn't make sense since the whole point of the function is to flatten nested objects onto one level.

Here's how I would type this. The effective FormikTouched and FormikErrors types already are the same shape as the SimpleObject type given, which means the casts in the next function can also be removed altogether.

type SimpleObjectNode = SimpleObject | Array<SimpleObjectNode> | string | number | boolean | null;
type SimpleObject = { [prop: string]: SimpleObjectNode };

function isSimpleObject(a: SimpleObjectNode): a is SimpleObject {
  return typeof a === 'object' && a !== null && !Array.isArray(a);
}

function flattenObject(
  obj: SimpleObject,
  parentKey = '',
  result: { [prop: string]: Exclude<SimpleObjectNode, SimpleObject> } = {}
): SimpleObjectNode {
  const appendResult = result;
  Object.keys(obj).forEach((key) => {
    const newKey = `${parentKey}${parentKey ? '.' : ''}${key}`;
    const item = obj[key];
    if (isSimpleObject(item)) {
      flattenObject(item, newKey, result);
    } else {
      appendResult[newKey] = item;
    }
  });
  return result;
}

In our use case, where we only care about the object-ness of each leaf and the keys of the output, we could even define SimpleObjectNode as SimpleObject | unknown and I believe it would still work.


/**
* Used to enable / disable "save and close" button.
* Flatten Formik's error and touched fields to compare touched substring against error fields.
* Error fields are more verbose than touched fields, including min, max etc. validation data
* for the whole nested data set, for example, company or employee. Touch fields only consist
* of single fields that are touched. Thus substring is used to match touched fields against error fields.
* */
export const findIntersectionOfTouchedAndErroredFields = (
touched: FormikTouched<Partial<Application>>,
errors: FormikErrors<Partial<Application>>
): string[] =>
Object.keys(touched).filter((field) => Object.keys(errors).includes(field));
Object.keys(flattenObject(touched as FlattenedObject)).filter((field) =>
Object.keys(flattenObject(errors as FlattenedObject)).some((errorField) =>
errorField.includes(field)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the above suggestion, these could just be passed as is.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, and flattenObject is being run for errors quite a few times if there are many touched fields. I doubt there's much of a reason to fear performance issues here, but the flattened objects could also be optionally assigned to variables first before this two-level lookup happens.

)
);
Loading