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

Refactor capacity component #181

Merged
merged 7 commits into from
Dec 26, 2023
Merged

Refactor capacity component #181

merged 7 commits into from
Dec 26, 2023

Conversation

Keith-CY
Copy link
Member

@Keith-CY Keith-CY commented Dec 22, 2023

Copy link

vercel bot commented Dec 22, 2023

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 Dec 26, 2023 9:40am

src/components/Capacity/Capacity.module.scss Outdated Show resolved Hide resolved
src/pages/Home/TableCard/index.module.scss Outdated Show resolved Hide resolved
@Keith-CY Keith-CY changed the base branch from fix-style-of-capacity-change to develop December 25, 2023 08:41
@WhiteMinds
Copy link

This PR is based on #180, and now we need to remove unnecessary changes from #180.

@Keith-CY
Copy link
Member Author

This PR is based on #180, and now we need to remove unnecessary changes from #180.

Updated and scss variables are used in media queries

@Keith-CY
Copy link
Member Author

UI: image

1、The color of the data () in the transaction page Lite is abnormal. ckb-explorer-frontend-in-magickbase-repo-r61x41klx-magickbase.vercel.app/transaction/0x1d320432dbe19599eb752d835be9877a596870b46980dd68edad3abd1fd10111?layout=lite image

2、The id information style and () color are inconsistent with before. ckb-explorer-frontend-in-magickbase-repo-r61x41klx-magickbase.vercel.app/transaction/0xb730c4ddaf41272532a809383b45731de990971bdee9eecfbe22fd5af816fdb1?layout=lite image

Previous style: image

3、When there is a label, the distance between it and the following information is small and needs to be adjusted. ckb-explorer-frontend-in-magickbase-repo-r61x41klx-magickbase.vercel.app/transaction/0x7ab270c95cb31d1ee32b1b56433248c0a73bb462536a20df873122f8b6bfa7e1?layout=lite image

The color of NFT ID and the distance between label and amount will be fixed.

The color of () need more detail because in previous design, the A and B in A(B) wre assumed to be the same, so the () has the same color. But in fact A and B can be colored differently, so the color of () should be specified to be the same as A or B cc @Sven-TBD

@Keith-CY
Copy link
Member Author

Keith-CY commented Dec 26, 2023

Ah, I recalled that color of NFT is not set because the direction of NFT is not set, as mentioned at Magickbase/ckb-explorer-public-issues#503

They are previously colored because the assumption that NFT is sent when the capacity is negative, or NFT is received when the capacity is positive. But the assumption is invalid.

So there need more detail returned from API to set the color of NFT ID cc @FrederLu

@FrederLu
Copy link

The color of NFT ID and the distance between label and amount will be fixed.
The color of () need more detail because in previous design, the A and B in A(B) wre assumed to be the same, so the () has the same color. But in fact A and B can be colored differently, so the color of () should be specified to be the same as A or B cc @Sven-TBD

Ah, I recalled that color of NFT is not set because the direction of NFT is not set, as mentioned at Magickbase/ckb-explorer-public-issues#503
They are previously colored because the assumption that NFT is sent when the capacity is negative, or NFT is received when the capacity is positive. But the assumption is invalid.
So there need more detail returned from API to set the color of NFT ID cc @FrederLu

To sum up, the label position can now be fine-tuned, and the rest needs to be processed later.

@Keith-CY
Copy link
Member Author

The color of NFT ID and the distance between label and amount will be fixed.
The color of () need more detail because in previous design, the A and B in A(B) wre assumed to be the same, so the () has the same color. But in fact A and B can be colored differently, so the color of () should be specified to be the same as A or B cc @Sven-TBD

Ah, I recalled that color of NFT is not set because the direction of NFT is not set, as mentioned at Magickbase/ckb-explorer-public-issues#503
They are previously colored because the assumption that NFT is sent when the capacity is negative, or NFT is received when the capacity is positive. But the assumption is invalid.
So there need more detail returned from API to set the color of NFT ID cc @FrederLu

To sum up, the label position can now be fine-tuned, and the rest needs to be processed later.

Yes

@FrederLu
Copy link

The color of NFT ID and the distance between label and amount will be fixed.
The color of () need more detail because in previous design, the A and B in A(B) wre assumed to be the same, so the () has the same color. But in fact A and B can be colored differently, so the color of () should be specified to be the same as A or B cc @Sven-TBD

Ah, I recalled that color of NFT is not set because the direction of NFT is not set, as mentioned at Magickbase/ckb-explorer-public-issues#503
They are previously colored because the assumption that NFT is sent when the capacity is negative, or NFT is received when the capacity is positive. But the assumption is invalid.
So there need more detail returned from API to set the color of NFT ID cc @FrederLu

To sum up, the label position can now be fine-tuned, and the rest needs to be processed later.

Yes

OK. The style here will be different from the previous design. cc @Sven-TBD

@Keith-CY
Copy link
Member Author

@Keith-CY Keith-CY merged commit 57184a4 into develop Dec 26, 2023
8 checks passed
@Keith-CY Keith-CY deleted the refactor-capacity branch December 26, 2023 10:05
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.

3 participants