(Feature) Add "Action" details in transaction list#1134
(Feature) Add "Action" details in transaction list#1134fernandomg merged 105 commits intodevelopmentfrom
Conversation
…ing-multiSend # Conflicts: # src/routes/safe/components/Transactions/TxsTable/ExpandedTx/TxDescription/index.tsx
…ing-multiSend # Conflicts: # src/routes/safe/components/Transactions/TxsTable/ExpandedTx/TxDescription/index.tsx
|
Travis automatic deployment: |
|
The ticket is in "Draft" status, so I cannot approve it, but it is approved from my side. It was tested with the #1106 |
|
CLA Assistant Lite All Contributors have signed the CLA. |
|
Travis automatic deployment: |
|
|
||
| const TxInfoDetails = ({ data }: { data: DataDecoded }): React.ReactElement => { | ||
| const methodName = data?.method ? ` (${data.method})` : '' | ||
|
|
There was a problem hiding this comment.
is it possible to have arguments if there is no method?
There was a problem hiding this comment.
nope! actually that check is unnecessary.
After this review, did some cleanup.
Thanks!
- removed unnecessary `methodName` variable - removed `<strong>` HTML element in favor of the `strong` Text property
- moved MultiSend custom data to `MultiSendCustomData` component - renamed previously named `MultiSendCustomData` to `MultiSendCustomDataAction`
|
Travis automatic deployment: |
| const TxInfoDetails = ({ data }: { data: DataDecoded }): React.ReactElement => ( | ||
| <TxInfo> | ||
| <TxDetailsMethodName size="lg"> | ||
| <strong>{data.method}</strong> |
There was a problem hiding this comment.
nitpick: here we're using strong tag but everywhere else in this file component <Bold /> is used, was it intended?
There was a problem hiding this comment.
Bold is a whole component just to put a <b/> tag, don't think it is necessary. Also, I prefer the semantic nature of strong.
That being said, I just removed this strong, in favor of the strong prop in the TxDetailsMethodName component 😬
There was a problem hiding this comment.
I don't think that strong's semantics have to do anything with what we want to there: https://www.w3schools.com/tags/tag_strong.asp
There was a problem hiding this comment.
Looking at your commit, it seems that you didn't understand my point. My point is that there are 4 occurrencies of <Bold /> component in this file and it would be great if we keep the things consistent
|
Travis automatic deployment: |
…in-tx-list' into feature/#882-add-action-details-in-tx-list
|
Travis automatic deployment: |
|
Travis automatic deployment: |
5 participants
This PR closes #882, by displaying tx
decodedDataalongData (hex encoded)tx details, whenever is possible.I'm not so sure about the final layout, but it's somehow done:
It's draft as it is derived from the PR #1106 branch,