feat: create simple Arbitrary Txs action #3624

Nortsova · 2024-11-04T19:11:24Z

Description

Figma designs: https://www.figma.com/design/5V8pr7iMwXsT9L3VAZsmUt/Actions-%26-Motions?node-id=24968-204505&node-type=frame&t=VK5oE9wW19JFFa0k-0
Feature spec: https://www.notion.so/colony/Arbitrary-Transactions-ba4b096b3b664cb09ef65ad35c4331fa

Add Arbitrary transaction action type to the action form alongside all the required fields and the Contract interaction modal that shows on form submission.

Note: This is UI only, it doesn't need to be wired to any sagas at this point.

Testing

Step 1. Select "Arbitrary transaction" Motion (usually I select simple payment and then choose Arbitrary Transaction motion)

Step 2. Check that this form has validation

Step 3. Fill in the basic fields
Step 4. Press "Add transaction"
Step 5. Verify that you can see Modal and this Modal matches the design

Note

Note that this is a UI-only task, and some of the fields will be dynamic.
"Unformat JSON" button will be added in the next iteration as well

Step 6. Fill this form with the correct address, 0xc9B6218AffE8Aba68a13899Cbf7cF7f14DDd304C, because this Modal doesn't have validation yet, and we want to test how the table will look in the next step

Step 7. After you submit this form, Modal should be closed

Step 8. Verify that the Table matches the design

Step 9. Submit the form and verify that you can see the form values in the console

Motion forms are pretty complicated, and I worked with them for the first time, so comments are very welcome here 🙌

Diffs

Changes 🏗

I removed FormSelect and FormInput from C2F to common components (forms, in general are a pain, and we should revisit them one day 🥲 )
Added mode and label prop to Textarea component
I moved some stuff from UserSelect to general components so I can reuse it 🙌
getMaskedAddress util was added

Contributes to #3532

Nortsova · 2024-11-05T19:38:21Z

src/components/v5/common/ActionSidebar/partials/forms/ArbitraryTxsForm/consts.ts

+            contract: string().defined(),
+            json: string().defined(),
+            method: string().defined(),
+            amount: string().defined(),
+            to: string().defined(),


this is temporary, in the next iterations we will update these fields to be dynamic (according to ABI from the contract)

Nortsova · 2024-11-05T19:41:31Z

src/redux/types/actions/arbitrary.ts

+
+export type ArbitraryActionTypes = ActionTypeWithPayload<
+  ActionTypes.CREATE_ARBITRARY_TRANSACTION,
+  any


this one will be updated during sagas work

Nortsova · 2024-11-05T21:28:22Z

src/components/v5/common/AvatarWithAddress/AvatarWithAddress.tsx

+  return isUserAddressValid ? (
+    <>
+      <UserAvatar
+        userName={userName}
+        userAddress={address}
+        userAvatarSrc={userAvatarSrc}
+        size={size}
+        {...rest}
+      />
+      <p
+        className={clsx('ml-2 truncate text-md font-medium', {
+          'text-warning-400': !isVerified,
+          'text-gray-900': isVerified,
+        })}
+      >
+        {title ?? address}
+      </p>
+      {isVerified && (
+        <CircleWavyCheck
+          size={14}
+          className="ml-1 flex-shrink-0 text-blue-400"
+        />
+      )}
+    </>
+  ) : (
+    <div className="flex items-center gap-1 text-negative-400">
+      <WarningCircle size={16} />
+      <span className="text-md">
+        {formatText({
+          id: 'actionSidebar.addressError',
+        })}
+      </span>
+    </div>


this one just a part of the logic from UserSelect

Nortsova · 2024-11-05T21:28:47Z

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

+              {
+                title: 'Method',
+                value: original.method,
+              },
+              {
+                title: '_to (address)',
+                value: original.to,
+              },
+              {
+                title: '_amount (uint256)',
+                value: original.amount,
+              },


will be dynamic soon 🔜

jakubcolony

Very nice work @Nortsova, thanks for picking this up.

I only spotted a few minor issues:

There is too much space above the button when the table is empty:

In Figma, the textarea stops growing at 166px height when it becomes scrollable:

Mobile view:

Long values overflow and go off the screen
The address should be formatted as 0xXXXX...XXXX as per Figma designs

Validation ✅

Transactions table ✅

Submitting form ✅

Mobile view ✅

jakubcolony · 2024-11-05T22:56:58Z

src/components/v5/common/ActionSidebar/hooks/useActionsList.ts

@@ -133,6 +133,18 @@ const useActionsList = () => {
          // },
        ],
      },
+      {
+        key: '6',


@arrenv Is there a preference regarding which group arbitrary transaction should show under?

src/graphql/generated.ts

src/redux/sagas/actions/arbitrationTxs.ts

src/redux/types/actions/arbitrary.ts

src/components/v5/common/ActionSidebar/partials/forms/ArbitraryTxsForm/types.ts

src/components/v5/common/ActionSidebar/partials/forms/ArbitraryTxsForm/hooks.ts

Nortsova · 2024-11-06T11:50:04Z

Thank you, @jakubcolony, for your review. I really appreciate your help :)

I made all the changes you requested 🙌 So please check it again in your free time.

jakubcolony

Thanks for such a quick turnaround! 💯

Looks like we're nearly there, just noticing the textarea has collapsed to a single line and it doesn't grow as I enter extra lines:

Screen.Recording.2024-11-06.at.17.39.03.mov

In Figma, both empty and filled textarea have a height of 166px, so we should set it to that in all cases:

All previously reported snags are resolved:

Button spacing ✅

Address masking ✅

Mobile view ✅

Nortsova · 2024-11-07T09:38:23Z

Thank you @jakubcolony for testing 🙌

There are probably some issues with height/textarea. I put a class with a height of 10.25rem, but your browser doesn't pick it up for some reason. Nice catch! I covered that case with rows={7}

So now, if the browser does not pick the height from styles, it will calculate the height based on rows={7}; otherwise, it will take 10.25rem.

The only difference here is that rows={7} height could be slightly smaller than with height 10.25rem.

rows:

height style:

Please recheck it, and if it is still an issue - let me know 🙌

jakubcolony

That's interesting, what browser are you using?

It looks good on my end now, great work @Nortsova! 💪

Nortsova · 2024-11-07T20:51:52Z

I am happy that it works now 🎉 Actually, after your comment, I tested Chrome, Safari, and Firefox, and for all of them, it works without adding a row 😕 I don't know exactly why it happened on your machine 😄

iamsamgibbs

Great work on this @Nortsova ! Excited to see how this feature develops! Everything looks great and is logging on submit as expected. I've found a couple minor things but I think they can just be picked up in later PRs for the sake of getting this merged now.

These validation messages could use proper formatting on the field names (Transactions rather than transactions and Decision method rather than decisionMethod).

This link should have font-weight 500.

I'm assuming this is coming in a later PR, but the meatball menu is missing so it is impossible to edit a transaction once you have created one, so if you get an error like this it is impossible to resolve.

The spacing between the last field and the action buttons on mobile seems a bit large to me. I'm also not sure if the action buttons should be fixed to the bottom on mobile or not.

Great work on this so far though!

bassgeta

Niiiiiice, I really like how you pulled those nicer components out and used them instead of trying to use our existing form components and complicating this simple feature 💯
Additionally, really love the simple way of convincing textarea to not always autosize :D
I've left a few refactor comments, which are absolutely not necessary to do now (as I'm a bit late with my review, sorry :') ), but if you are feeling like it, you can do it here/on the feature branch/not do them at all

Arbitrary transaction shows up 👀 ✔️

Validation is there ✔️

A question, when adding tokens/verified members, we start out with an empty table, here we start out with just a button, so validation of an empty field is a bit weird... should we make this look the same (not really applicable to this PR, more of an UX question cc @melyndav )

Anyhow, the modal works and adds data to the table ✔️

And submit logs stuff out, nice one 🔥

bassgeta · 2024-11-14T08:27:29Z

src/components/v5/common/ActionSidebar/partials/forms/ArbitraryTxsForm/hooks.ts

+      [],
+    ),
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+    transform: useCallback(


can we try and remove useCallback here since it's a weird pattern we have anyways 🤷
I've tried removing mapPayload too, but typescript gets really really angry at me :D

bassgeta · 2024-11-14T08:31:18Z

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

+          });
+          return (
+            <span className="flex max-w-full items-center gap-2">
+              {isAddressValid && <UserAvatar userAddress={address} size={20} />}


Do you think we could use AvatarWithAddress here? not sure if it would be too much work, but it seems like a nifty component 💯

bassgeta · 2024-11-14T08:34:20Z

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

+            isFull: !isMobile,
+          });
+          return (
+            <CellDescription


What if we make CellDescription just this:

and just render 3 CellDescription components a la

<div className="w-full"> <CellDescription title="Method" value={original.method} /> <CellDescription title="_to (address)" value={addresTo.result} /> <CellDescription title="_amount (uint256)" value={original.amount} /> </div>

I think it's about the same, just more JSXish :D

I will handle this one in the scope of #3694 , thanks 🙌

bassgeta · 2024-11-14T08:35:31Z

src/components/v5/common/AvatarWithAddress/AvatarWithAddress.tsx

+  const isUserAddressValid = isAddress(address);
+  return isUserAddressValid ? (
+    <>
+      <UserAvatar


do you think we could just compose the avatar in, so this component doesn't need to care about the size and whatnot? so we would just do {avatar} even if we need to pass the address to both UserAvatar and AvatarWithAddress?
Then we could also potentially reuse this for the colony and token avatars too!

Thank you for your comments, I just decided that this AvatarWithAddress looks more complex than I thought, so I will create a separate ticket to refactor it. And I reverted this changes for this PR

Nortsova · 2024-11-14T13:52:14Z

Here is the ticket for refactoring the Avatar component #3711

feat: create simple Arbitrary Txs motion

a684e81

Nortsova self-assigned this Nov 4, 2024

Nortsova requested review from a team as code owners November 4, 2024 19:11

Nortsova marked this pull request as draft November 4, 2024 19:11

Nortsova added 4 commits November 4, 2024 22:53

feat: add transactions table and modal

d3e2d4b

fix: create basic form validation

3663e65

feat: implement transaction validation and throw data to the saga

162f6c1

fix: align select and textarea to the design

041c358

Nortsova commented Nov 5, 2024

View reviewed changes

feat: update styles for Modal and transaction table

a31ec3b

Nortsova force-pushed the feat/3532-motion-arbitrary-txs branch from c7bff3a to a31ec3b Compare November 5, 2024 21:25

Nortsova commented Nov 5, 2024

View reviewed changes

fix: refactor some small ui issues in textarea

9f70138

jakubcolony changed the title ~~[WIP]: feat: create simple Arbitrary Txs motion~~ [WIP]: feat: create simple Arbitrary Txs action Nov 5, 2024

Nortsova marked this pull request as ready for review November 5, 2024 21:45

feat: mobile alignment

2f4da3b

jakubcolony requested changes Nov 5, 2024

View reviewed changes

Nortsova changed the title ~~[WIP]: feat: create simple Arbitrary Txs action~~ feat: create simple Arbitrary Txs action Nov 6, 2024

fix: refactor function naming and fixing minor issues

ae319b3

jakubcolony self-requested a review November 6, 2024 15:54

jakubcolony requested changes Nov 6, 2024

View reviewed changes

fix: textarea height issue

325ec20

jakubcolony self-requested a review November 7, 2024 11:02

jakubcolony approved these changes Nov 7, 2024

View reviewed changes

iamsamgibbs approved these changes Nov 12, 2024

View reviewed changes

bassgeta approved these changes Nov 14, 2024

View reviewed changes

fix: revert AvatarWithAddress component

eed7927

Nortsova mentioned this pull request Nov 14, 2024

[Refactoring]: Avatar/UserAvatar/AvatarWithAddress #3711

Open

Nortsova merged commit f20a848 into feat/arbitrary-txs Nov 14, 2024
3 of 5 checks passed

Nortsova deleted the feat/3532-motion-arbitrary-txs branch November 14, 2024 13:57

Nortsova mentioned this pull request Nov 18, 2024

Feat/3533 arbitrary contract interaction #3694

Merged

9 tasks

Nortsova linked an issue Nov 19, 2024 that may be closed by this pull request

[Arbitrary Txs] Add arbitrary transaction action form #3532

Closed

4 tasks

jakubcolony mentioned this pull request Dec 18, 2024

Feature: Arbitrary Transactions (partial, behind a FF) #3943

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: create simple Arbitrary Txs action #3624

feat: create simple Arbitrary Txs action #3624

Nortsova commented Nov 4, 2024 •

edited

Loading

Nortsova Nov 5, 2024 •

edited

Loading

Nortsova Nov 5, 2024

Nortsova Nov 5, 2024

Nortsova Nov 5, 2024

jakubcolony left a comment

jakubcolony Nov 5, 2024

Nortsova commented Nov 6, 2024

jakubcolony left a comment

Nortsova commented Nov 7, 2024

jakubcolony left a comment

Nortsova commented Nov 7, 2024

iamsamgibbs left a comment

bassgeta left a comment

bassgeta Nov 14, 2024

bassgeta Nov 14, 2024

bassgeta Nov 14, 2024

Nortsova Nov 14, 2024

bassgeta Nov 14, 2024

Nortsova Nov 14, 2024

Nortsova commented Nov 14, 2024

feat: create simple Arbitrary Txs action #3624

feat: create simple Arbitrary Txs action #3624

Conversation

Nortsova commented Nov 4, 2024 • edited Loading

Description

Testing

Diffs

Nortsova Nov 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakubcolony left a comment

Choose a reason for hiding this comment

Submitting form ✅

Choose a reason for hiding this comment

Nortsova commented Nov 6, 2024

jakubcolony left a comment

Choose a reason for hiding this comment

Nortsova commented Nov 7, 2024

jakubcolony left a comment

Choose a reason for hiding this comment

Nortsova commented Nov 7, 2024

iamsamgibbs left a comment

Choose a reason for hiding this comment

bassgeta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nortsova commented Nov 14, 2024

Nortsova commented Nov 4, 2024 •

edited

Loading

Nortsova Nov 5, 2024 •

edited

Loading