Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

MPDX-7878 - Dynamically loading field array on Add Multiple Contacts Modal #917

Closed
wants to merge 22 commits into from

Conversation

dr-bizz
Copy link
Contributor

@dr-bizz dr-bizz commented Apr 19, 2024

Description

The FieldArray on the Add Multiple Contacts Modal was taking at least 3 seconds to load. This fix shows the modal straight away and then loads in the FieldArray

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

@dr-bizz dr-bizz requested a review from canac April 19, 2024 21:09
Copy link
Contributor

github-actions bot commented Apr 19, 2024

Bundle sizes [mpdx-react]

Compared against f8c580c

Route Size (gzipped) Diff
/ 79.88 KB +2.33 KB
/404 80.45 KB +2.33 KB
/500 80.41 KB +2.33 KB
/_app 369.12 KB -208.26 KB
/_error 79.68 KB +2.33 KB
/accountLists 119.81 KB +36.51 KB
/accountLists/[accountListId] 271.52 KB +40.84 KB
/accountLists/[accountListId]/coaching 84.13 KB +3.18 KB
/accountLists/[accountListId]/coaching/[coachingId] 245.76 KB +45.86 KB
/accountLists/[accountListId]/contacts/[[...contactId]] 103.94 KB -236.6 KB
/accountLists/[accountListId]/contacts/flows/setup 145.77 KB -10.6 KB
/accountLists/[accountListId]/reports/coaching 245.77 KB +45.86 KB
/accountLists/[accountListId]/reports/designationAccounts 241.27 KB +8.3 KB
/accountLists/[accountListId]/reports/donations/[[...contactId]] 343.98 KB -51.71 KB
/accountLists/[accountListId]/reports/expectedMonthlyTotal 107.72 KB -21.29 KB
/accountLists/[accountListId]/reports/partnerCurrency/[[...contactId]] 133.68 KB -184.01 KB
/accountLists/[accountListId]/reports/partnerGivingAnalysis/[[...contactId]] 131.28 KB -184.53 KB
/accountLists/[accountListId]/reports/responsibilityCenters 242.21 KB +8.39 KB
/accountLists/[accountListId]/reports/salaryCurrency/[[...contactId]] 133.67 KB -184.01 KB
/accountLists/[accountListId]/settings/admin 134.65 KB +2.29 KB
/accountLists/[accountListId]/settings/integrations 155.24 KB +4.69 KB
/accountLists/[accountListId]/settings/manageAccounts 143 KB +5.11 KB
/accountLists/[accountListId]/settings/manageCoaches 138.78 KB +3.28 KB
/accountLists/[accountListId]/settings/notifications 135.31 KB +4.45 KB
/accountLists/[accountListId]/settings/organizations 136.48 KB +2.39 KB
/accountLists/[accountListId]/settings/organizations/accountLists 132.5 KB -20.84 KB
/accountLists/[accountListId]/settings/organizations/contacts 131.33 KB -20.89 KB
/accountLists/[accountListId]/settings/preferences 210.63 KB +50.73 KB
/accountLists/[accountListId]/tasks/[[...contactId]] 136.44 KB -173.73 KB
/accountLists/[accountListId]/tools 114.45 KB +35.39 KB
/accountLists/[accountListId]/tools/appeals 159.42 KB +74.72 KB
/accountLists/[accountListId]/tools/appeals/[appealId] 181.06 KB +23.18 KB
/accountLists/[accountListId]/tools/fixCommitmentInfo 90.91 KB +3.5 KB
/accountLists/[accountListId]/tools/fixEmailAddresses 89.82 KB +2.91 KB
/accountLists/[accountListId]/tools/fixMailingAddresses 92.67 KB +3.66 KB
/accountLists/[accountListId]/tools/fixPhoneNumbers 90.12 KB +2.9 KB
/accountLists/[accountListId]/tools/fixSendNewsletter 88.66 KB +2.35 KB
/accountLists/[accountListId]/tools/mergeContacts 86.93 KB +2.71 KB
/accountLists/[accountListId]/tools/mergePeople 86.5 KB +2.33 KB
/login 119.67 KB +36.1 KB
/logout 80.27 KB +2.33 KB
Dynamic import Size (gzipped) Diff
../src/components/Contacts/ContactDetails/ContactDetailsHeader/ContactDetailsMoreActions/DynamicMoreActionHideContactModal.tsx -> ./MoreActionHideContactModal 598 B added
../src/components/Contacts/ContactDetails/ContactDetailsHeader/DeleteContactModal/DynamicDeleteContactModal.tsx -> ./DeleteContactModal 651 B added
../src/components/Contacts/ContactDetails/ContactDetailsTab/DynamicContactDetailsTab.tsx -> ./ContactDetailsTab 60.92 KB added
../src/components/Contacts/ContactDetails/ContactDonationsTab/DynamicContactDonationsTab.tsx -> ./ContactDonationsTab 101.87 KB added
../src/components/Contacts/ContactDetails/ContactNotesTab/DynamicContactNotesTab.tsx -> ./ContactNotesTab 2.25 KB added
../src/components/Contacts/ContactDetails/ContactReferralTab/DynamicContactReferralTab.tsx -> ./ContactReferralTab 4.42 KB added
../src/components/Contacts/ContactFlow/DynamicContactFlow.tsx -> ./ContactFlow 40.58 KB added
../src/components/Contacts/ContactsList/DynamicContactsList.tsx -> ./ContactsList 28.74 KB added
../src/components/Contacts/ContactsMap/DynamicContactsMap.tsx -> ./ContactsMap 30.65 KB added
../src/components/Contacts/ContactsMap/DynamicContactsMapPanel.tsx -> ./ContactsMapPanel 6.21 KB added
../src/components/Contacts/ContactsRightPanel/DynamicContactsRightPanel.tsx -> ./ContactsRightPanel 135.5 KB added
../src/components/Contacts/MassActions/AddTags/DynamicMassActionsAddTagsModal.tsx -> ./MassActionsAddTagsModal 39.17 KB added
../src/components/Contacts/MassActions/AddToAppeal/DynamicMassActionsAddToAppealModal.tsx -> ./MassActionsAddToAppealModal 38.48 KB added
../src/components/Contacts/MassActions/AddToAppeal/DynamicMassActionsCreateAppealModal.tsx -> ./MassActionsCreateAppealModal 27 KB added
../src/components/Contacts/MassActions/EditFields/DynamicMassActionsEditFieldsModal.tsx -> ./MassActionsEditFieldsModal 82.72 KB added
../src/components/Contacts/MassActions/Exports/DynamicExportsModal.tsx -> ./ExportsModal 2.5 KB added
../src/components/Contacts/MassActions/Exports/Emails/DynamicMassActionsExportEmailsModal.tsx -> ./MassActionsExportEmailsModal 2.95 KB added
../src/components/Contacts/MassActions/Exports/MailMergedLabelModal/DynamicMailMergedLabelModal.tsx -> ./MailMergedLabelModal 28.74 KB added
../src/components/Contacts/MassActions/Merge/DynamicMassActionsMergeModal.tsx -> ./MassActionsMergeModal 4.1 KB added
../src/components/Contacts/MassActions/RemoveTags/DynamicMassActionsRemoveTagsModal.tsx -> ./MassActionsRemoveTagsModal 27.8 KB added
../src/components/Dashboard/ThisWeek/NewsletterMenu/MenuItems/ExportEmail/DynamicExportEmail.tsx -> ./ExportEmail 2.31 KB added
../src/components/Dashboard/ThisWeek/NewsletterMenu/MenuItems/ExportPhysical/DynamicExportPhysical.tsx -> ./ExportPhysical 3.53 KB added
../src/components/Dashboard/ThisWeek/NewsletterMenu/MenuItems/LogNewsLetter/DynamicLogNewsletter.tsx -> ./LogNewsletter 90.69 KB added
../src/components/Dashboard/ThisWeek/WeeklyActivity/WeeklyReportModal/DynamicWeeklyReportModal.tsx -> ./WeeklyReportModal 34.55 KB added
../src/components/EditDonationModal/DynamicEditDonationModal.tsx -> ./EditDonationModal 73.43 KB added
../src/components/Layouts/Primary/TopBar/Items/AddMenu/Items/AddDonation/DynamicAddDonation.tsx -> ./AddDonation 86.23 KB added
../src/components/Layouts/Primary/TopBar/Items/AddMenu/Items/CreateContact/DynamicCreateContact.tsx -> ./CreateContact 28.44 KB added
../src/components/Layouts/Primary/TopBar/Items/AddMenu/Items/CreateMultipleContacts/DynamicCreateMultipleContacts.tsx -> ./CreateMultipleContacts 32.52 KB added
../src/components/Layouts/Primary/TopBar/Items/AddMenu/Items/CreateMultipleContacts/DynamicCreateMultipleContactsFieldArray.tsx -> ./CreateMultipleContactsFieldArray 43.63 KB added
../src/components/Layouts/Primary/TopBar/Items/SearchMenu/DynamicSearchDialog.tsx -> ./SearchDialog 16.17 KB added
../src/components/Shared/Filters/DynamicFilterPanel.tsx -> ./FilterPanel 100.41 KB added
../src/components/Shared/HideContactsModal/DynamicHideContactsModal.tsx -> ./HideContactsModal 1.34 KB added
../src/components/Task/MassActions/AddTags/DynamicMassActionsTasksAddTagsModal.tsx -> ./MassActionsTasksAddTagsModal 40.13 KB added
../src/components/Task/MassActions/ConfirmationModal/DynamicMassActionsTasksConfirmationModal.tsx -> ./MassActionsTasksConfirmationModal 1.3 KB added
../src/components/Task/MassActions/EditTasks/DynamicMassActionsEditTasksModal.tsx -> ./MassActionsEditTasksModal 98.29 KB added
../src/components/Task/MassActions/RemoveTags/DynamicMassActionsTasksRemoveTagsModal.tsx -> ./MassActionsTasksRemoveTagsModal 28.63 KB added
../src/components/Task/Modal/Comments/DynamicTaskModalCommentsList.tsx -> ./TaskModalCommentsList 74.16 KB added
../src/components/Task/Modal/Form/Complete/DynamicTaskModalCompleteForm.tsx -> ./TaskModalCompleteForm 98.35 KB added
../src/components/Task/Modal/Form/DynamicTaskModalForm.tsx -> ./TaskModalForm 107.17 KB added
../src/components/Task/Modal/Form/LogForm/DynamicTaskModalLogForm.tsx -> ./TaskModalLogForm 141.14 KB added
../src/components/common/Modal/DynamicModal.tsx -> ./Modal no change removed

@dr-bizz dr-bizz requested a review from canac April 22, 2024 17:11
@canac
Copy link
Contributor

canac commented Apr 22, 2024

One caveat is that this changes introduces a waterfall. Instead of loading all the scripts for the modal in one request, it now waits for the CreateMultipleContacts chunk to load, then waits for the DynamicCreateMultipleContactsFieldArray chunk to load, and only then renders the modal content. Unless I'm mistaken, that will mean it will take even longer for the form to load and be usable than it did before.

You mentioned it taking >3 seconds to load. For me in staging, it takes <200ms. Do you know what might be causing a discrepancy that significant?
Screenshot 2024-04-22 at 1 11 15 PM

I do notice the modals hanging for a second on localhost but not in staging. Is that the 3 seconds you're referring to?

@dr-bizz
Copy link
Contributor Author

dr-bizz commented Apr 25, 2024

Yes, I tested it locally, as I didn't know you merged it into staging. That is where the 3 seconds came from. I do not see it taking longer with my fix, it takes about the same time.

I've merged it with staging, and it's loading a few milliseconds faster (Or appearing to due to the modal rendering straight away).

On your network tab, throttle your connection to fast 3G or slow 3G. This is where you will see the biggest difference with my fix since it shows the modal and a spinning loading icon while the FieldArray loads.

Copy link
Contributor

@canac canac left a comment

Choose a reason for hiding this comment

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

I tested this a couple times with preloading disabled. Here's the recording. With slow 3G throttling on localhost, the old version (i.e. the lazy-components branch) takes 12.66s to render the modal content. The new version (your branch) shows the field headers in about 7s but takes 14.83s to render the rest of the modal content (~17% longer, probably due to the request waterfall I mentioned earlier). Personally, since they can see the modal title and the close button is functional, Since user can't start typing until the second load completes, I don't think it helps the UX to show the heading row early at the expense of slightly slowing down loading the rest of the content. But my measurements may not be representative, and I trust your judgement if you think this is a UX improvement, so I'm going to approve this.

Old.version.mov
New.version.mov

@canac canac force-pushed the lazy-components branch from 830f831 to da17fb4 Compare April 29, 2024 13:31
Base automatically changed from lazy-components to main April 29, 2024 14:24
@dr-bizz
Copy link
Contributor Author

dr-bizz commented Apr 30, 2024

Thank you for showing me that; I can see how this might look better in the code than on the website. As there is already a loading icon on the modal when it loads slower and it seems to load fast for most users, I'm going to close this PR as I don't want to slow the modal dow futher.

@dr-bizz dr-bizz closed this Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants