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.

Sign up for GitHub

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

Jump to bottom

Improvement blocks listing page #1013

Merged
merged 11 commits into from Mar 22, 2022
Merged

Improvement blocks listing page #1013

merged 11 commits into from Mar 22, 2022

Conversation

AnthonyLaw
Copy link
Member

@AnthonyLaw AnthonyLaw commented Dec 17, 2021 •
edited

Current behavior

  • Blocks listing page's table didn't show block reward info.
  • Blocks listing page's top-right corner display chain height info.
  • Blocks listing page's table header include type and timestamp columns.

What was the issue?

  • Inflation was missing on the block listing page.
  • Chain height info on the top-right is duplicated, the user always can get the latest height from the table first row.
  • The type and timestamp columns info are not useful for users.

What's the fix?

  • Add a new column Block Reward ('inflation') on the blocks listing page.
  • Remove 'Chain Height' on the top right.
  • Remove the type and timestamp columns.
  • Reorder table header Height, Created, Harvester, Block Reward, Fees

image

@netlify
Copy link

netlify bot commented Dec 17, 2021 •
edited

✅ Deploy Preview for explorer-dev ready!

🔨 Explore the source changes: c46e131

🔍 Inspect the deploy log: https://app.netlify.com/sites/explorer-dev/deploys/6238b35e187d3400082f2a5b

😎 Browse the preview: https://deploy-preview-1013--explorer-dev.netlify.app/

@AnthonyLaw AnthonyLaw marked this pull request as ready for review December 17, 2021 05:26
@AnthonyLaw AnthonyLaw linked an issue Dec 17, 2021 that may be closed by this pull request
Improvement for blocks listing page #1001
Closed
@OlegMakarenko
Copy link
Contributor

add inflation.json

@AnthonyLaw
Is it the same for all networks? Should we consider returning this data from REST?

@AnthonyLaw
Copy link
Member Author

Is it the same for all networks? Should we consider returning this data from REST?

@Wayonb what do you think about this add inflation data on REST?

@Wayonb
Copy link
Contributor

Wayonb commented Dec 23, 2021 •
edited

@Wayonb what do you think about this add inflation data on REST?

Yes, inflation data can be different per network and would be better coming from REST.
I havent looked at the PR in detail but why are you hardcoding the inflation?

@AnthonyLaw
Copy link
Member Author

@Wayonb what do you think about this add inflation data on REST?

Yes, inflation data can be different per network and would be better coming from REST. I havent looked at the PR in detail but why are you hardcoding the inflation?

Reason for hardcore, because I think the value is fixed.
right now in Rest is only able to retrieve the single inflation rate from each block, using transaction statement and filter with receipt type 20803

it may not well option to fire 10 requests to get an inflation rate from 10 blocks.

Jaguar0625
Jaguar0625 requested changes Jan 4, 2022
View reviewed changes
src/config/inflation.json Outdated Show resolved Hide resolved
src/helper.js Outdated Show resolved Hide resolved
@Wayonb
Copy link
Contributor

Wayonb commented Jan 4, 2022

right now in Rest is only able to retrieve the single inflation rate from each block, using transaction statement and filter with receipt type 20803

Cann't you use the fromHeight and toHeight filter options so something like this?
https://001-joey-dual.symboltest.net:3001/statements/transaction?receiptType=20803&fromHeight=10000&toHeight=10010

@AnthonyLaw
Copy link
Member Author

https://001-joey-dual.symboltest.net:3001/statements/transaction?receiptType=20803&fromHeight=10000&toHeight=10010

oh, I miss out on this endpoint. thanks for remind 👍🏼

@sonarcloud
Copy link

sonarcloud bot commented Jan 4, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@Jaguar0625
Copy link

please remember to add some regression tests for this change

@AnthonyLaw AnthonyLaw force-pushed the gh1001-improvement-blocks-list branch from 33598cb to 3626627 Compare March 21, 2022 17:09
@sonarcloud
Copy link

sonarcloud bot commented Mar 21, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@AnthonyLaw AnthonyLaw merged commit 7f55187 into dev Mar 22, 2022
@AnthonyLaw AnthonyLaw deleted the gh1001-improvement-blocks-list branch March 22, 2022 16:55
Wayonb pushed a commit that referenced this pull request Mar 22, 2022
@AnthonyLaw @Wayonb
[explorer] fix: improvement blocks listing page (#1013)
1b3966e 
* fix: remove chain info on top right blocks listing page

* feat: add inflation data

* fix: refactor code

* fix: re-arrage block listing table head

* feat: new translation

* Fix: get inflation from statement receipt

* task: fix indent

* task: removed unused code

* task: update get block list unit test

* task: add missing library

* task: lint fix import order
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jaguar0625 Jaguar0625 Jaguar0625 approved these changes

@OlegMakarenko OlegMakarenko Awaiting requested review from OlegMakarenko

@0x6861746366574 0x6861746366574 Awaiting requested review from 0x6861746366574

@gimre-xymcity gimre-xymcity Awaiting requested review from gimre-xymcity

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Improvement for blocks listing page
4 participants
@AnthonyLaw @OlegMakarenko @Wayonb @Jaguar0625