Add mobile history pull to refresh - #758
Conversation
|
@Emelie-Dev Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughWalkthroughThis PR adds pull-to-refresh gesture support to the mobile transaction history screen with network-aware state management. A new ChangesPull-to-refresh with offline awareness
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/mobile-wallet/src/components/PullToRefreshControl.tsx`:
- Around line 49-53: The indicator text is misleading because
shouldShowIndicator turns true for any pullDistance > 0; update the conditional
rendering inside PullToRefreshControl (where shouldShowIndicator and
isRefreshing are used) to branch the copy based on whether pullDistance has
reached the refresh threshold (e.g., replace the current 'Release to refresh'
with a conditional that shows 'Pull to refresh' when pullDistance <
refreshThreshold and 'Release to refresh' when pullDistance >=
refreshThreshold), keeping the isRefreshing case unchanged so it still shows
'Refreshing transactions...' when isRefreshing is true and preserving the
aria-live/role attributes.
- Around line 18-40: Guard pull-to-refresh by requiring the scroll container be
at the top and correct the indicator copy logic: in
handleTouchStart/handleTouchMove record/validate only when the target scroll
container's scrollTop === 0 (or a provided scrollContainerRef's
current.scrollTop === 0) so downward swipes mid-scroll don't start a pull; in
handleTouchEnd only call onRefresh when pullDistance >= PULL_THRESHOLD_PX and
the container was at top and !isRefreshing; also change the indicator text logic
to show "Pull to refresh" when pullDistance > 0 but < PULL_THRESHOLD_PX,
"Release to refresh" when pullDistance >= PULL_THRESHOLD_PX and not
isRefreshing, and the existing refreshing text when isRefreshing is true,
updating any component that reads pullDistance/isRefreshing to use these
thresholds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d6446ef9-d37d-4836-8aa4-8b7069b410f1
📒 Files selected for processing (6)
apps/mobile-wallet/src/components/PullToRefreshControl.tsxapps/mobile-wallet/src/components/TransactionHistoryList.tsxapps/mobile-wallet/src/screens/history/HistoryScreen.tsxapps/mobile-wallet/src/screens/history/__tests__/TransactionHistoryList.test.tsxapps/mobile-wallet/src/screens/history/usePaginatedTransactionHistory.test.tsxapps/mobile-wallet/src/screens/history/usePaginatedTransactionHistory.ts
| const handleTouchStart = (event: TouchEvent<HTMLElement>) => { | ||
| startYRef.current = event.touches[0]?.clientY ?? null; | ||
| }; | ||
|
|
||
| const handleTouchMove = (event: TouchEvent<HTMLElement>) => { | ||
| if (startYRef.current === null || isRefreshing) { | ||
| return; | ||
| } | ||
|
|
||
| const nextY = event.touches[0]?.clientY ?? startYRef.current; | ||
| setPullDistance(Math.max(0, Math.min(nextY - startYRef.current, PULL_THRESHOLD_PX))); | ||
| }; | ||
|
|
||
| const handleTouchEnd = () => { | ||
| const shouldRefresh = pullDistance >= PULL_THRESHOLD_PX; | ||
|
|
||
| startYRef.current = null; | ||
| setPullDistance(0); | ||
|
|
||
| if (shouldRefresh && !isRefreshing) { | ||
| void onRefresh(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect how the list/section is laid out and whether an ancestor is the scroll container.
rg -nP -C4 'overflow(-y)?\s*[:=]|overflow-(y-)?(auto|scroll)|PullToRefreshControl' --type=ts --type=tsx
fd -e tsx -e ts | rg -i 'history|scroll|layout'Repository: ancore-org/ancore
Length of output: 1168
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Read the full PullToRefreshControl component to verify touch/indicator logic.
FILE="apps/mobile-wallet/src/components/PullToRefreshControl.tsx"
if [ -f "$FILE" ]; then
wc -l "$FILE"
sed -n '1,220p' "$FILE" | cat -n
fi
# 2) Find all usages of PullToRefreshControl to identify the actual scroll container.
rg -n "PullToRefreshControl" --hidden -S .
# 3) Search history/transaction list components for scroll container refs/handlers.
rg -n "onScroll|scrollTop|ref=.*scroll|overflow(-y)?|Section|<section|TransactionHistory|HistoryScreen" apps/mobile-wallet/src/screens/history apps/mobile-wallet/src/components -SRepository: ancore-org/ancore
Length of output: 10589
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect how PullToRefreshControl wraps the history list and what the scroll container is.
FILE1="apps/mobile-wallet/src/components/TransactionHistoryList.tsx"
FILE2="apps/mobile-wallet/src/screens/history/HistoryScreen.tsx"
for f in "$FILE1" "$FILE2"; do
echo "==== $f ===="
wc -l "$f"
sed -n '1,260p' "$f" | cat -n
done
# 2) Search for scroll/overflow indicators specifically in those files/folders.
rg -n --hidden "scrollTop|onScroll|overflow(-y)?\s*:|overflow(-y)?\s*=" apps/mobile-wallet/src/components/TransactionHistoryList.tsx apps/mobile-wallet/src/screens/history/HistoryScreen.tsx
rg -n --hidden "overflow(-y)?\s*[:=]|onScroll|scrollTop" apps/mobile-wallet/src/screens/history apps/mobile-wallet/src/components -SRepository: ancore-org/ancore
Length of output: 6181
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Find where HistoryScreen is mounted.
rg -n --hidden "HistoryScreen" apps/mobile-wallet/src -S
# 2) Find scroll containers in mobile-wallet (tailwind overflow-y-* or CSS overflow-y).
rg -n --hidden "(overflow-y-auto|overflow-y-scroll|overflow-y)\-" apps/mobile-wallet/src -S
rg -n --hidden "overflow(-y)?\s*[:=]" apps/mobile-wallet/src -S
rg -n --hidden "onScroll=" apps/mobile-wallet/src -S
# 3) If HistoryScreen is used inside some layout/container, inspect that file(s).
# (We’ll print only the nearby lines around matches.)
rg -n --hidden "HistoryScreen" apps/mobile-wallet/src -S | head -n 50Repository: ancore-org/ancore
Length of output: 316
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find where HistoryScreen is instantiated in the app.
rg -n --hidden "<HistoryScreen|HistoryScreen\s*=" apps/mobile-wallet/src -S
# Search for the actual scrolling container/styling and touch gesture handling.
rg -n --hidden "overflow(-y)?(-auto|-scroll|-hidden|-visible)?|scrollTop|onScroll=|touch-action|pan-y|overscroll" apps/mobile-wallet/src -S
# If nothing turns up, expand to the whole repo (still limited to mobile-wallet).
rg -n --hidden "overflow(-y)?(-auto|-scroll|-hidden|-visible)?|onScroll=|touch-action|pan-y|overscroll" apps/mobile-wallet/src -SRepository: ancore-org/ancore
Length of output: 217
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Find where HistoryScreen/TransactionHistoryList are used/mounted anywhere in the repo.
rg -n --hidden "<\s*HistoryScreen\b|HistoryScreen\s*=" . -S
rg -n --hidden "<\s*TransactionHistoryList\b|TransactionHistoryList\s*=" . -S
# 2) Search for likely scroll-container / overflow handling and touch-gesture modifiers.
rg -n --hidden "overflow(-y)?-(auto|scroll|hidden|visible)|overscroll|scrollTop|onScroll=|touch-action|pan-y|ScrollView" . -S
# 3) Narrow to mobile-wallet layout/entrypoints if present.
rg -n --hidden "mobile-wallet" . -S | head -n 80Repository: ancore-org/ancore
Length of output: 10262
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find how the history list is actually rendered/scrolls in the mobile-wallet app (RN primitives).
rg -n --hidden "ScrollView|FlatList|SectionList|VirtualizedList|onScroll|scrollEnabled|contentOffset" apps/mobile-wallet/src -S
# Find where TransactionHistoryList/HistoryScreen are used in mobile-wallet.
rg -n --hidden "TransactionHistoryList" apps/mobile-wallet/src -S
rg -n --hidden "HistoryScreen" apps/mobile-wallet/src -S
# Find any styling that indicates a scroll container in mobile-wallet (RN uses style objects; web uses className/overflow).
rg -n --hidden "overflow(-y)?|touch-action|pan-y|overscroll|scrollTop" apps/mobile-wallet/src -SRepository: ancore-org/ancore
Length of output: 43
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for any CSS overflow/scroll class usage in mobile-wallet.
rg -n --hidden "overflow(-y)?-?(auto|scroll|hidden|visible)|overscroll|scrollbar|touch-action|pan-y" apps/mobile-wallet/src -S
# Find mobile-wallet entry/root layout to locate scroll container.
rg -n --hidden "createRoot|ReactDOM\.createRoot|router|Routes|route|BrowserRouter|MemoryRouter|Scroll" apps/mobile-wallet/src -S
# List top-level files to guide where to inspect.
ls -R apps/mobile-wallet/src | head -n 120Repository: ancore-org/ancore
Length of output: 43
🏁 Script executed:
#!/bin/bash
set -uo pipefail
echo "=== mobile-wallet src tree (partial) ==="
ls -R apps/mobile-wallet/src | head -n 160
echo "=== searches (non-failing) ==="
rg -n --hidden "overflow(-y)?-?(auto|scroll|hidden|visible)|overscroll|touch-action|pan-y|scrollTop|scrollbar" apps/mobile-wallet/src -S || true
rg -n --hidden "ScrollView|FlatList|SectionList|VirtualizedList|onScroll|scrollEnabled|contentOffset" apps/mobile-wallet/src -S || true
rg -n --hidden "createRoot|ReactDOM\.createRoot|router|Routes|route|BrowserRouter|MemoryRouter|Scroll" apps/mobile-wallet/src -S || trueRepository: ancore-org/ancore
Length of output: 5240
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in \
apps/mobile-wallet/src/app/App.tsx \
apps/mobile-wallet/src/navigation/MobileWalletShell.tsx \
apps/mobile-wallet/src/navigation/index.ts \
apps/mobile-wallet/src/app/bootstrap.ts \
apps/mobile-wallet/src/index.ts
do
echo "==== $f ===="
wc -l "$f" || true
sed -n '1,220p' "$f" | cat -n || true
doneRepository: ancore-org/ancore
Length of output: 4610
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --hidden "pull-to-refresh|Release to refresh|Refreshing transactions|onRefresh\\(" apps/mobile-wallet/src -S || true
rg -n --hidden "pullDistance|PULL_THRESHOLD_PX" . -S || true
# Inspect TransactionHistoryList tests (may include indicator / refresh expectations)
sed -n '1,260p' apps/mobile-wallet/src/screens/history/__tests__/TransactionHistoryList.test.tsx | cat -nRepository: ancore-org/ancore
Length of output: 11208
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/mobile-wallet/src/screens/history/__tests__/TransactionHistoryList.test.tsx"
sed -n '330,470p' "$FILE" | cat -nRepository: ancore-org/ancore
Length of output: 4023
Add “scroll-at-top” gating to prevent unintended refresh, and fix the indicator copy
apps/mobile-wallet/src/components/PullToRefreshControl.tsxtouch handlers derivepullDistancefrom finger delta only and callonRefreshoncepullDistance >= 48—there’s no check that the relevant scroll container is atscrollTop === 0, so a downward swipe can initiate refresh while the list is mid-scroll.- The indicator text shows “Release to refresh” for any
pullDistance > 0(it only switches whenisRefreshingchanges), even when the pull is still below the threshold—this copy is misleading.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/mobile-wallet/src/components/PullToRefreshControl.tsx` around lines 18 -
40, Guard pull-to-refresh by requiring the scroll container be at the top and
correct the indicator copy logic: in handleTouchStart/handleTouchMove
record/validate only when the target scroll container's scrollTop === 0 (or a
provided scrollContainerRef's current.scrollTop === 0) so downward swipes
mid-scroll don't start a pull; in handleTouchEnd only call onRefresh when
pullDistance >= PULL_THRESHOLD_PX and the container was at top and
!isRefreshing; also change the indicator text logic to show "Pull to refresh"
when pullDistance > 0 but < PULL_THRESHOLD_PX, "Release to refresh" when
pullDistance >= PULL_THRESHOLD_PX and not isRefreshing, and the existing
refreshing text when isRefreshing is true, updating any component that reads
pullDistance/isRefreshing to use these thresholds.
| {shouldShowIndicator ? ( | ||
| <p aria-live="polite" role="status"> | ||
| {isRefreshing ? 'Refreshing transactions...' : 'Release to refresh'} | ||
| </p> | ||
| ) : null} |
There was a problem hiding this comment.
"Release to refresh" is misleading below the threshold.
shouldShowIndicator becomes true as soon as pullDistance > 0, so the message reads "Release to refresh" even when the user hasn't pulled far enough — releasing then does nothing. Branch the copy on whether the threshold is met.
💡 Proposed change
const isPulling = pullDistance > 0 && !isRefreshing;
const shouldShowIndicator = isPulling || isRefreshing;
+ const hasReachedThreshold = pullDistance >= PULL_THRESHOLD_PX; {shouldShowIndicator ? (
<p aria-live="polite" role="status">
- {isRefreshing ? 'Refreshing transactions...' : 'Release to refresh'}
+ {isRefreshing
+ ? 'Refreshing transactions...'
+ : hasReachedThreshold
+ ? 'Release to refresh'
+ : 'Pull to refresh'}
</p>
) : null}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/mobile-wallet/src/components/PullToRefreshControl.tsx` around lines 49 -
53, The indicator text is misleading because shouldShowIndicator turns true for
any pullDistance > 0; update the conditional rendering inside
PullToRefreshControl (where shouldShowIndicator and isRefreshing are used) to
branch the copy based on whether pullDistance has reached the refresh threshold
(e.g., replace the current 'Release to refresh' with a conditional that shows
'Pull to refresh' when pullDistance < refreshThreshold and 'Release to refresh'
when pullDistance >= refreshThreshold), keeping the isRefreshing case unchanged
so it still shows 'Refreshing transactions...' when isRefreshing is true and
preserving the aria-live/role attributes.
Description
Type of Change
Security Impact
Testing
Test Coverage
Manual Testing Steps
Breaking Changes
Checklist
For High-Security Changes
Related Issues
Closes #614
Related to #
Additional Context
Reviewer Notes
Summary by CodeRabbit