-
Notifications
You must be signed in to change notification settings - Fork 24
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
New tab for show internal transactions history for a given address #346
New tab for show internal transactions history for a given address #346
Conversation
✅ Deploy Preview for teloscan-stage ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for dev-mainnet-teloscan ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Deploying with Cloudflare Pages
|
1fdc14b
to
1ea123b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks really good overall, nice work! i think this seems to accomplish the goal of the ticket. lets see if @tomtomcrypto agrees as he made the original ticket.
the only issue i found is that if you go a few pages in the table, then navigate to somewhere else like the Health page, if you try to navigate back in history it goes to a URL like https://deploy-preview-346--dev-mainnet-teloscan.netlify.app/?page=2&pagesize=10
(which is the home page with pagination). I think the issue might be at line 138 in the new component because it uses pushState
instead of replaceState
.
@EJSG, thanks for that review! That bug you found was the last drop I needed to push myself to find a better solution. Fortunately, I did. I changed the strategy from a homemade solution using history.pushState to a more easy and elegant solution using $router (which is intended for these cases). async onPaginationChange(props) {
const { page, rowsPerPage } = props.pagination;
this.$router.push({
hash: window.location.hash,
query: {
...this.$route.query,
page: `${page},${rowsPerPage}`,
},
});
}, watch: {
'$route.query.page': {
handler(_pag) {
let pag = _pag;
let page = 1;
let size = this.pagination.rowsPerPage;
// we also allow to pass a single number as the page number
if (typeof pag === 'number') {
page = pag;
} else if (typeof pag === 'string') {
// we also allow to pass a string of two numbers: [page, rowsPerPage]
const [p, s] = pag.split(',');
page = p;
size = s;
}
this.setPagination(page, size);
},
immediate: true,
},
}, I'm going to create a new issue to implement this same solution on the TransactionTable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry haha, found one issue, see my comment here
#351 (review)
7f1470b
to
8a4b877
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏
Fixes #226
Description
This PR adds a new tab on the account page to show the same transaction pagination but focused on showing the internal transactions for each parent transaction.
As default behavior, the internal transactions data is visible for all transactions but the user can close and reopen the section for each transaction individually using a switcher on the right of the transaction header.
Test scenarios
Screenshots
Checklist: