Skip to content
This repository was archived by the owner on Jun 24, 2022. It is now read-only.

1334/load new tokens not yet loaded #1504

Merged

Conversation

alfetopito
Copy link
Contributor

@alfetopito alfetopito commented Oct 1, 2021

Summary

Waterfalls into #1395

Loading tokens from the chain when some order has a token not present in any loaded list

To Test

  1. On rinkeby, trade ZRX token
  2. On a different browser/device, connect the same account
  • On the activity list you should be able to see the order with ZRX token
  • ZRX should be shown as user added token
    screenshot_2021-10-01_15-49-10
  1. Remove the token and refresh the page
  • Token should be added back as user token

NOT addressed here

@alfetopito alfetopito self-assigned this Oct 1, 2021
@alfetopito alfetopito force-pushed the 1334/activity-orders-from-api branch from 85a4436 to 21971ed Compare October 5, 2021 18:30
@alfetopito alfetopito force-pushed the 1334/load-new-tokens-not-yet-loaded branch from f8a5d5c to f9c2a52 Compare October 5, 2021 18:41
@github-actions
Copy link
Contributor

github-actions bot commented Oct 5, 2021

  • 🔭 GP Swap: Gnosis Protocol v2 Swap UI

@elena-zh
Copy link

elena-zh commented Oct 6, 2021

HI @alfetopito ,
changes LGTM: I see all activity logs with custom tokens, tokens appear in the custom token list even if they were previously deleted.
However, I'd like mention this tiny UI issues that is related to all latest PRs: wallet icon is clued to a wallet address in the side bar. May be we could fix it somehow in the current PR?
icon

@alfetopito
Copy link
Contributor Author

HI @alfetopito , changes LGTM: I see all activity logs with custom tokens, tokens appear in the custom token list even if they were previously deleted. However,

Thanks for the early review Elena!

I still have to work a bit more on this PR though.

I'd like mention this tiny UI issues that is related to all latest PRs: wallet icon is clued to a wallet address in the side bar. May be we could fix it somehow in the current PR? icon

That change might be addressed by @biocom on his second iteration of the orders panel

@alfetopito alfetopito force-pushed the 1334/activity-orders-from-api branch 2 times, most recently from decfc22 to fd9f391 Compare October 7, 2021 20:53
@alfetopito alfetopito force-pushed the 1334/load-new-tokens-not-yet-loaded branch from f0e3d84 to 0daf32d Compare October 7, 2021 22:51
@alfetopito alfetopito marked this pull request as ready for review October 7, 2021 23:00
@alfetopito alfetopito requested review from a team October 7, 2021 23:00
Copy link

@MareenG MareenG left a comment

Choose a reason for hiding this comment

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

LGTM and works as expected.

Only one thing I noticed which is not super important is that when you click clear all the tokens remain in the list as intended but as a user this could also look like bug.

Bildschirmaufnahme.2021-10-08.um.08.36.35.mov

The other question would be then how could I remove the tokens if I want to?
Should we add some blurb to explain this?

@alfetopito
Copy link
Contributor Author

LGTM and works as expected.

Only one thing I noticed which is not super important is that when you click clear all the tokens remain in the list as intended but as a user this could also look like bug.
Bildschirmaufnahme.2021-10-08.um.08.36.35.mov

The other question would be then how could I remove the tokens if I want to? Should we add some blurb to explain this?

The behaviour changed a little on the next PR #1536 as in, the tokens will not be added back immediately.
But they will on refresh, anyway.

I haven't given much thought about this. I decided to re-use the existing logic for user added tokens for simplicity.

If this is a problem, we could do one or a combination of:

  1. Save tokens in a new category, other than user-added tokens
  2. Have a flag on these tokens with a tooltip explaining they were auto-added
  3. Keep as is, and add a tooltip on token management modal saying that some tokens were auto-added

alfetopito and others added 3 commits October 12, 2021 15:37
* Removing annoying log

* No longer re-fetching orders when a token is added to the list

Co-authored-by: Leandro Boscariol <[email protected]>
- Added contracts cache
- Improved comments
@alfetopito alfetopito requested a review from anxolin October 12, 2021 23:25
@alfetopito alfetopito merged commit 7c2836f into 1334/activity-orders-from-api Oct 15, 2021
@alfetopito alfetopito deleted the 1334/load-new-tokens-not-yet-loaded branch October 15, 2021 18:48
alfetopito added a commit that referenced this pull request Oct 15, 2021
* Added non-hook methods for dealing with contracts

* New hook useTokensLazy

* Exporting stuff from original codebase

* Added getMultipleCallsResults under custom/state/multicall/utils

Non-hook way of fetching multiple onchain queries through multicall

* Refactor ApiOrdersUpdater to load tokens not yet in the UI

* Typo fix

* 1334/do not refetch when token added (#1536)

* Removing annoying log

* No longer re-fetching orders when a token is added to the list

Co-authored-by: Leandro Boscariol <[email protected]>

* Refactored useTokensLazy:

- Added contracts cache
- Improved comments

* Improved doc string on multicall utils

Co-authored-by: Leandro Boscariol <[email protected]>
alfetopito added a commit that referenced this pull request Oct 21, 2021
* Added non-hook methods for dealing with contracts

* New hook useTokensLazy

* Exporting stuff from original codebase

* Added getMultipleCallsResults under custom/state/multicall/utils

Non-hook way of fetching multiple onchain queries through multicall

* Refactor ApiOrdersUpdater to load tokens not yet in the UI

* Typo fix

* 1334/do not refetch when token added (#1536)

* Removing annoying log

* No longer re-fetching orders when a token is added to the list

Co-authored-by: Leandro Boscariol <[email protected]>

* Refactored useTokensLazy:

- Added contracts cache
- Improved comments

* Improved doc string on multicall utils

Co-authored-by: Leandro Boscariol <[email protected]>
alfetopito added a commit that referenced this pull request Nov 29, 2021
* Added /account/<address>/orders api endpoint

* Added helper function to get explorer address link

* Requiring needed properties instead of concrete types on utils fns

* Added addOrUpdateOrdersBatch to orders/actions

* Added useAddOrUpdateOrdersBatch to orders/hooks

* Added addOrUpdateOrdersBatch to orders/reducer

* Added APIOrdersUpdater that checks for orders from API on connection

* Adjusted displayed orders:

- At most 10 regular orders and as much pending as there are
- Show only connected account orders
- Sort orders by creation date descending

* Show only txs for connected account

* Refactoring

* Don't actually need a helper fn...

* Added comment to make sort more explicit

* Using 'Link to Explorer' instead of 'Clear activities'

* Refactoring

* Refactor: cache tokens loaded const

* Refactor: removed duplicated css property

* Refactor: removed duplicated CSS property

* Refactor: extracted function transformApiOrderToStoreOrder

* Refactor: renamed _orders to apiOrders

* Refactor: removed duplicated CSS property

* Adjusting `View all orders` link size

* Added `View all orders` to the bottom of account activity

* Refactored computeOrderSummary to deal with pending orders

* Computing order summary for orders loaded from API

* Setting isCancelling state if order is pending and cancelled on API

* Actually syncing `isCancelling` flag for existing orders

* Refactor: created const for magic number MAXIMUM_ORDERS_TO_DISPLAY

* Refactor: simplified addOrUpdateOrdersBatch reducer

* Refactor: renamed API -> Api on file name

* Refactor: replaced magic number with AMOUNT_OF_ORDERS_TO_FETCH

* Refactor: converted classifyLocalStatus fn to a map

* Refactor: removed `Batch` from addOrUpdateOrders

* Refactor: moved error exit condition up

* 1334/load new tokens not yet loaded (#1504)

* Added non-hook methods for dealing with contracts

* New hook useTokensLazy

* Exporting stuff from original codebase

* Added getMultipleCallsResults under custom/state/multicall/utils

Non-hook way of fetching multiple onchain queries through multicall

* Refactor ApiOrdersUpdater to load tokens not yet in the UI

* Typo fix

* 1334/do not refetch when token added (#1536)

* Removing annoying log

* No longer re-fetching orders when a token is added to the list

Co-authored-by: Leandro Boscariol <[email protected]>

* Refactored useTokensLazy:

- Added contracts cache
- Improved comments

* Improved doc string on multicall utils

Co-authored-by: Leandro Boscariol <[email protected]>

* Fix new Erc20 import path

* Adjust to new multicall interface and fetchChunk changes

* Adapt to renamed of ApiOrderStatus to OrderTransitionStatus

* Adjusting (again) styles for `view all activity`

* Handling presignaturePending state from backend

* The order is concrete at this point, no need for `?.`

* 1334/only update connected wallet (#1684)

* Only update cancelled orders for connected wallet

* Only check for unfillable orders for connected wallet

* Only check pending orders for connected wallet

* Always create a new state object when adding an order (#1686)

Co-authored-by: Leandro Boscariol <[email protected]>

* Refactor: comparing account with order.owner lowercased for safety

Co-authored-by: Leandro Boscariol <[email protected]>

* Added new hook useTokenLazyNoMulticall to fetch token data one by one

* Using new hook to fetch tokens instead of multicall

* Removed multicall/utils

* Removed useTokensLazy hook

* Renamed file useTokensLazy to useTokenLazy

* Renamed useTokenLazyNoMulticall to useTokenLazy

* Removed unused import

* Updated debug log messages

* Small refactor in #1639 (#1857)

* Only init bytes32Contract if we really need to

* Fixing build error: only proceed if library is set

Co-authored-by: Leandro Boscariol <[email protected]>

* 1334/with swr (#1906)

* Refactor: removed unnecessary variable

* Added SWR as dev dependency

* Added hook useApiOrders using useSWR hook

* Using new hook for fetching api orders

* Removed unused imports...

Co-authored-by: Leandro Boscariol <[email protected]>

Co-authored-by: Leandro Boscariol <[email protected]>
Co-authored-by: Anxo Rodriguez <[email protected]>
anxolin pushed a commit to anxolin/cowswap that referenced this pull request Apr 1, 2022
* Added non-hook methods for dealing with contracts

* New hook useTokensLazy

* Exporting stuff from original codebase

* Added getMultipleCallsResults under custom/state/multicall/utils

Non-hook way of fetching multiple onchain queries through multicall

* Refactor ApiOrdersUpdater to load tokens not yet in the UI

* Typo fix

* 1334/do not refetch when token added (gnosis/cowswap#1536)

* Removing annoying log

* No longer re-fetching orders when a token is added to the list

Co-authored-by: Leandro Boscariol <[email protected]>

* Refactored useTokensLazy:

- Added contracts cache
- Improved comments

* Improved doc string on multicall utils

Co-authored-by: Leandro Boscariol <[email protected]>
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.

5 participants