Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support cell data decoding #267

Conversation

PainterPuppets
Copy link

Copy link

vercel bot commented Mar 17, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ckb-explorer-frontend-in-magickbase-repo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 29, 2024 8:46am

const DECODER = {
utf8: 'UTF-8',
number: 'Number',
// todo: support decode address & Big-Endian
Copy link
Member

@Keith-CY Keith-CY Mar 18, 2024

Choose a reason for hiding this comment

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

Suggested change
// todo: support decode address & Big-Endian
// todo: support decode address & Little-Endian & JSON string

Copy link
Author

Choose a reason for hiding this comment

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

Here I have a bit of a question, what should be the decode rule for address? Is hex used as a public key and then parse to address or what does it mean?

And are the rules for decoding json different from utf-8?

Copy link
Member

Choose a reason for hiding this comment

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

I think parsing address into a script is pretty enough

@PainterPuppets
Copy link
Author

Updated.

Support for Witness and adding more parsing methods will be further upgraded in the other pr

@Keith-CY
Copy link
Member

Conflicted

@PainterPuppets
Copy link
Author

Resloved

@@ -0,0 +1,87 @@
import { useMemo, useRef, useState } from 'react'
import { Select } from 'antd'
Copy link
Member

Choose a reason for hiding this comment

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

It's better not to use antd because it's hard to customize

@Sven-TBD
Copy link

While checking address detail page, it shows like this
image

@Keith-CY
Copy link
Member

@FrederLu
Copy link

https://ckb-explorer-frontend-in-magickbase-repo-2es2w4zo8-magickbase.vercel.app/address/ckt1qzda0cr08m85hc8jlnfp3zer7xulejywt49kt2rr0vthywaa50xwsqtyy4lspd4k86v8vz06n03dpjrdx5gzp7cxulwv8
image
The api data is displayed normally, but the address data, balance, live cells, and total number of transactions on the page are all 0.

@PainterPuppets
Copy link
Author

https://ckb-explorer-frontend-in-magickbase-repo-2es2w4zo8-magickbase.vercel.app/address/ckt1qzda0cr08m85hc8jlnfp3zer7xulejywt49kt2rr0vthywaa50xwsqtyy4lspd4k86v8vz06n03dpjrdx5gzp7cxulwv8 image The api data is displayed normally, but the address data, balance, live cells, and total number of transactions on the page are all 0.

Checked and found that it's not caused by this pr, I'll follow up on this issue in other pr

@Keith-CY
Copy link
Member

ckb-explorer-frontend-in-magickbase-repo-2es2w4zo8-magickbase.vercel.app/address/ckt1qzda0cr08m85hc8jlnfp3zer7xulejywt49kt2rr0vthywaa50xwsqtyy4lspd4k86v8vz06n03dpjrdx5gzp7cxulwv8 image The api data is displayed normally, but the address data, balance, live cells, and total number of transactions on the page are all 0.

Checked and found that it's not caused by this pr, I'll follow up on this issue in other pr

Caused by Magickbase/ckb-explorer-public-issues#580, a PR to improve the compatibility will be added to the develop branch

@PainterPuppets
Copy link
Author

Changed the decode window to the global, I tried it myself and felt it was fine, but it feels like may need to test more into whether it will lead to a less experience in any areas

@Keith-CY
Copy link
Member

Changed the decode window to the global, I tried it myself and felt it was fine, but it feels like may need to test more into whether it will lead to a less experience in any areas

Hot zone could be marked to trigger this popover, I'll propose a PR to this branch

Copy link
Member

Choose a reason for hiding this comment

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

Are these update still necessary when the selection is listened globally

Copy link
Author

@PainterPuppets PainterPuppets Mar 31, 2024

Choose a reason for hiding this comment

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

These code is to make the component look the same as the design, I'll remove it if our plan to update it later.

@Keith-CY
Copy link
Member

Hold on because a quick/simple implementation(nervosnetwork@4710e4c) was proposed to cater for recent projects

@PainterPuppets
Copy link
Author

Hold on because a quick/simple implementation(nervosnetwork@4710e4c) was proposed to cater for recent projects

Maybe this pr can be close?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants