Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

Feature #388: Use adbk list in new transactions #461

Merged
merged 29 commits into from
Feb 3, 2020

Conversation

Agupane
Copy link
Contributor

@Agupane Agupane commented Jan 20, 2020

Closes #388
Closes #464
Closes #481
Closes #483

Description

  • Implements the addressbook list within the transaction creation/send funds

Looks like this now:

image

Note

  • The designs shows that the selected entry should look like this:
    image

I just displayed the address for now, with our current material ui components was not easy to implement that, so I just simplified it, I'll open an issue for improving this if you guys agree.

germartinez and others added 11 commits December 18, 2019 13:35
* Adds cookie permissions to localStorage/redux state

* Adds action

* Adds files to git

* (fix) linting issues

* (update) flow-typed

* (update) .eslint and .flowconfig

* (add) cookie banner

* Finish cookie banner implementation

* (Add) checkbox's disabled style.

* Removes redux for cookiesStorage

* Fix cookieStore deletion

* Fixs cookies acceptance

* Fixs cookies banner verbiage
Fix "x" in wrong place for snackbar messages

* (remove) unused library

* Adds cookies utils
Replaces localStorage with cookies
Adds js-cookie

* (fix) added correct polished library and import, updated flow-typed

* (update) removed polish flow type, added js-cookie flow type

* Add link to cookie policy, use generic links for legal docs

* Remove link to cookie policy from sidebar, link cookie policy in the banner

* Let the user re-open the cookie banner

* remove withMutations from cookies reducer, move utils/cookies to logic/cookies

* Now the sidebar closes when the cookie banner is toggled

* Feature #169: Intercom (#301)

* Implements intercom
Adds REACT_APP_INTERCOM_ID_MAINNET and REACT_APP_INTERCOM_ID_RINKEBY env vars

* Adds .env.example

* Adds intercom env vars

* Updates env vars
Replaces "rinkeby" and "mainnet" with "non-production" and "production"

* Now loads intercom after the user accepted the analytics

* Add env variable for production intercom id

* Update .env.example

* Removes react-intercom
Fixs getIntercomId with default dev appID
Now loads intercom as script

* Renegerate flow-types

* Remove 'Hide zero balances' (#310)

* Use medium font size for 'select an asset' label (#312)

* Feature #272: Google Analytics (#299)

* Adds google analytics tracking for every route

* Adds cookies acceptance check before tracking

* Fix react-ga dependency

* Fix cookieStore deletion

* Merge with #189-cookie-banner

* Fixs react ga version
Refactored HOC with hooks

* Fix TYPO

* Fix path for cookies utils

* Fix imports

* remove flow type definition for polish

* Add GA ID log

* Fix load GA After cookies acceptance

* Feature #224: Activate tokens automatically (#300)

* Replace 'Manage Tokens' with 'Manage List'

* prevent 301 redirects

* Add `BLACKLISTED_TOKENS` key to persist through immortal

* Add store/action to extract _activate tokens by its balance_

- keeps already activated tokens
- discards blacklisted tokens
- adds tokens whose vales are bigger than zero and are not blacklisted

* Add `blacklistedTokens` list to safe's store

* Display activeTokensByBalance in 'Balances' screen

* Enable token's blacklisting functionality in Tokens List

* Retrieve balance from API

* Rename action to `activateTokensByBalance`

* Fix linting errors

- line too long
- required return

* Do not persist a separate list into `BLACKLISTED_TOKENS`

* Typo fix (#326)

* Fix security vulnerability: Remove uglifyjs, use terser plugin (#327)

* Remove uglifyjs, use terser plugin

* fix css-loader config

* Feature #256: Sticky header (#308)

* Add sticky header

* Remove react-headroom, set position to fixed for header

* Regenerate yarn lock

* Remove unused headroom style from root.scss

* Pull from dev, conflict fixes

* Update welcome text (#323)

* Feature #137: Tx list improvements (#222)

* Fix swapOwners threshold displayed as hex in tx list

* Refactor spinner in empty table

* Fix number of rows per page in table pagination

* Add use of EtherscanLink component

* Set short version of strings in tx list

* Adjust styles in tx list

* Add more styles to table

* WIP

* An attempt to fix #204 by showing UNKNOWN instead of failed to fetch token symbol

* Table pagination style fixes

* Show confirm transaction button in owner list

* Update dependencies

* Add confirmation icons to owner list in tx list

* exclude unneeded stuff from travis.yml

* Adds cookie permissions to localStorage/redux state

* Update dependencies

* Adds action

* Adds files to git

* (fix) linting issues

* (update) flow-typed

* (update) .eslint and .flowconfig

* (add) cookie banner

* Finish cookie banner implementation

* (Add) checkbox's disabled style.

* Removes redux for cookiesStorage

* Fix cookieStore deletion

* Increase TO_EXP for bignumber.js

* Fixs cookies acceptance

* Fixs cookies banner verbiage
Fix "x" in wrong place for snackbar messages

* (fix) added correct polished library and import, updated flow-typed

* (update) removed polish flow type, added js-cookie flow type

* Add link to cookie policy, use generic links for legal docs

* Remove link to cookie policy from sidebar, link cookie policy in the banner

* Mock Safe creation transaction

* Format code

* Fix break statement

* Remove deployment of storybook

* Let the user re-open the cookie banner

* Update tx status messages and visual confirmation progress

* Fix svg in tx confirmation progress

* Add styles to tx type in tx list

* Replace nonce in tx list with tx id

* Update opacity of cancelled tx

* Fix short version of address

* remove withMutations from cookies reducer, move utils/cookies to logic/cookies

* Now the sidebar closes when the cookie banner is toggled

* Fix styles in tx list

* Add Pending status in tx description

* (remove) unused library

* Adds cookies utils
Replaces localStorage with cookies
Adds js-cookie

* Set 25 rows per page in tx list by default

* Align tx table

* Adjust tx table and tx details borders

* Fix fetching transactions to show Safe creation tx alone

* Fix failed Safe creation transaction

* Add styles to tx data

* Refactor and fix owner list in transaction

* Refactor use of theme variables

* Remove storybook files

* Update dependencies

* Fix warnings

* Fix dependencies

* Update file-loader config

* Fix owner colors in the tx confirmation progress

* Fix transaction type icon height

* Tx list adjustments

* Update readme

* (Feature) Etherscan button icon (#331)

* (add) new open on etherscan button icon

* (remove) unused asset

* (fix) icon background

* Feature #239: Replace early access label with network label (#311)

* Remove early access label

* Revert "Remove early access label"

This reverts commit 34682f0.

* Replace early access label with network label

* Capitalzie first letter of the network name

* Adds threshold update on checkAndUpdateSafe (#320)

* Feature #159: Pending transaction that requires user confirmation (#330)

* Creates a new notification: waitingConfirmation
Adds key as optional parameter for notification
Implemented getAwaitingTransactions to get the transactions that needs to be confirmed by the current user
Not fetchTransactions action also dispatch a notification for awaiting transactions
Improved performance of routes/safe/container/index to avoid re-rendering

* Removes notification logic on fetchTransactions
Adds notificationsMiddleware

* Moves fetchTransaction to container

* Removes unused param on fetchTransactions

* Fixs null safe check

* Fixs middleware declaration

* Removes lodash

* Changes cancelled transaction detection logic

* Feature #122: Multisig migration (#315)

* Adds query-string package.json
Parses query string on open layout

* Implements load all the values on openSafe view from param querys

* Adds query params validation

* Moves query parse logic to open.jsx

* Changes default no metamask component on open page

* Replaces global isNaN

* Fix threshold parsing validation

* Updates the welcome component with new verbiage for open

* Renames isOpenSafe to isOldMultisigMigration

* Merge branch 'development' of https://github.com/gnosis/safe-react into 122-multisig-migration

# Conflicts:
#	src/routes/open/components/Layout.jsx

* Merge branch 'development' of https://github.com/gnosis/safe-react into 159-pending-transactions

# Conflicts:
#	src/routes/safe/components/Transactions/index.jsx
#	yarn.lock

* set anonymizeIp to true (#335)

* Feature #180: Predict transaction nonce (#293)

* Dep bump

* Fetch transactions when safe view is mounted

* eslint fix

* Calculate new tx nonce from latest tx in service

* Fix tx cancellation, allow passing nonce to createTransaction

* dep bump

* Refactor createTransaction/processTransaction to use object as argument

* Adopting transactions table to new send tx flow with predicted nonces

* dep bump, disable esModule in file-loader options after new v5 release

* Don't show older tx annotation for already executed txs

* sort tx by nonce

* get new safe nonce after tx execution

* Bugfixes

* remove whitespace for showOlderTxAnnotation

* Feature #329: Rename to Multisig (#334)

* Rename to Multisig

* migration text fix

* replace safe for teams with multisig

* Fixs race condition (#341)

Fixs typo

* (Feature) Incoming transactions (#333)

* Add `blockNumber` to transactions model

* Create `incomingTransaction` node in store and load it along with `transactions`

* Add incoming transfers to the Transactions table

* Rename `transactionHash` to `executionTxHash` for better incoming/outgoing txs unification in Transactions table

* Add incoming transactions details

* Add transaction type icon in table row

* Add snackbar notification for incoming txs

* Make incoming transaction snackbar to show on any tab

* Use makeStyles hooks

* Fix incoming amounts conversion from wei

* Make concurrent promise calls

* Use date to calculate transactions ids

* Prevent repeating messages

- also move logic to display snack bar into the notifications middleware

* Merge transactions and incomingTxs to the transactions selector

* Show 'Multiple incoming transfers' if they are more than 3

* Prevent incoming transactions snack bar for first-timer users

* Set ID as the default order

* Use constant for _incoming_ type

* Feature #154: Fiat Balances (#290)

* Adds DropdownCurrency
Adds redux store for currencyValues
Adds Value column on the assets table
Adds mocked currency values

* (add) base currency dropdown

* (add) dropdown styles

* Refactors data fetching of the balances list
Now uses the endpoint

* Fix column value styling

* Adds support for ECB currency values

* Fixs list overflow

* Changes endpoint url
Adds decimals for balance values

* (fix) remove inline style

* (add) currencies dropdown search field

* (fix) list items' hover color

* Implements filter search

* Fix warning on dropdown template

* Saves selected currency in localStorage

* Remove spaces on curly braces
Add alt
Renames rowItem to cellItem
Improves fetchCurrenciesRates handling

* Removes withMutations

* Removes middleware
Export style to another file for dropdownCurrency

* Adds classNames

* Fix incomming transactions fetching (#346)

* Feature: Activate fortmatic (#339)

* Add fortmatic integration to web3connect

* add fortmatic

* Safe open form improvements: limit calling initContracts to 1 time

* update .env.example

* Feature #336: Confirmation required notification for non-owners fix (#338)

* Refactors grantedSelector with isUserOwner function
Checks if the user is owner of the safe before sending notification

* Adds safeParamAddressFromStateSelector
Refactors notificationsMiddleware with new selector

* Remove old size check

* safe notifications middleware fixes

* add apt-get update to travis yml

* (Fix) Incoming transactions inline-styles (#344)

* Remove inline styles

* Replace ternary with logical && operator

* use cn as shortcut for classnames

* Makes minMaxLength 2 to AddCustomToken (#363)

* Fixs ETH display on balances list (#360)

* Bug #348: Safelist entries get removed (#358)

* Fix balances saved to localStorage not in format [tokenAdd, balance] but [balance]

* Updates localStorage version value

* Use submission instead of execution date to sort outgoing txs (#364)

* Feature #190: Sidebar improvements (#347)

* Change icons
Adds checked icon

* Adds safeParamAddressFromStateSelector for get current safe selected
Implements check icon on sidebar

* Remove overflow on sidebar
Start alignments

* Removes headerPlaceholder

* Improves header

* Improves header

* Fix header style

* use sameAddress function to check address eqaulity when fetching transactions (#365)

* Bug #352: Owner shown multiple times (#367)

* Ensure lowercased string comparison for owners' addresses

* Use `sameAddress` for addresses comparison

* Use transaction value as a string (#369)

* Update isTokenTransfer to use value as a string

* Rename error message

* Update dependencies

* Refactor

* Fix alternative token abi and token address for incoming transactions (#373)

* Bug #313: Payload breaks ui (#371)

* Makes minMaxLength 2 to AddCustomToken

* Fix styling

* Fix typo

* Feature #200: Show version number (#370)

* Add `dotenv-expand` as a dependency

* Add app version to sidebar

* Add hardcoded latest safe version to env variables

* Add `semver` to compare current vs latest version

* Add Safe version to Safe Details

* Adjustments in version number

* Fix transaction description value (#377)

* Fix transaction description value

* Remove duplicated symbols

* fix checkAndUpdateSafe logic (#379)

* Update .env.example

* update package json version

* update package json version

* Fix app version in side bar

* add REACT_APP_APP_VERSION global env var

* add react_app_version to build script

* remove react_app_app_version from build-mainnet
Adds AddressBookInput
@Agupane Agupane requested a review from mmv08 January 20, 2020 23:27
@Agupane Agupane self-assigned this Jan 20, 2020
@ghost
Copy link

ghost commented Jan 20, 2020

Travis automatic deployment:
https://pr461--safereact.review.gnosisdev.com

@mmv08
Copy link
Member

mmv08 commented Jan 21, 2020

Can be improved:
If I manually enter address that is in my address book, it doesn't get resolved to an address book entry

Edit: oh, we don't show the name even if we select an address book entry

@mmv08
Copy link
Member

mmv08 commented Jan 21, 2020

I just displayed the address for now, with our current material ui components

Why do you have to use material ui for that? We used custom component for the token, I think you can do something similar

Copy link
Member

@mmv08 mmv08 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job :)

@lukasschor
Copy link
Member

Searching for address book entries should not be case-sensitive.

Screenshot 2020-01-21 at 12 11 02

Screenshot 2020-01-21 at 12 11 08

@lukasschor
Copy link
Member

Ideally, it would also show address book name and identicon after selecting the entry in the input field. But also fine, leaving this as a future enhancement.

@Agupane
Copy link
Contributor Author

Agupane commented Jan 21, 2020

Fixed case sensitive @lukasschor. Also now the user is able to search by putting an address or the name

@ghost
Copy link

ghost commented Jan 21, 2020

Travis automatic deployment:
https://pr461--safereact.review.gnosisdev.com

@Agupane
Copy link
Contributor Author

Agupane commented Jan 21, 2020

Opened an issue for improve the UI here.

@Agupane Agupane requested a review from mmv08 January 21, 2020 11:37
@mmv08
Copy link
Member

mmv08 commented Jan 21, 2020

Got a whitescreen after clicking "review" with these errors
Screenshot 2020-01-21 at 17 00 53

the tx:
Screenshot 2020-01-21 at 17 01 40

@ghost
Copy link

ghost commented Jan 21, 2020

Travis automatic deployment:
https://pr461--safereact.review.gnosisdev.com

@Agupane
Copy link
Contributor Author

Agupane commented Jan 21, 2020

@mikheevm please check it now, I have also managed to display the identicon, name and selected address

@Agupane
Copy link
Contributor Author

Agupane commented Jan 27, 2020

I created the issue 483 for that

@francovenica
Copy link
Contributor

francovenica commented Jan 28, 2020

I've found these issues for the address book
#477
#478
#479
#480
#481
#482

@lukasschor can you give em a priority label?

@ghost
Copy link

ghost commented Jan 29, 2020

Travis automatic deployment:
https://pr461--safereact.review.gnosisdev.com

@fernandomg
Copy link
Contributor

Added a changed that closes #483, the current behavior:

display-addressbook-list-on-change

@Agupane, @lukasschor

@ghost
Copy link

ghost commented Jan 29, 2020

Travis automatic deployment:
https://pr461--safereact.review.gnosisdev.com

@mmv08
Copy link
Member

mmv08 commented Jan 30, 2020

nice fix @fernandomg

@francovenica
Copy link
Contributor

francovenica commented Jan 30, 2020

I have some issues regarding 481 and 483:
481: Now the review button is grayed out until you put a valid address from the address book (that's fine), but now I can only use address that I have in my address book. If I copy-paste an address I dont have in the address book is not validated and I cannot use it.
Is this intended? do I have to save an address in the address book to send funds?

This is a valid address I dont have in the address book
image

483:
It seems that the first focus is done in the "X" to close the popUp, since I have to tab 3 times to reach the "Recipient" field. Probably is not ok, but still is not defined where the focus should be when you open "send funds" or "custom tx"

@lukasschor @mikheevm

@fernandomg
Copy link
Contributor

in regard to #483 observation, it's doable if this is the intended behavior:

autofocus-on-loading

@lukasschor
Copy link
Member

Copy-pasting an address should definitely validate it. Also, ENS validation does not work.

@ghost
Copy link

ghost commented Jan 30, 2020

Travis automatic deployment:
https://pr461--safereact.review.gnosisdev.com

@ghost
Copy link

ghost commented Jan 30, 2020

Travis automatic deployment:
https://pr461--safereact.review.gnosisdev.com

@francovenica
Copy link
Contributor

The focus on the field when opening "send funds" or "custom tx" is working fine.

The validation on the recipient field when copy-pasting is still missing

@ghost
Copy link

ghost commented Jan 31, 2020

Travis automatic deployment:
https://pr461--safereact.review.gnosisdev.com

The title of review custom tx was "Send Funds" instead of "Send Custom Tx"
The title of review custom tx was "Send Funds" instead of "Send Custom Tx"
@ghost
Copy link

ghost commented Jan 31, 2020

Travis automatic deployment:
https://pr461--safereact.review.gnosisdev.com

@francovenica
Copy link
Contributor

Update in the issues reported:
#477
#478
#479
#480
#481
#482

Besides the 3 open issues there are, there is now an issue in the PR461 where no address are shown for custom transaction. At some point it was tried to be implemented that only contract address would show in the selector for custom Tx, but not even those are showing.
I've tested with the WBTC contract: 0x577D296678535e4903D59A4C929B718e1D575e0A

@lukasschor @Agupane

@lukasschor
Copy link
Member

Besides the 3 open issues there are, there is now an issue in the PR461 where no address are shown for custom transaction. At some point it was tried to be implemented that only contract address would show in the selector for custom Tx, but not even those are showing.
I've tested with the WBTC contract: 0x577D296678535e4903D59A4C929B718e1D575e0A

I think we should merge this and create a seperate ticket for the mentioned issue. This does not seem like a high-priority bug to me.

@Agupane
Copy link
Contributor Author

Agupane commented Feb 3, 2020

I agree, I'll merge this pr and will continue working on the issues related to the feature in other prs

@Agupane Agupane merged commit 226b525 into development Feb 3, 2020
@fernandomg fernandomg mentioned this pull request Feb 5, 2020
@Agupane Agupane deleted the 388-adbk-new-transactions branch April 29, 2020 18:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants