Fix: Issues with actions table pagination and refactor table component #3935

mmioana · 2024-12-13T08:22:50Z

Description

This PR started off by addressing the broken pagination shown on the tables from the Manage verified members and Manage tokens actions and ended with a big refactoring of the Table component for a better separation of concerns in the supported table features/layouts etc.

Testing

TODO: Unfortunately for a throughout testing we'll have to go thorough all the tables we have in our app.

However, before we do this, let's check the issues for the Manage verified members and Manage tokens actions were solved.

Manage tokens

Step 1. Let's start off with a fresh new dev env
Step 2. Run node scripts/create-data.js --timeout 100 --coloniesCount 12
Step 3. Create a Manage tokens action
Step 4. Copy the token addresses from the other colonies and add them to the table until there are more than 10
Step 5. Check the pagination is not shown.
Step 6. Check the mobile version of the table.
Step 7. Create the action and check the table on the completed page

Screen.Recording.2024-12-19.at.17.41.44.mov

Manage verified members

Step 1. Create a Manage verified members action
Step 2. Select Add members - this is needed only for having a higher number of rows in the table
Step 3. Add more than 10 users
Step 4. Check the pagination is not shown.
Step 5. Check the mobile version of the table.
Step 6. Create the action and check the table on the completed page

Screen.Recording.2024-12-19.at.17.44.14.mov

Important

Ok, now let's move to the remaining actions that include tables.
I will kindly ask you to check both desktop and mobile screen sizes.
Also for each action, please complete the action and check the table on the completed action sidebar

Advanced payments action

Split payment action

Staged payments action

First enable the extension here http://localhost:9091/planex/extensions/StagedExpenditure

Manage reputation

Manage permissions

Edit colony details - the social links table

Arbitrary transactions

In order to test with a locally deployed contract, please add at line 21 in src/components/v5/common/ActionSidebar/partials/forms/ArbitraryTxsForm/partials/AddTransactionModal/useGenerateABI.ts
return;

Add a new transaction using the following values
Contract address: 0xeF841fe1611ce41bFCf0265097EFaf50486F5111
ABI/JSON:

[{"type":"constructor","payable":false,"inputs":[{"type":"string","name":"_name"},{"type":"string","name":"_symbol"},{"type":"uint8","name":"_decimals"}]},{"type":"event","anonymous":false,"name":"Approval","inputs":[{"type":"address","name":"src","indexed":true},{"type":"address","name":"guy","indexed":true},{"type":"uint256","name":"wad","indexed":false}]},{"type":"event","anonymous":false,"name":"Burn","inputs":[{"type":"address","name":"guy","indexed":true},{"type":"uint256","name":"wad","indexed":false}]},{"type":"event","anonymous":false,"name":"LogSetAuthority","inputs":[{"type":"address","name":"authority","indexed":true}]},{"type":"event","anonymous":false,"name":"LogSetOwner","inputs":[{"type":"address","name":"owner","indexed":true}]},{"type":"event","anonymous":false,"name":"MetaTransactionExecuted","inputs":[{"type":"address","name":"user","indexed":false},{"type":"address","name":"relayerAddress","indexed":false},{"type":"bytes","name":"functionSignature","indexed":false}]},{"type":"event","anonymous":false,"name":"Mint","inputs":[{"type":"address","name":"guy","indexed":true},{"type":"uint256","name":"wad","indexed":false}]},{"type":"event","anonymous":false,"name":"Transfer","inputs":[{"type":"address","name":"src","indexed":true},{"type":"address","name":"dst","indexed":true},{"type":"uint256","name":"wad","indexed":false}]},{"type":"function","name":"DOMAIN_SEPARATOR","constant":true,"stateMutability":"view","payable":false,"inputs":[],"outputs":[{"type":"bytes32"}]},{"type":"function","name":"PERMIT_TYPEHASH","constant":true,"stateMutability":"view","payable":false,"inputs":[],"outputs":[{"type":"bytes32"}]},{"type":"function","name":"allowance","constant":true,"stateMutability":"view","payable":false,"inputs":[{"type":"address","name":"src"},{"type":"address","name":"guy"}],"outputs":[{"type":"uint256"}]},{"type":"function","name":"approve","constant":false,"payable":false,"inputs":[{"type":"address","name":"guy"},{"type":"uint256","name":"wad"}],"outputs":[{"type":"bool"}]},{"type":"function","name":"authority","constant":true,"stateMutability":"view","payable":false,"inputs":[],"outputs":[{"type":"address"}]},{"type":"function","name":"balanceOf","constant":true,"stateMutability":"view","payable":false,"inputs":[{"type":"address","name":"src"}],"outputs":[{"type":"uint256"}]},{"type":"function","name":"burn","constant":false,"payable":false,"inputs":[{"type":"uint256","name":"wad"}],"outputs":[]},{"type":"function","name":"burn","constant":false,"payable":false,"inputs":[{"type":"address","name":"guy"},{"type":"uint256","name":"wad"}],"outputs":[]},{"type":"function","name":"decimals","constant":true,"stateMutability":"view","payable":false,"inputs":[],"outputs":[{"type":"uint8"}]},{"type":"function","name":"executeMetaTransaction","constant":false,"stateMutability":"payable","payable":true,"inputs":[{"type":"address","name":"_user"},{"type":"bytes","name":"_payload"},{"type":"bytes32","name":"_sigR"},{"type":"bytes32","name":"_sigS"},{"type":"uint8","name":"_sigV"}],"outputs":[{"type":"bytes"}]},{"type":"function","name":"getMetatransactionNonce","constant":true,"stateMutability":"view","payable":false,"inputs":[{"type":"address","name":"_user"}],"outputs":[{"type":"uint256","name":"nonce"}]},{"type":"function","name":"locked","constant":true,"stateMutability":"view","payable":false,"inputs":[],"outputs":[{"type":"bool"}]},{"type":"function","name":"mint","constant":false,"payable":false,"inputs":[{"type":"address","name":"guy"},{"type":"uint256","name":"wad"}],"outputs":[]},{"type":"function","name":"mint","constant":false,"payable":false,"inputs":[{"type":"uint256","name":"wad"}],"outputs":[]},{"type":"function","name":"name","constant":true,"stateMutability":"view","payable":false,"inputs":[],"outputs":[{"type":"string"}]},{"type":"function","name":"nonces","constant":true,"stateMutability":"view","payable":false,"inputs":[{"type":"address","name":"_user"}],"outputs":[{"type":"uint256","name":"nonce"}]},{"type":"function","name":"owner","constant":true,"stateMutability":"view","payable":false,"inputs":[],"outputs":[{"type":"address"}]},{"type":"function","name":"permit","constant":false,"payable":false,"inputs":[{"type":"address","name":"owner"},{"type":"address","name":"spender"},{"type":"uint256","name":"value"},{"type":"uint256","name":"deadline"},{"type":"uint8","name":"v"},{"type":"bytes32","name":"r"},{"type":"bytes32","name":"s"}],"outputs":[]},{"type":"function","name":"setAuthority","constant":false,"payable":false,"inputs":[{"type":"address","name":"authority_"}],"outputs":[]},{"type":"function","name":"setOwner","constant":false,"payable":false,"inputs":[{"type":"address","name":"owner_"}],"outputs":[]},{"type":"function","name":"symbol","constant":true,"stateMutability":"view","payable":false,"inputs":[],"outputs":[{"type":"string"}]},{"type":"function","name":"totalSupply","constant":true,"stateMutability":"view","payable":false,"inputs":[],"outputs":[{"type":"uint256"}]},{"type":"function","name":"transfer","constant":false,"payable":false,"inputs":[{"type":"address","name":"dst"},{"type":"uint256","name":"wad"}],"outputs":[{"type":"bool"}]},{"type":"function","name":"transferFrom","constant":false,"payable":false,"inputs":[{"type":"address","name":"src"},{"type":"address","name":"dst"},{"type":"uint256","name":"wad"}],"outputs":[{"type":"bool"}]},{"type":"function","name":"unlock","constant":false,"payable":false,"inputs":[],"outputs":[]},{"type":"function","name":"verify","constant":true,"stateMutability":"view","payable":false,"inputs":[{"type":"address","name":"_user"},{"type":"uint256","name":"_nonce"},{"type":"uint256","name":"_chainId"},{"type":"bytes","name":"_payload"},{"type":"bytes32","name":"_sigR"},{"type":"bytes32","name":"_sigS"},{"type":"uint8","name":"_sigV"}],"outputs":[{"type":"bool"}]}]

And select any method - probably mint(uint256) it's the simplest
Enter a value for the method parameters, confirm the modal and check the table

Important

Now on to the other pages having tables.
I will kindly ask you to check both desktop and mobile screen sizes.

Crypto to fiat

Go to http://localhost:9091/account/crypto-to-fiat
Given we don't actually have data to test the table, please add the following at line 47 in src/components/frame/v5/pages/UserCryptoToFiatPage/partials/FiatTransfersTable/FiatTransfersTable.tsx

const { /* sortedData, */ loading } = useFiatTransfersData(sorting);
const sortedData=[{id:"1",amount:"10",currency:"EUR",state:"funds_received",createdAt:"2024-12-10",receipt:{url:"http://localhost:9091"}},{id:"2",amount:"10",currency:"EUR",state:"funds_received",createdAt:"2024-12-10",receipt:{url:"http://localhost:9091"}},{id:"3",amount:"10",currency:"EUR",state:"funds_received",createdAt:"2024-12-10",receipt:{url:"http://localhost:9091"}},{id:"4",amount:"10",currency:"EUR",state:"funds_received",createdAt:"2024-12-10",receipt:{url:"http://localhost:9091"}},{id:"5",amount:"10",currency:"EUR",state:"funds_received",createdAt:"2024-12-10",receipt:{url:"http://localhost:9091"}},{id:"6",amount:"10",currency:"EUR",state:"funds_received",createdAt:"2024-12-10",receipt:{url:"http://localhost:9091"}},{id:"7",amount:"10",currency:"EUR",state:"funds_received",createdAt:"2024-12-10",receipt:{url:"http://localhost:9091"}},{id:"8",amount:"10",currency:"EUR",state:"funds_received",createdAt:"2024-12-10",receipt:{url:"http://localhost:9091"}},{id:"9",amount:"10",currency:"EUR",state:"funds_received",createdAt:"2024-12-10",receipt:{url:"http://localhost:9091"}},{id:"10",amount:"10",currency:"EUR",state:"funds_received",createdAt:"2024-12-10",receipt:{url:"http://localhost:9091"}},{id:"11",amount:"10",currency:"EUR",state:"funds_received",createdAt:"2024-12-10",receipt:{url:"http://localhost:9091"}},{id:"12",amount:"10",currency:"EUR",state:"funds_received",createdAt:"2024-12-10",receipt:{url:"http://localhost:9091"}},{id:"13",amount:"10",currency:"EUR",state:"funds_received",createdAt:"2024-12-10",receipt:{url:"http://localhost:9091"}}];

That's hopefully all folks ✨ but please let your creativity loose when testing these changes 🦾

Diffs

Changes 🏗

Clearer structure of the Table component

Resolves #3404
Resolves #3467

Fix tables page size issue and refactor table component

davecreaser

Hey @mmioana really great work on this one! I've half reviewed it, and I remember looking at it when it was in draft mode and you had some questions about it, so I roughly already know what the code is like. I'll do a full review soon, but wanted to first point out an issue that I'm seeing on mobile. It's a problem with the table overflowing the screen when I scroll down (making an advanced payment), it's easiest to just take a look at the video :)

Screen.Recording.2025-01-06.at.13.58.32.mov

On desktop everything is looking great though! 👏

Manage tokens:

Manage verified members:

Advanced payment:

davecreaser · 2025-01-06T14:09:18Z

src/components/common/ColonyActionsTable/RecentActivityTable.tsx

@@ -23,13 +24,20 @@ const RecentActivityTable: FC<ColonyActionsTableProps> = ({
      domainId: nativeDomainId,
    });

-  const pageCount = Math.ceil(totalActions / (props.pageSize ?? 10));


Okay this is such a small little tidy up thing, and it has no right to make me as happy as it does, but it's these tiny type of things that just add up over time to make a wayyyy more readable and clean codebase 🧹

davecreaser

Just finished reviewing the code. Looking really good, and I think it's a lot easier to read and simpler to use / reuse. It is still quite a complicated table but I think that's unavoidable at the moment! You've done a really nice job with it, wasn't a fun refactor I'm sure 👍

Still just keeping this review as request changes purely because of the errors I found while testing (in my last review) but code-wise it's all good. The comments I've made in the code are super minor. Nice one! 🎉

davecreaser · 2025-01-06T14:21:21Z

...ls/forms/ArbitraryTxsForm/partials/ArbitraryTransactionsTable/ArbitraryTransactionsTable.tsx

+        }}
+        layout={isMobile ? 'vertical' : 'horizontal'}
+        // sizeUnit={isMobile ? undefined : '%'}
+        // meatBallMenuSize={isMobile ? undefined : 10}


Leftover comments?

davecreaser · 2025-01-06T14:22:39Z

.../ActionSidebar/partials/forms/ArbitraryTxsForm/partials/ArbitraryTransactionsTable/hooks.tsx

@@ -113,11 +129,11 @@ export const useArbitraryTxsTableColumns = ({
          }
          return <CellDescription data={data} />;
        },
-        size: isMobile ? 100 : 67,
+        staticSize: isMobile ? '100px' : '67%',
      }),
    ],
    [columnHelper, isMobile, openTransactionModal, isError],


Probably best to fix this dependency array warning while we're here! 😄

davecreaser · 2025-01-06T14:54:29Z

src/components/v5/common/Table/types.ts

+  pageSize?: number; // Number of rows per page
+  canNextPage?: boolean; // Enable "next page"
+  canPreviousPage?: boolean; // Enable "previous page"
+  onPageChange?: (direction: PaginationDirection) => void;


Being pedantic, but ideally onPageChange should also have a comment if all the other properties do 😆

davecreaser · 2025-01-06T14:55:00Z

src/components/v5/common/Table/types.ts

+export interface BaseTableProps<T> extends BaseTableOptions<T> {
+  table: Table<T>; // React Table instance
+  rows?: TableRowOptions<T>; // Row-specific options
+  moreActions?: MoreActionsOptions<T>;


Again, a comment would be nice for moreActions just to keep it consistent

Fix: Virtualised row overflowing

Chore: Changes after review

mmioana · 2025-01-06T16:22:13Z

@davecreaser I have fixed the bug with the overflowing rows, which was reproducible also on master. It was caused by the width setup for the td in VirtualisedRow once the number of payments was > 10.
Also took care of the other comments, if you'd like to have another look 🙌

davecreaser

Awesome, the issue is fixed! Thanks for the code changes too. 🎉

Screen.Recording.2025-01-06.at.16.54.35.mov

bassgeta

Sheeeeeeesh now this is a lot of changes, legit stuff 💪
Props for untangling all the props, it's a major step in the right direction, but sadly the lib itself isn't that JSX friendly, so we gotta rely on a ton of JSX in hooks, but hey, can't have everything 🤷

I love the overrides approach, and overall the table now seems more sane to use, even if it's still quite complicated due to all the different variations 🔥
I've tried the meatball menu actions too, but I'm not gonna drop 1 GB of videos onto this review :D

Manage tokens ✔️

On mobile ✔️

Advanced payments ✔️

WITH 400 ROWS 😎

(we really should fix this to not take ages to load :D )

Split payments ⚠️

On mobile, the total number overlaps the Total text

Manage reputation ✔️

This one doesn't even need to be a table, but we can might as well use the styles it provides 🤷

Manage permissions ✔️

Edit colony details ✔️

Manage verified members ✔️

From this point onwards I'll just drop some weird bits so I don't spam this too much

I somehow got the recent activity table stuck in a weird state with pagination? But it seems like it may be just my local dev env crashing since the full activity table one didn't have multiple pages

src/components/v5/common/ActionSidebar/partials/BatchPaymentsTable/BatchPaymentsTable.tsx

Fix: Update colSpan for split payments table

mmioana · 2025-01-06T19:07:32Z

@bassgeta I fixed the Split payments footer text overlapping issue, but this was already present on master - wondering how many more there are 🫠

And regarding the pagination bug, I didn't manage to reproduce it

bassgeta

Other tables still looking good, seems like my bug was more due to my locally broken dev env (I've tried reproducing it since, and I wasn't able to)...
Anyhow, nice job fixing the colspan issue 😎

🚢

davecreaser

Still all looking good to me!

Split Payments table:

And completed action:

Note: the next button at the bottom has no padding, but that is an issue on master and not caused by this PR

rumzledz

Awesome awesome updates @mmioana ! 🙌

Found some tiny UI issues here and there but Ioana will deal with them separately 🔧

Merge it!!!

mmioana self-assigned this Dec 13, 2024

mmioana force-pushed the fix/3404-broken-action-table-pagination branch 8 times, most recently from 53ad3b6 to 4dc09b0 Compare December 19, 2024 16:16

mmioana marked this pull request as ready for review December 19, 2024 17:07

mmioana requested a review from a team as a code owner December 19, 2024 17:07

mmioana changed the title ~~WIP: Fix issue and refactor table component~~ Fix: issues with actions table pagination and refactor table component Dec 19, 2024

mmioana changed the title ~~Fix: issues with actions table pagination and refactor table component~~ Fix: Issues with actions table pagination and refactor table component Dec 19, 2024

mmioana force-pushed the fix/3404-broken-action-table-pagination branch from 4dc09b0 to 0f8a145 Compare December 20, 2024 08:37

Fix tables page size issue and refactor table component

db380ed

Fix tables page size issue and refactor table component

mmioana force-pushed the fix/3404-broken-action-table-pagination branch from 0f8a145 to db380ed Compare December 23, 2024 08:47

davecreaser requested changes Jan 6, 2025

View reviewed changes

davecreaser reviewed Jan 6, 2025

View reviewed changes

davecreaser requested changes Jan 6, 2025

View reviewed changes

mmioana added 2 commits January 6, 2025 16:27

Fix: Virtualised row overflowing

9643967

Fix: Virtualised row overflowing

Chore: Changes after review

a12ddb5

Chore: Changes after review

davecreaser previously approved these changes Jan 6, 2025

View reviewed changes

bassgeta requested changes Jan 6, 2025

View reviewed changes

src/components/v5/common/ActionSidebar/partials/BatchPaymentsTable/BatchPaymentsTable.tsx Show resolved Hide resolved

Fix: Update colSpan for split payments table

84b80f1

Fix: Update colSpan for split payments table

mmioana dismissed davecreaser’s stale review via 84b80f1 January 6, 2025 19:03

bassgeta approved these changes Jan 7, 2025

View reviewed changes

davecreaser approved these changes Jan 7, 2025

View reviewed changes

rumzledz approved these changes Jan 7, 2025

View reviewed changes

mmioana merged commit 84a52d4 into master Jan 7, 2025
2 checks passed

mmioana deleted the fix/3404-broken-action-table-pagination branch January 7, 2025 15:26

This was referenced Jan 7, 2025

[Tables]: Small findings after refactoring #4046

Closed

Fix: Findings regarding table refactoring #4047

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix: Issues with actions table pagination and refactor table component #3935

Fix: Issues with actions table pagination and refactor table component #3935

mmioana commented Dec 13, 2024 •

edited

Loading

davecreaser left a comment

davecreaser Jan 6, 2025

davecreaser left a comment

davecreaser Jan 6, 2025

davecreaser Jan 6, 2025

davecreaser Jan 6, 2025

davecreaser Jan 6, 2025

mmioana commented Jan 6, 2025

davecreaser left a comment

bassgeta left a comment

mmioana commented Jan 6, 2025

bassgeta left a comment

davecreaser left a comment

rumzledz left a comment

Fix: Issues with actions table pagination and refactor table component #3935

Fix: Issues with actions table pagination and refactor table component #3935

Conversation

mmioana commented Dec 13, 2024 • edited Loading

Description

Testing

Manage tokens

Manage verified members

Advanced payments action

Split payment action

Staged payments action

Manage reputation

Manage permissions

Edit colony details - the social links table

Arbitrary transactions

Dashboard

Activity

Balances

Incoming

Verified members

Crypto to fiat

Diffs

davecreaser left a comment

Choose a reason for hiding this comment

davecreaser Jan 6, 2025

Choose a reason for hiding this comment

davecreaser left a comment

Choose a reason for hiding this comment

davecreaser Jan 6, 2025

Choose a reason for hiding this comment

davecreaser Jan 6, 2025

Choose a reason for hiding this comment

davecreaser Jan 6, 2025

Choose a reason for hiding this comment

davecreaser Jan 6, 2025

Choose a reason for hiding this comment

mmioana commented Jan 6, 2025

davecreaser left a comment

Choose a reason for hiding this comment

bassgeta left a comment

Choose a reason for hiding this comment

Manage tokens ✔️

Advanced payments ✔️

Split payments ⚠️

Manage reputation ✔️

Manage permissions ✔️

Edit colony details ✔️

Manage verified members ✔️

mmioana commented Jan 6, 2025

bassgeta left a comment

Choose a reason for hiding this comment

davecreaser left a comment

Choose a reason for hiding this comment

rumzledz left a comment

Choose a reason for hiding this comment

mmioana commented Dec 13, 2024 •

edited

Loading