-
Notifications
You must be signed in to change notification settings - Fork 1
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-7955 ecreate Flows and Implement Contact Move Functionality #1030
Conversation
6176d1a
to
97f5dd9
Compare
Bundle sizes [mpdx-react]Compared against c2b3d21
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few style suggestions I noticed, but this is really good! I'm a little concerned that we had to copy/paste the DonationTable
component, but it would probably be fairly difficult to refactor it into a generic component that can be used in both places.
src/components/Tool/Appeal/Flow/ContactFlowDragLayer/ContactFlowRowPreview.tsx
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,279 @@ | |||
import React, { useEffect, useMemo } from 'react'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this component is too different from the existing DonationTable
to reuse it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so. It can probably be done, but the component might then become too confusing. Maybe this is something we can flag for post-switch to investigate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's good as-is for now.
src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsHelper.ts
Outdated
Show resolved
Hide resolved
src/components/Tool/Appeal/Modals/UpdateDonationsModal/UpdateDonationsHelper.ts
Outdated
Show resolved
Hide resolved
So this is what I initially did, but since it got so complex and I needed to move queries and functionality to the parent component, I had to switch to using my own table. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you've changed anything since I reviewed last. Still looks good! I agree about the donations table. Sometimes duplication is better for components that serve different enough purposes.
becf743
to
3cfc621
Compare
3cfc621
to
a95487b
Compare
MPDX-7955 Create Flows and Implement Contact Move Functionality
This PR was merged into production. something weird happened when I merged it and it think's it was closed. |
Description
In this PR, I added the column switching functionality to the flows view.
You can view the changes in this PR like so:
I will be adding the following functionality in up coming PRs.
Checklist: