MPDX-8678 - Improve AddDonation by zweatshirt · Pull Request #1369 · CruGlobal/mpdx-react

zweatshirt · 2025-07-15T14:36:39Z

Description

Jira ticket

Standardizes the AddDonation form, since many of the errors across fields were inconsistent.
For consistency, I tried to make my changes relatively similar to the changes made in MPDX-8679

Checklist:

I have given my PR a title with the format "MPDX-(JIRA#) (summary sentence max 80 chars)"
I have applied the appropriate labels. (Add the label "On Staging" to get the branch automatically merged into staging.)
I have requested a review from another person on the project

…e to accept onBlur and/or custom TextFieldProps props

github-actions · 2025-07-15T14:37:35Z

Preview branch generated at https://MPDX-8678-improve-add-donation.d3dytjb8adxkk5.amplifyapp.com

github-actions · 2025-07-15T14:39:32Z

Bundle sizes [mpdx-react]

Compared against f62263d

No significant changes found

wjames111

Looks great! thanks for improving this form. The only issue I had was getting the DateField working.

src/components/Layouts/Primary/TopBar/Items/AddMenu/Items/AddDonation/AddDonation.tsx

wjames111 · 2025-07-15T18:44:03Z

So right now we are essentially hacking the touched state by either leaving out the check for touched or forcing it to trigger on load onBlur={handleBlur('fieldValue')}. instead we could ensure Formik is properly updating the touched value by using formik's setFieldTouched.

 onChange={(_, value) => {
   setFieldValue('fieldValue', value);
   setFieldTouched('fieldValue', true, false);
}}
onBlur={() => {
  setFieldTouched('fieldValue', true);
}}

I think this is the best way to handle the form validation, it also works on the CustomDateField. CC @kegrimes

zweatshirt · 2025-07-15T20:31:14Z

So right now we are essentially hacking the touched state by either leaving out the check for touched or forcing it to trigger on load onBlur={handleBlur('fieldValue')}. instead we could ensure Formik is properly updating the touched value by using formik's setFieldTouched.
 onChange={(_, value) => {
   setFieldValue('fieldValue', value);
   setFieldTouched('fieldValue', true, false);
}}
onBlur={() => {
  setFieldTouched('fieldValue', true);
}}
I think this is the best way to handle the form validation, it also works on the CustomDateField. CC @kegrimes

@wjames111 Is this a change that should be made on all form fields or just the DateField one?

wjames111 · 2025-07-15T20:40:57Z

@zweatshirt Ideally all of them if possible.

…ere onBlur is required

…is therefore redundant

wjames111

Looks great!

wjames111 · 2025-07-16T17:09:22Z

src/components/Layouts/Primary/TopBar/Items/AddMenu/Items/AddDonation/AddDonation.tsx

  useMediaQuery,
 } from '@mui/material';
 import { FastField, Field, FieldProps, Form, Formik } from 'formik';
+import i18n from 'i18next';


I noticed this one of Daniel's reviews, it looks like we should be importing from src/lib/i18n.

Suggested change

import i18n from 'i18next';

import i18n from 'src/lib/i18n';

wjames111 · 2025-07-16T17:11:40Z

src/components/Layouts/Primary/TopBar/Items/AddMenu/Items/AddDonation/AddDonation.tsx

+    .typeError(i18n.t('Appeal amount must be a valid number'))
    .nullable()
    .test(
      'Is appeal amount in valid currency format?',
-      'Appeal amount must be in valid currency format',
+      i18n.t('Appeal amount must be in valid currency format'),
      (amount) =>
        !amount || /\$?[0-9][0-9.,]*/.test(amount as unknown as string),
    )
    .test(
      'Is positive?',
-      'Must use a positive number for appeal amount',
+      i18n.t('Must use a positive number for appeal amount'),
      (value) => !value || parseFloat(value as unknown as string) > 0,
    ),
  appealId: yup.string().nullable(),
-  currency: yup.string().required(),
-  designationAccountId: yup.string().required(),
-  donationDate: requiredDateTime(),
-  donorAccountId: yup.string().required(),
+  currency: yup.string().required(i18n.t('Currency is required')),
+  designationAccountId: yup
+    .string()
+    .required(i18n.t('Designation account is required')),
+  donationDate: requiredDateTime(i18n.t('Date is required')),
+  donorAccountId: yup.string().required(i18n.t('Partner Account is required')),


I think this will be a lot more helpful to the user!

wjames111 · 2025-07-16T17:12:36Z

src/components/Layouts/Primary/TopBar/Items/AddMenu/Items/AddDonation/AddDonation.tsx

+                            onChange={(e) => {
+                              setFieldValue('amount', e.target.value);
+                              setFieldTouched('amount', true, false);
+                            }}
+                            onBlur={() => setFieldTouched('amount', true)}


Thanks for using the setFieldTouched on these!

wjames111 · 2026-04-01T20:59:34Z

src/components/Layouts/Primary/TopBar/Items/AddMenu/Items/AddDonation/AddDonation.tsx

Thanks for making these changes!

zweatshirt added 3 commits July 15, 2025 09:30

Standardizes AddDonation form errors behavior

2f208bf

Change CustomDateField, CurrencyAutocomplete, DonorAccountAutocomplet…

819e6b1

…e to accept onBlur and/or custom TextFieldProps props

fixup: Import FocusEventHandler in CurrencyAutocomplete

b4544f8

zweatshirt added the Preview Environment Add this label to create an Amplify Preview label Jul 15, 2025

zweatshirt requested a review from wjames111 July 15, 2025 14:54

wjames111 requested changes Jul 15, 2025

View reviewed changes

src/components/Layouts/Primary/TopBar/Items/AddMenu/Items/AddDonation/AddDonation.tsx Outdated Show resolved Hide resolved

Change onBlur handler to adhere to Formik expectations, for fields wh…

84967f1

…ere onBlur is required

zweatshirt requested a review from wjames111 July 16, 2025 17:05

zweatshirt added 2 commits July 16, 2025 13:33

Removes onBlur from CustomDateField, it doesn't work as intended and …

3a34c77

…is therefore redundant

Changes i18n import statement

a3dad43

wjames111 approved these changes Apr 1, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MPDX-8678 - Improve AddDonation#1369

MPDX-8678 - Improve AddDonation#1369
zweatshirt wants to merge 6 commits intomainfrom
MPDX-8678-improve-add-donation

zweatshirt commented Jul 15, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Jul 15, 2025

Uh oh!

github-actions bot commented Jul 15, 2025 •

edited

Loading

Uh oh!

wjames111 left a comment

Uh oh!

Uh oh!

wjames111 commented Jul 15, 2025 •

edited

Loading

Uh oh!

zweatshirt commented Jul 15, 2025

Uh oh!

wjames111 commented Jul 15, 2025

Uh oh!

wjames111 left a comment

Uh oh!

wjames111 Jul 16, 2025

Uh oh!

wjames111 Jul 16, 2025

Uh oh!

wjames111 Jul 16, 2025

Uh oh!

wjames111 Apr 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

zweatshirt commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist:

Uh oh!

github-actions bot commented Jul 15, 2025

Uh oh!

github-actions bot commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bundle sizes [mpdx-react]

Uh oh!

wjames111 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wjames111 commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zweatshirt commented Jul 15, 2025

Uh oh!

wjames111 commented Jul 15, 2025

Uh oh!

wjames111 left a comment

Choose a reason for hiding this comment

Uh oh!

wjames111 Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

wjames111 Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

wjames111 Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

wjames111 Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zweatshirt commented Jul 15, 2025 •

edited

Loading

github-actions bot commented Jul 15, 2025 •

edited

Loading

wjames111 commented Jul 15, 2025 •

edited

Loading