[MPDX-7853] Add contact details designation account column #871

canac · 2024-02-12T15:56:44Z

Description

Reuse the donation table from the reports page in the contact donations tab.
Make the donation table remember which columns have been hidden by the user (to hide/show columns, hover over a header column, click the triple dots, then click Show columns).

https://jira.cru.org/browse/MPDX-7853

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

canac · 2024-02-12T15:57:33Z

...cts/ContactDetails/ContactDetailsHeader/ContactHeaderSection/ContactHeaderPartnerSection.tsx

@@ -29,30 +29,20 @@ export const ContactHeaderPartnerSection: React.FC<Props> = ({
        <TextSkeleton variant="text" />
      </ContactHeaderSection>
    );
-  } else {


No functionality is changed here, I just reduced some nesting and removed the unnecessary <Fragment> and <span>.

aws-amplify-us-east-1 · 2024-02-12T16:08:58Z

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-871.d3dytjb8adxkk5.amplifyapp.com

canac · 2024-02-13T01:25:57Z

@dr-bizz You can hold off on reviewing this. Scott wants the visibility of this column to be user-configurable instead of automatic.

canac · 2024-02-16T21:41:18Z

@caleballdrin @dr-bizz Let me know if you have any UX suggestions. One thing I noticed is that it's hard to get the totals columns to line up with the data grid columns when the user hides columns. I added explicit Amount and Foreign Amount columns just in case they don't line up so that it's still obvious what the totals numbers are. I think it's an overall improvement though because the old version didn't show the totals at all.

Also, I cherry-picked the commit that adds support for expect.toHaveGqlOperation() so that I could use it here, but I'll rebase it out if #868 lands before this.

src/theme.ts

wrandall22 · 2024-02-19T20:52:20Z

@j2trumpet might also have UX suggestions

dr-bizz · 2024-02-20T20:11:16Z

One thing I spotted when testing is that, the columns are too close together and cause the UX not to be as good. Also the column popup is cut off by the screen.

dr-bizz · 2024-02-20T20:13:27Z

Also when viewing it on responsive at 700px, I would make the donation table be full-width of the drawer.

dr-bizz

Great work on this! This is 95% done, just some finishing touches to make it look better.

package.json

src/theme.ts

dr-bizz · 2024-02-20T19:42:03Z

src/hooks/useLocalStorage.ts

+
+export const useLocalStorage = <T>(
+  key: string,
+  initialValue: T,


Shouldn't the initialValue be optional? or default to undefined

initialValue provides the value if the item is not currently saved in localStorage. If you want the initialValue to be undefined, I would think you would make that explicit with useLocalStorage<Value | undefined>('key', undefined).

Right now, there is only one place that uses useLocalStorage. Are you OK with considering making initialValue optional if/when we have a need for that in the app? I'd rather not make this hook support use cases that we aren't using right now.

Should this be stored in the database so that if people log in on different computers their configuration remains?

@caleballdrin Ultimately, probably so. Adding a new preference will require a server-side change. I was trying to get an MVP working that I could get feedback on from Scott.

src/components/Contacts/ContactDetails/ContactDonationsTab/ContactDonationsTab.tsx

__tests__/extensions/toHaveGraphqlOperation.ts

canac · 2024-02-20T20:20:42Z

One thing I spotted when testing is that, the columns are too close together and cause the UX not to be as good.

Yeah, that's the problem. Most people don't need all the columns, but a few people are asking for all the original columns, which is more than fits on smaller screens. I'm honestly not sure how to satisfy both constraints.

Also the column popup is cut off by the screen.

Can you scroll to show the rest of it? I was able to.

j2trumpet

I haven't touched MPDX for a while, so I'm not sure what the exact requirements or scope are for this ticket, but a few things stood out to me.

This is a little nit-picky, but when you hover over a column header, the sorting arrow fades in but the three dots pop in immediately. Could those be synced?

Do we really need the "hide column" functionality in both the column header dropdown and through the "Show columns" menu toggles? I feel like we could get rid of the "Hide" menu option and run everything through "Show columns" which I might rename to "Manage columns".

After opening a column header menu, should the three dots close the menu on a second click?

I thought date sorting wasn't working, but then I noticed that every entry on the report I'm looking at was logged on the same date.

The gray tone difference for the sorting arrow is so minimal between descending and unsorted that I didn't realize what was happening. It looked like there were two different sorts that showed an up arrow. Maybe we could find a more obvious difference for the states?

j2trumpet · 2024-02-20T20:54:39Z

To the concern about not having enough horizontal room for all the columns, maybe we could add a note somewhere that says "Not seeing all your data columns? Double check that the desired column is set to show or try scrolling/swiping right."

canac · 2024-02-20T21:00:24Z

@dr-bizz What do you think about setting a minimum width on all of the columns to keep them from getting too small and then if there isn't enough space, they will just have to scroll horizontally to view all the columns?

canac · 2024-02-20T21:36:58Z

@j2trumpet All of the header functionality is provided by MUI X Data Grid.

This is a little nit-picky, but when you hover over a column header, the sorting arrow fades in but the three dots pop in immediately. Could those be synced?

I believe this is possible.

Do we really need the "hide column" functionality in both the column header dropdown and through the "Show columns" menu toggles? I feel like we could get rid of the "Hide" menu option and run everything through "Show columns" which I might rename to "Manage columns".

I believe that removing the "Hide" menu option will require upgrading to MUI X v6.

After opening a column header menu, should the three dots close the menu on a second click?

I don't think that this is configurable.

The gray tone difference for the sorting arrow is so minimal between descending and unsorted that I didn't realize what was happening. It looked like there were two different sorts that showed an up arrow. Maybe we could find a more obvious difference for the states?

That's a valid point, at least if applying a sort order doesn't change the order of the items much. I believe the arrow can be styled. What kind of a different state would you recommend?

j2trumpet

I'm approving this because none of my thoughts are deal-breaking for approval.

dr-bizz · 2024-03-14T12:26:12Z

@dr-bizz What do you think about setting a minimum width on all of the columns to keep them from getting too small and then if there isn't enough space, they will just have to scroll horizontally to view all the columns?

I think that's a good idea.

canac · 2024-03-15T17:27:26Z

src/components/Layouts/SidePanelsLayout.tsx

@@ -115,7 +115,7 @@ export const SidePanelsLayout: FC<SidePanelsLayoutProps> = ({
  headerHeight = '0px',
 }) => {
  const isMobile = useMediaQuery((theme: Theme) =>
-    theme.breakpoints.down('sm'),
+    theme.breakpoints.down('md'),


This breakpoint made the detail pretty small on small screens, like ones around 700px. I increased the breakpoint to hide the list and just show the panel on those small screens.

canac · 2024-03-15T17:28:45Z

@dr-bizz I finally got around to updating this based on your feedback.

dr-bizz

I spotted some issues (donations flashing onLoad), but overall it looked good.

What is the need for the MUI patch?

src/components/Contacts/ContactDetails/ContactDonationsTab/DonationsGraph/DonationsGraph.tsx

src/components/Contacts/ContactDetails/ContactDonationsTab/ContactDonationsTab.tsx

canac · 2024-03-19T21:24:33Z

What is the need for the MUI patch?

@dr-bizz By default, the data grid preferences popup opens down, which causes the weird scrolling and clipping issues I believe you were seeing when you said "Also the column popup is cut off by the screen." I couldn't find a way to fix it because of how our layout is designed, so I patched MUI to open the popup up instead.

dr-bizz

I left one comment, but it's looking good so I'm approving it

src/components/DonationTable/DonationTable.tsx

github-actions · 2024-04-26T19:39:05Z

Bundle sizes [mpdx-react]

Compared against aad2df0

Dynamic import	Size (gzipped)	Diff
`../src/components/Contacts/ContactDetails/ContactDonationsTab/DynamicContactDonationsTab.tsx -> ./ContactDonationsTab`	181.95 KB	+80.07 KB

canac added the On Staging Will be merged to the staging branch by Github Actions label Feb 12, 2024

canac requested a review from dr-bizz February 12, 2024 15:56

canac commented Feb 12, 2024

View reviewed changes

canac removed the request for review from dr-bizz February 13, 2024 01:25

canac force-pushed the 7853-designation-column branch from 6dd838a to 1d5f7d6 Compare February 16, 2024 21:25

canac requested review from dr-bizz and caleballdrin February 16, 2024 21:41

canac commented Feb 16, 2024

View reviewed changes

src/theme.ts Show resolved Hide resolved

canac force-pushed the 7853-designation-column branch 3 times, most recently from 3b33149 to 795cac0 Compare February 19, 2024 14:34

canac requested a review from j2trumpet February 19, 2024 21:46

dr-bizz requested changes Feb 20, 2024

View reviewed changes

j2trumpet requested changes Feb 20, 2024

View reviewed changes

j2trumpet approved these changes Feb 20, 2024

View reviewed changes

canac removed the On Staging Will be merged to the staging branch by Github Actions label Mar 15, 2024

canac force-pushed the 7853-designation-column branch from 795cac0 to 165f309 Compare March 15, 2024 17:22

canac commented Mar 15, 2024

View reviewed changes

canac requested a review from dr-bizz March 15, 2024 17:28

dr-bizz requested changes Mar 18, 2024

View reviewed changes

src/components/Contacts/ContactDetails/ContactDonationsTab/DonationsGraph/DonationsGraph.tsx Show resolved Hide resolved

src/components/Contacts/ContactDetails/ContactDonationsTab/ContactDonationsTab.tsx Outdated Show resolved Hide resolved

canac requested a review from dr-bizz March 19, 2024 21:24

dr-bizz approved these changes Mar 20, 2024

View reviewed changes

src/components/DonationTable/DonationTable.tsx Show resolved Hide resolved

canac added the Preview Environment Add this label to create an Amplify Preview label Apr 26, 2024

canac added 12 commits April 30, 2024 14:31

Simplify partner account rendering in header

77d2c1b

Use DonationTable in contact donations tab

912dff1

Improve currency mocking

563e013

Improve test coverage

0beb936

Remove extra whitespace from donations sub-tabs

0ecb1da

Prevent unnecessary unmounting

695e83e

Improve donations tab responsiveness

0eeb9e1

Merge donations graph loading skeletons

0dec237

Give DataGrid preferences top-start placement

d0fc5c5

Fix right panel covering filter panel on mobile

68b4a6b

Add tooltip to donor and designation account cells

a8ae3c0

Fix appeal name sorting

ebcba43

canac force-pushed the 7853-designation-column branch from 2bdfa98 to ebcba43 Compare April 30, 2024 19:43

canac merged commit 427565b into main Apr 30, 2024
18 checks passed

canac deleted the 7853-designation-column branch April 30, 2024 19:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MPDX-7853] Add contact details designation account column #871

[MPDX-7853] Add contact details designation account column #871

canac commented Feb 12, 2024 •

edited

Loading

canac Feb 12, 2024

aws-amplify-us-east-1 bot commented Feb 12, 2024

canac commented Feb 13, 2024

canac commented Feb 16, 2024

wrandall22 commented Feb 19, 2024

dr-bizz commented Feb 20, 2024

dr-bizz commented Feb 20, 2024

dr-bizz left a comment

dr-bizz Feb 20, 2024

canac Mar 13, 2024

caleballdrin Mar 13, 2024

canac Mar 13, 2024

canac commented Feb 20, 2024

j2trumpet left a comment •

edited

Loading

j2trumpet commented Feb 20, 2024

canac commented Feb 20, 2024

canac commented Feb 20, 2024

j2trumpet left a comment

dr-bizz commented Mar 14, 2024

canac Mar 15, 2024

canac commented Mar 15, 2024

dr-bizz left a comment

canac commented Mar 19, 2024

dr-bizz left a comment

github-actions bot commented Apr 26, 2024 •

edited

Loading

[MPDX-7853] Add contact details designation account column #871

[MPDX-7853] Add contact details designation account column #871

Conversation

canac commented Feb 12, 2024 • edited Loading

Description

Checklist:

canac Feb 12, 2024

Choose a reason for hiding this comment

aws-amplify-us-east-1 bot commented Feb 12, 2024

canac commented Feb 13, 2024

canac commented Feb 16, 2024

wrandall22 commented Feb 19, 2024

dr-bizz commented Feb 20, 2024

dr-bizz commented Feb 20, 2024

dr-bizz left a comment

Choose a reason for hiding this comment

dr-bizz Feb 20, 2024

Choose a reason for hiding this comment

canac Mar 13, 2024

Choose a reason for hiding this comment

caleballdrin Mar 13, 2024

Choose a reason for hiding this comment

canac Mar 13, 2024

Choose a reason for hiding this comment

canac commented Feb 20, 2024

j2trumpet left a comment • edited Loading

Choose a reason for hiding this comment

j2trumpet commented Feb 20, 2024

canac commented Feb 20, 2024

canac commented Feb 20, 2024

j2trumpet left a comment

Choose a reason for hiding this comment

dr-bizz commented Mar 14, 2024

canac Mar 15, 2024

Choose a reason for hiding this comment

canac commented Mar 15, 2024

dr-bizz left a comment

Choose a reason for hiding this comment

canac commented Mar 19, 2024

dr-bizz left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 26, 2024 • edited Loading

Bundle sizes [mpdx-react]

canac commented Feb 12, 2024 •

edited

Loading

j2trumpet left a comment •

edited

Loading

github-actions bot commented Apr 26, 2024 •

edited

Loading