Magickbase / godwoken_explorer

Godwoken Explorer
https://v1.gwscan.com
18 stars 8 forks source link

data sorting/filtering #556

Open Keith-CY opened 2 years ago

Keith-CY commented 2 years ago

Simple ERC20 transfer list

  1. filtered by from: https://github.com/Magickbase/godwoken-explorer-ui/pull/235
  2. filtered by to: https://github.com/Magickbase/godwoken-explorer-ui/pull/235

ERC20 transfer list

  1. filtered by block number (from and to) https://github.com/Magickbase/godwoken-explorer-ui/pull/720
  2. filtered by from https://github.com/Magickbase/godwoken-explorer-ui/pull/720
  3. filtered by to https://github.com/Magickbase/godwoken-explorer-ui/pull/720
  4. filtered by age (from and to) https://github.com/Magickbase/godwoken-explorer-ui/pull/720
  5. filtered by token type

Bridge transfer list

  1. filtered by age (from and to)
  2. filtered by type

Transaction list

  1. filtered by block number (from and to) https://github.com/Magickbase/godwoken-explorer-ui/pull/740
  2. filtered by method id/name https://github.com/Magickbase/godwoken-explorer-ui/pull/740
  3. filtered by from https://github.com/Magickbase/godwoken-explorer-ui/pull/740
  4. filtered by to https://github.com/Magickbase/godwoken-explorer-ui/pull/740
  5. filtered by age (from and to) https://github.com/Magickbase/godwoken-explorer-ui/pull/740

Contract list

  1. sorted by balance
  2. sorted by tx count

PR:

Token list

  1. sorted by total supply/circulating supply
  2. sorted by holders
  3. sorted by name
  4. filtered by name (fuzzy): https://github.com/Magickbase/godwoken-explorer-ui/pull/391

PR:

@Kirl70 Please add the filter/sort component in design draft

Keith-CY commented 2 years ago

Since we are going to use graphql in UI, maybe we can just implement them in graphql only? @zmcNotafraid @Naupio

Naupio commented 2 years ago

graphql token list (udt list) enhancement link https://github.com/Magickbase/godwoken_explorer/issues/625

Keith-CY commented 2 years ago

All APIs are ready, will be updated in front-end by 08/03

Keith-CY commented 1 year ago

I still don't have bandwidth for these features so request help from @qiweiii and @JeffreyMa597

The main goal is simple, add order or filter functions in the table/list as https://www.gwscan.com/tx/0xde6f86bcead05320f667d2422985db6043aadbff3297d8cb026b7db30d84365b image

ERC 20 transfer list, bridge transfer list and transaction list for @qiweiii Contract list, token list for @JeffreyMa597

Is that ok?

PS: PR for each table/list could be submitted separately.

qiweiii commented 1 year ago

👍

qiweiii commented 1 year ago

a few questions abt api @Naupio @Keith-CY :

  1. seems token_transfers does not support filter by token type/name, do we need to add this feature?
  2. Does combine_from_to mean filter by both from_address and to_address?

questions abt product/design:

  1. how to allow user to choose age range, using string input format like yyyy-mm-dd or using a date picker component
  2. In user account page, if user filter erc20 transfers list by from , does it show all txs with this from address or only this user’s txs with this from address?
Keith-CY commented 1 year ago

a few questions abt api @Naupio @Keith-CY :

  1. seems token_transfers does not support filter by token type/name, do we need to add this feature?
  2. Does combine_from_to mean filter by both from_address and to_address?
  1. Filtering by token type/name is a good idea and you can set a API request to @Naupio
  2. combine_from_to: true means filtering by from_address || to_address, it was used as
    from_address: A
    to_address: A
    combine_from_and_to: true // default

    So transactions sent by A and transactions sent to A are all returned

questions abt product/design:

  1. how to allow user to choose age range, using string input format like yyyy-mm-dd or using a date picker component
  2. In user account page, if user filter erc20 transfers list by from , does it show all txs with this from address or only this user’s txs with this from address?
  1. a text field of yyyy-mm-dd with a datetime picker could be used, just as the native HTML component, its layout/appearance could be desicded by @Kirl70
  2. here's a misleading, erc20 transfers are a group of logs emitted by erc20 contracts, a log looks like event Transfer(address indexed from, address indexed to, uint256 value)(source code), when from filter is adopted, logs which has the specific from address as its address indexed from are displayed.
Naupio commented 1 year ago

@qiweiii

  1. Here has three api for token_transfer, token_transfers for erc20, erc721_token_transfers for erc721, erc1155_token_transfers for erc1155, token_name filter does not exist now, we can add it.
  2. if combine_from_to is true, then from_address and to_address are combined into query condition like address = from_address OR address = to_address and the default value is true, if combine_from_to set to false, the query strategy here like address = from_address AND address = to_address
Kirl70 commented 1 year ago

@qiweiii

Date selector style as shown in the figure, detailed time selection using the framework that comes with the component, note that the selection color needs to be consistent with the theme color of the page.

https://www.figma.com/file/6XNoimRDbFTTNm016rbIdU/Magickbase?node-id=2319%3A17211&t=POEXN65s7iY8Mw4k-1

image
alexsupa597 commented 1 year ago

@Keith-CY @Naupio Hi, Team. I also have a few questions about this issue:

Contract list 1、Can all fields sorts be combined and used at the same time? Such as sorting by balance in "ASC" and sorting by tx count in "DESC" at the same time. 2、The value of sorted by balance is missing from the sort_value in the API. 3、Does the EX_TX_COUNT in the sort_value refer to the tx count in the description? 4、In our page, I found both fetch and graphql are used to obtain our list data. However, we only used the loading state of graphql request, and the data in the list used the data obtained by the fetch request. I wonder if there is any historical reason for this? Do I need to make any changes? 5、Currentpage is used for page-turning, do I need to change the cursor for page-turning?

image

Token list 1、Does this page means the page of Bridged Token List? Or is the page of Layer 2 Native Token List also included in it? 2、The name column is not in the list, so I need to add it, right?

Keith-CY commented 1 year ago

@Keith-CY @Naupio Hi, Team. I also have a few questions about this issue:

Contract list 1、Can all fields sorts be combined and used at the same time? Such as sorting by balance in "ASC" and sorting by tx count in "DESC" at the same time.

Sure, sorters could be used together

2、The value of sorted by balance is missing from the sort_value in the API. 3、Does the EX_TX_COUNT in the sort_value refer to the tx count in the description?

To @Naupio

4、In our page, I found both fetch and graphql are used to obtain our list data. However, we only used the loading state of graphql request, and the data in the list used the data obtained by the fetch request. I wonder if there is any historical reason for this? Do I need to make any changes?

It should be a historical problem, we will use graphql for all network requests to backend server

5、Currentpage is used for page-turning, do I need to change the cursor for page-turning?

Cursors returned from the APIs are pointing to other pages, so they should be set in the link to other pages.

image

Token list 1、Does this page means the page of Bridged Token List? Or is the page of Layer 2 Native Token List also included in it?

Token list pages was including bridged token list and native token list, now it should also include erc721 token list and erc1155 token list

2、The name column is not in the list, so I need to add it, right?

The name field was changed to the token field

alexsupa597 commented 1 year ago

@Keith-CY Thanks for your reply. There are some other questions after that:

1、Cursors returned from the APIs are pointing to other pages, so they should be set in the link to other pages.

I mean if we use grapgql request to get our data, I'll change the page-turning by using cursor.

2、Token list pages was including bridged token list and native token list, now it should also include erc721 token list and erc1155 token list

That means the whole pages in the Token selector on the home page.

3、The name field was changed to the token field

I found we use name to represent the token in our code. That means I should sort the list by using name instead of by using token?

image

4、The field of total supply is not in the list item of list 721. Do I need to add the display of this field?

Keith-CY commented 1 year ago

I mean if we use grapgql request to get our data, I'll change the page-turning by using cursor.

Sure, pagination should be implemented with cursor instead of page number

That means the whole pages in the Token selector on the home page.

Yes

I found we use name to represent the token in our code. That means I should sort the list by using name instead of by using token?

Correct, it's displayed with token icon/cover but sorted by token name

4、The field of total supply is not in the list item of list 721. Do I need to add the display of this field?

total supply could be ignored because minted count of ERC 721 is the total supply of ERC 20

alexsupa597 commented 1 year ago

@Naupio These values we will use to sort are missing in our APIs. Please help to add the following values, thanks.

image
alexsupa597 commented 1 year ago

PR for contract list: https://github.com/Magickbase/godwoken-explorer-ui/pull/735

alexsupa597 commented 1 year ago

PR for Bridge Token and Layer 2 Native Token: https://github.com/Magickbase/godwoken-explorer-ui/pull/736

alexsupa597 commented 1 year ago

PR for NFT collections: https://github.com/Magickbase/godwoken-explorer-ui/pull/737

alexsupa597 commented 1 year ago

@Keith-CY

4、The field of total supply is not in the list item of list 721. Do I need to add the display of this field? total supply could be ignored because minted count of ERC 721 is the total supply of ERC 20

How about the ERC1155?

Keith-CY commented 1 year ago

@Keith-CY

4、The field of total supply is not in the list item of list 721. Do I need to add the display of this field? total supply could be ignored because minted count of ERC 721 is the total supply of ERC 20

How about the ERC1155?

List of ERC 1155 has two special columns:

These two columns should support sorting and total supply could be ignored because type type of erc 1155 is similar to total supply of erc 20

alexsupa597 commented 1 year ago

These two columns should support sorting and total supply could be ignored because type type of erc 1155 is similar to total supply of erc 20

ok, @Naupio

total supply could be ignored

Is that mean we don't have to use minted Count instead of total supply to sort?

Keith-CY commented 1 year ago

These two columns should support sorting and total supply could be ignored because type type of erc 1155 is similar to total supply of erc 20

ok, @Naupio

total supply could be ignored

Is that mean we don't have to use minted Count instead of total supply to sort?

It's not necessary for the page

alexsupa597 commented 1 year ago

@Naupio The latest values we should confirm and add.

image
alexsupa597 commented 1 year ago

PR for Multi collections: https://github.com/Magickbase/godwoken-explorer-ui/pull/738

qiweiii commented 1 year ago

@Keith-CY @Naupio For bridged transfers list

  1. now it's using fetch to get data, I found there is a graphql api account_current_bridged_udts provides similar data but not 100% same, also it does not provide sorter/filter input like other endpoints, do we need another api request or which endpoint should we use?
  2. For current headers in this table, which column do we need to filter/sort?

Image

Keith-CY commented 1 year ago

@Keith-CY @Naupio For bridged transfers list

  1. now it's using fetch to get data, I found there is a graphql api account_current_bridged_udts provides similar data but not 100% same, also it does not provide sorter/filter input like other endpoints, do we need another api request or which endpoint should we use?
  2. For current headers in this table, which column do we need to filter/sort?

Image

It's a good time to replace the restful API with graphql API @Naupio

Sorted by age and filtered by type seem good to me.

Naupio commented 1 year ago

@Naupio The latest values we should confirm and add. image

ex_tx_count sorter use for sort by transaction count with smart contract list

Naupio commented 1 year ago

add balance sorter link issue: #1172

Naupio commented 1 year ago

add minted count sorter for erc721_udts #1173

Naupio commented 1 year ago

add type count sorter for erc1155_udts #1174

alexsupa597 commented 1 year ago

@Naupio Once you finished adding these values, please tell me to finish my PR. Thanks a lot.

Naupio commented 1 year ago

@Keith-CY @Naupio For bridged transfers list

  1. now it's using fetch to get data, I found there is a graphql api account_current_bridged_udts provides similar data but not 100% same, also it does not provide sorter/filter input like other endpoints, do we need another api request or which endpoint should we use?
  2. For current headers in this table, which column do we need to filter/sort?

Image

It's a good time to replace the restful API with graphql API @Naupio

Sorted by age and filtered by type seem good to me.

add new graphql api for deposits and withdrawals history #1186

Keith-CY commented 1 year ago

Maybe sorting should not be composed with each other. For example, on page https://www.gwscan.com/tokens/bridge?name=&page_size=30&holder_count_sort=DESC&name_sort=DESC&supply_sort=ASC

holder count is prior to other sorting fields, so other fields are definitely useless because most tokens have different holder count, but they are ordered by holder count first, which means the order is fiexed. It's impossible to list records by circulating supply.

image

Correct me if I'm wrong @Naupio @JeffreyMa597 @qiweiii @FrederLu

But sorting and filtering could work together.

qiweiii commented 1 year ago

Yea it make sense, maybe there should be only one active sorter, so it doesn't confuse users

Multiple filters can definitely work well, so we can allow multiple filter + one sorter

Naupio commented 1 year ago

@Naupio Once you finished adding these values, please tell me to finish my PR. Thanks a lot. @JeffreyMa597

issue: https://github.com/Magickbase/godwoken_explorer/issues/1172 https://github.com/Magickbase/godwoken_explorer/issues/1173 https://github.com/Magickbase/godwoken_explorer/issues/1174

pr: https://github.com/Magickbase/godwoken_explorer/pull/1257 https://github.com/Magickbase/godwoken_explorer/pull/1279 https://github.com/Magickbase/godwoken_explorer/pull/1280

alexsupa597 commented 1 year ago

Now rest PRs in QA: https://github.com/Magickbase/godwoken-explorer-ui/pull/738 https://github.com/Magickbase/godwoken-explorer-ui/pull/737 https://github.com/Magickbase/godwoken-explorer-ui/pull/735

FrederLu commented 1 year ago

Now rest PRs in QA: Magickbase/godwoken-explorer-ui#738 Magickbase/godwoken-explorer-ui#737 Magickbase/godwoken-explorer-ui#735

735/737/738 verified.