-
Notifications
You must be signed in to change notification settings - Fork 97
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
chore(TransactionFeedV2): Add "No more transactions" toast #6153
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6153 +/- ##
========================================
Coverage 88.76% 88.77%
========================================
Files 728 728
Lines 30827 30835 +8
Branches 5639 5640 +1
========================================
+ Hits 27364 27373 +9
- Misses 3266 3420 +154
+ Partials 197 42 -155
... and 67 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
|
||
const isFirstPage = originalArgs?.endCursor === FIRST_PAGE_TIMESTAMP | ||
const moreThanMinumumTransactions = (data?.transactions || []).length > MIN_NUM_TRANSACTIONS | ||
const isFirstPageWithEnoughtTransactions = isFirstPage && moreThanMinumumTransactions |
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.
const isFirstPageWithEnoughtTransactions = isFirstPage && moreThanMinumumTransactions | |
const isFirstPageWithEnoughTransactions = isFirstPage && moreThanMinumumTransactions |
const isFirstPage = originalArgs?.endCursor === FIRST_PAGE_TIMESTAMP | ||
const moreThanMinumumTransactions = (data?.transactions || []).length > MIN_NUM_TRANSACTIONS | ||
const isFirstPageWithEnoughtTransactions = isFirstPage && moreThanMinumumTransactions | ||
const shouldShowToast = isFirstPageWithEnoughtTransactions || !isFirstPage |
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.
Hrm this logic feels a bit weird in the edge cases - imagine we have only 2 pages, both with 1 TX each. In that case, we won't hit the "minimum transactions", but we'll always show the toast since we're not on the first page. On the other hand, if we only have 1 page with 2 TXs, we will not show the toast, since we're on the first page with not enough transactions. It seems like the only thing we really need to check is whether or not there are enough transactions to reach the minimum, and we can safely ignore what page we're on? (Once the feed is populated with data, the concept of pagination is hidden from the user anyways, so why do we need to conditionally display the toast depending on which page # we're on?)
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.
@jophish You're right, I totally overthought this one!
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.
🎉
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.
🚀
### Description 7th PR for RET-1207. Add "No more transactions" toast when: - the first page is the only page and there's decent amount of transactions to trigger the scroll - next page returns an empty array This behaviour will be changed in the follow-up PR once connected to the new blockchain-api handler. ### Test plan Adds tests to check the new toast behaviour. ### Related issues - Relates to RET-1207 ### Backwards compatibility Yes ### Network scalability If a new NetworkId and/or Network are added in the future, the changes in this PR will: - [x] Continue to work without code changes, OR trigger a compilation error (guaranteeing we find it when a new network is added)
Description
7th PR for RET-1207. Add "No more transactions" toast when:
This behaviour will be changed in the follow-up PR once connected to the new blockchain-api handler.
Test plan
Adds tests to check the new toast behaviour.
Related issues
Backwards compatibility
Yes
Network scalability
If a new NetworkId and/or Network are added in the future, the changes in this PR will: