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

[DNM] Hero Graph remove old block for new block #101

Conversation

schmentle
Copy link
Contributor

@schmentle schmentle commented Jun 23, 2020

Description

When the explorer is opened, new blocks will keep rolling in and the graph becomes unreadable.

Screencast

https://share.getcloudapp.com/E0uzYYjj

Closes: #70

Copy link
Collaborator

@Krakaw Krakaw left a comment

Choose a reason for hiding this comment

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

Don't modify data in a render. Move this to the redux action. Add an env for MAX_BLOCKS and if latestBlocks > MAX_BLOCKS then splice off the last.

@schmentle
Copy link
Contributor Author

Don't modify data in a render. Move this to the redux action. Add an env for MAX_BLOCKS and if latestBlocks > MAX_BLOCKS then splice off the last.

Thanks @Krakaw. I've moved it to the API call.

@@ -13,6 +13,7 @@ const config = {
process.env.REACT_APP_EXPLORER_API_DOMAIN + '/ws'
),
tokenName: process.env.REACT_APP_TOKEN_NAME || 'tXTR',
initialBlockCount: +(process.env.REACT_APP_INITIAL_BLOCK_COUNT || 100)
initialBlockCount: +(process.env.REACT_APP_INITIAL_BLOCK_COUNT || 100),
maxBlocks: +(process.env.REACT_APP_MAX_BLOCKS || 100)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add the env to the .env.sample file

Comment on lines +29 to +31
if(blocks.blocks.length > config.maxBlocks) {
blocks.blocks.splice(-1,1);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move this to the redux action. Also how do we know that we only need to reduce by 1? We could've fetched 200 blocks.

@schmentle schmentle changed the title Hero Graph remove old block for new block [DNM] Hero Graph remove old block for new block Jun 24, 2020
@delta1
Copy link

delta1 commented Oct 18, 2020

@msbodetti any chance you could please take care of @Krakaw's feedback? Then I'll get this approved and merged - it's a definite improvement!

@schmentle
Copy link
Contributor Author

@msbodetti any chance you could please take care of @Krakaw's feedback? Then I'll get this approved and merged - it's a definite improvement!

Hey @delta1 went off my radar. Let me look at it today! I'll try get something up later today or this week!

@delta1
Copy link

delta1 commented Oct 18, 2020

Awesome thanks! There's no rush 🙏

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.

[QA] Keep active blocks set to a certain limit
4 participants