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: Implement switching to next/previous block on detail pages #395

Merged

Conversation

janmichek
Copy link
Collaborator

@janmichek janmichek commented Jul 24, 2023

Description

resolves #375
fixes #406

compared to the visual material, the design is old for consistency and will be improved here #148

Demo

firefox_pMVLrk9NPG.mp4

Checklist:

@github-actions
Copy link

Deployed to https://pr-395-aescan.stg.aepps.com

Copy link
Collaborator

@michele-franchi michele-franchi left a comment

Choose a reason for hiding this comment

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

It looks good but is there a way to disable the next arrow if the keyblock is the last one to avoid the "Requested keyblock has never been seen in the network." screen?

@lukeromanowicz
Copy link
Contributor

I think it should be doable by checking height against blockHeight in recentBlocksStore

@janmichek
Copy link
Collaborator Author

It looks good but is there a way to disable the next arrow if the keyblock is the last one to avoid the "Requested keyblock has never been seen in the network." screen?

Fixed and regenerated the demo

Copy link
Contributor

@lukeromanowicz lukeromanowicz left a comment

Choose a reason for hiding this comment

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

I expected that more recent keyblock button will be on the left side since every content we display is always latest first, and older stuff is available when you navigate right. But I guess this works too, just a little bit unexpected.

Overall very good job, I left only one not but I'm afraid it needs to be fixed before merging

'keyblock-details-panel__button',
'keyblock-details-panel__button--next',
]"
:disabled="!isNextKeyblockMined">
Copy link
Contributor

Choose a reason for hiding this comment

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

the button is still clickable, even if it's disabled :/

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch. You are right, haven't spotted this. Fixed

Copy link
Contributor

@lukeromanowicz lukeromanowicz left a comment

Choose a reason for hiding this comment

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

Now it works great, thank you for the fix!

Copy link
Collaborator

@michele-franchi michele-franchi left a comment

Choose a reason for hiding this comment

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

It works great!

Could you please open a followup to improve arrows handling and enable again the next one if a new keyblock comes from the websocket? Not super needed right now but this would improve it.

@janmichek janmichek merged commit 6bb0996 into develop Jul 26, 2023
3 checks passed
@janmichek janmichek deleted the Implement_switching_to_next/previous_block_on_detail_pages branch July 26, 2023 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants