-
Notifications
You must be signed in to change notification settings - Fork 24
Add pull-to-refresh with haptics to mobile lists #235
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
Add pull-to-refresh with haptics to mobile lists #235
Conversation
- Added `expo-haptics` dependency to mobile app. - Implemented pull-to-refresh on `HomeScreen` (groups list). - Implemented pull-to-refresh on `GroupDetailsScreen` (expenses list). - Separated `isLoading` (initial) and `isRefreshing` (update) states to prevent UI flickering. - Added light haptic feedback on refresh trigger. - Updated tracking files (`todo.md`, `knowledge.md`, `changelog.md`).
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the WalkthroughAdds pull-to-refresh with haptic feedback to mobile screens, adds the Changes
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
✅ Deploy Preview for split-but-wiser canceled.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @.Jules/knowledge.md:
- Around line 185-214: Add blank lines before and after each fenced code block
in the "Bad Pattern" and "Good Pattern" examples to satisfy MD031; specifically
ensure there is an empty line immediately preceding each ```javascript fence and
an empty line immediately following each closing ``` fence so the
FlatList/Spinner example and the fetchData example render correctly and have
proper spacing around the fenced code blocks.
In @.Jules/todo.md:
- Around line 46-49: Update the completion date for the Pull-to-refresh task in
.Jules/todo.md from "Completed: 2026-01-01" to "Completed: 2026-01-11" so it
matches the PR (commit 91c5194); locate the checklist item text "- [x] **[ux]**
Pull-to-refresh with haptic feedback on all list screens" and change the date on
its "Completed:" line accordingly.
- Around line 46-49: The comment notes FriendsScreen.js lacks pull-to-refresh
with haptic feedback; either implement it or update the task note. To implement:
in FriendsScreen.js add a refreshing state (useState), pass refreshing and
onRefresh props to the FlatList, implement an onRefresh handler that triggers
data reload and calls Haptics (or the existing haptics helper) when refresh
begins/ends, and ensure you reuse the same data-fetch function used by
HomeScreen/GroupDetailsScreen; alternatively, if you choose not to implement,
update the TODO entry text to remove "all list screens" or list only
HomeScreen.js and GroupDetailsScreen.js so documentation matches actual
implementation.
In @mobile/package.json:
- Line 19: The dependency "expo-haptics": "~15.0.8" in package.json is
incompatible with Expo SDK 54; replace it with the SDK-54-compatible version by
running the recommended command (npx expo install expo-haptics) or update the
"expo-haptics" entry in package.json to the version that npx expo install
selects, then reinstall dependencies and rebuild the native app to ensure no
runtime/native mismatch.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
mobile/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
.Jules/changelog.md.Jules/knowledge.md.Jules/todo.mdmobile/package.jsonmobile/screens/GroupDetailsScreen.jsmobile/screens/HomeScreen.js
🧰 Additional context used
📓 Path-based instructions (1)
mobile/screens/**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Mobile screens should be created as components in
mobile/screens/and registered in the navigation structure
Files:
mobile/screens/GroupDetailsScreen.jsmobile/screens/HomeScreen.js
🧬 Code graph analysis (2)
mobile/screens/GroupDetailsScreen.js (1)
mobile/screens/HomeScreen.js (2)
isRefreshing(23-23)onRefresh(113-115)
mobile/screens/HomeScreen.js (4)
mobile/screens/GroupDetailsScreen.js (2)
isRefreshing(26-26)onRefresh(62-64)mobile/api/client.js (1)
isRefreshing(7-7)mobile/api/auth.js (2)
refresh(13-15)refresh(13-15)mobile/context/AuthContext.js (1)
refresh(15-15)
🪛 markdownlint-cli2 (0.18.1)
.Jules/knowledge.md
186-186: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
195-195: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🔇 Additional comments (5)
.Jules/changelog.md (1)
13-13: LGTM!The changelog entry accurately documents the new pull-to-refresh feature with haptic feedback for both screens.
mobile/screens/GroupDetailsScreen.js (2)
34-60: LGTM! Clean separation of loading states.The implementation correctly separates initial loading (
isLoading) from pull-to-refresh (isRefreshing). The fire-and-forget pattern forHaptics.impactAsyncis acceptable since haptic feedback is non-critical and shouldn't block data fetching. Thefinallyblock properly resets both states.
62-64: LGTM!The
onRefreshhandler and FlatList integration follow the documented pattern correctly. The refresh indicator will display during the refresh operation without hiding the existing list content.Also applies to: 218-219
mobile/screens/HomeScreen.js (2)
71-105: LGTM! Well-structured refresh logic.The
fetchGroupsfunction correctly handles both initial load and refresh scenarios. The settlement status fetching is appropriately included in both paths, ensuring data consistency after refresh. The implementation is consistent withGroupDetailsScreen.js.
113-115: LGTM!The refresh integration follows the documented best practice. Since the
FlatListonly renders when!isLoading, the pull-to-refresh indicator will work correctly without causing the list to disappear during refresh operations.Also applies to: 262-263
| "@react-navigation/native-stack": "^7.3.23", | ||
| "axios": "^1.11.0", | ||
| "expo": "^54.0.25", | ||
| "expo-haptics": "~15.0.8", |
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.
🧩 Analysis chain
🌐 Web query:
expo-haptics 15.0.8 compatibility Expo SDK 54
💡 Result:
Short answer: Probably not — use npx expo install to get the SDK-compatible build.
- Expo SDK 54 expects the set of Expo package versions published for SDK 54; Expo’s docs recommend installing Expo SDK packages via npx expo install so you get the version that matches your SDK. [1][2]
- expo-haptics has v15.x releases (e.g. 15.0.8 is published on registries), which are part of the newer 15.x package line — those releases typically target newer SDKs. Using a 15.x package with SDK 54 may cause native/runtime mismatches. [3][4]
- Recommendation: run npx expo install expo-haptics in a project that has expo@~54.x. That command will install the expo-haptics version that is compatible with SDK 54. If you already manually set expo-haptics to 15.0.8 and see build/runtime errors, change it to the version npx expo install picks or downgrade to the SDK-54-compatible version. [1][2]
Sources:
- Expo SDK 54 docs / upgrade & package guidance. [docs.expo.dev versions v54]. [1]
- Expo workflow: “npx expo install” picks compatible package versions. [docs.expo.dev]. [2]
- expo-haptics package listing showing 15.0.8 exists. [jsdelivr / npm listings]. [3]
- Release history showing expo-haptics 15.x releases. [release listing]. [4]
expo-haptics 15.0.8 is incompatible with Expo SDK 54.
This version targets newer SDKs and will cause native/runtime mismatches. Use npx expo install expo-haptics to install the SDK-54-compatible version.
🤖 Prompt for AI Agents
In @mobile/package.json at line 19, The dependency "expo-haptics": "~15.0.8" in
package.json is incompatible with Expo SDK 54; replace it with the
SDK-54-compatible version by running the recommended command (npx expo install
expo-haptics) or update the "expo-haptics" entry in package.json to the version
that npx expo install selects, then reinstall dependencies and rebuild the
native app to ensure no runtime/native mismatch.
|
@jules In @.Jules/knowledge.md: - Around line 185-214: Add blank lines before and after each fenced code block in the "Bad Pattern" and "Good Pattern" examples to satisfy MD031; specifically ensure there is an empty line immediately preceding each |
Thank you for the detailed review. I have addressed all points:
|
- Fixed `expo-haptics` version incompatibility in `mobile/package.json`. - Implemented pull-to-refresh on `FriendsScreen.js` to complete "all list screens" task. - Fixed markdown formatting in `.Jules/knowledge.md`. - Updated completion date and file list in `.Jules/todo.md`.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mobile/screens/FriendsScreen.js (1)
25-64: Indentation inconsistency in fetchData.Lines 37-54 have inconsistent indentation compared to the surrounding code (appears to have extra indentation). This should be aligned with the rest of the function body for consistency.
🔧 Suggested fix
const fetchData = async (refresh = false) => { if (refresh) { setIsRefreshing(true); Haptics.impactAsync(Haptics.ImpactFeedbackStyle.Light); } else { setIsLoading(true); } try { // Fetch friends balance + groups concurrently for group icons const friendsResponse = await getFriendsBalance(); const friendsData = friendsResponse.data.friendsBalance || []; - const groupsResponse = await getGroups(); - const groups = groupsResponse?.data?.groups || []; - const groupMeta = new Map( - groups.map((g) => [g._id, { name: g.name, imageUrl: g.imageUrl }]) - ); - - const transformedFriends = friendsData.map((friend) => ({ - id: friend.userId, - name: friend.userName, - imageUrl: friend.userImageUrl || null, - netBalance: friend.netBalance, - groups: (friend.breakdown || []).map((group) => ({ - id: group.groupId, - name: group.groupName, - balance: group.balance, - imageUrl: groupMeta.get(group.groupId)?.imageUrl || null, - })), - })); + const groupsResponse = await getGroups(); + const groups = groupsResponse?.data?.groups || []; + const groupMeta = new Map( + groups.map((g) => [g._id, { name: g.name, imageUrl: g.imageUrl }]) + ); + + const transformedFriends = friendsData.map((friend) => ({ + id: friend.userId, + name: friend.userName, + imageUrl: friend.userImageUrl || null, + netBalance: friend.netBalance, + groups: (friend.breakdown || []).map((group) => ({ + id: group.groupId, + name: group.groupName, + balance: group.balance, + imageUrl: groupMeta.get(group.groupId)?.imageUrl || null, + })), + })); setFriends(transformedFriends); } catch (error) {
🤖 Fix all issues with AI agents
In @.Jules/todo.md:
- Around line 154-157: The "Pull-to-refresh with haptic feedback on all list
screens" entry in the Completed Tasks section has inconsistent metadata: update
its completion date to 2026-01-11 to match the Mobile section and add the
missing file `FriendsScreen.js` to the "Files modified" list so it reads the
same as the Mobile section (include `HomeScreen.js`, `GroupDetailsScreen.js`,
`FriendsScreen.js`); ensure the task title and checklist mark remain unchanged.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
mobile/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
.Jules/knowledge.md.Jules/todo.mdmobile/package.jsonmobile/screens/FriendsScreen.js
🧰 Additional context used
📓 Path-based instructions (1)
mobile/screens/**/*.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Mobile screens should be created as components in
mobile/screens/and registered in the navigation structure
Files:
mobile/screens/FriendsScreen.js
🧬 Code graph analysis (1)
mobile/screens/FriendsScreen.js (4)
mobile/screens/GroupDetailsScreen.js (3)
isRefreshing(26-26)fetchData(34-60)onRefresh(62-64)mobile/screens/HomeScreen.js (2)
isRefreshing(23-23)onRefresh(113-115)web/services/api.ts (1)
getFriendsBalance(49-49)mobile/api/groups.js (2)
getFriendsBalance(29-30)getFriendsBalance(29-30)
🔇 Additional comments (3)
.Jules/knowledge.md (1)
178-219: Good documentation of the pull-to-refresh pattern.The separation of
isLoadingvsisRefreshingstates is well documented with clear bad/good pattern examples. The recommendation to useexpo-hapticsaligns with the implementation.Minor: The date at line 180 shows "2026-01-01" but this content was added in the current PR (2026-01-11). Consider updating for accuracy.
mobile/screens/FriendsScreen.js (2)
66-68: LGTM!The
onRefreshhandler correctly delegates tofetchData(true), consistent with the pattern used inHomeScreen.jsandGroupDetailsScreen.js.
251-252: LGTM!The FlatList is correctly wired with
onRefreshandrefreshing={isRefreshing}, following the documented "Good Pattern" and matching the implementation in other screens. As per coding guidelines, this screen component is properly located inmobile/screens/.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@jules the preview is failing here's the traceback |
I have updated the GitHub Actions workflows to use Node.js 22. This should resolve the |
- Updated `.github/workflows/preview.yml`, `rn-bundle-analysis.yml`, `bundle-analysis.yml`, and `publish.yml` to use Node.js 22. - This fixes build failures caused by `expo-updates` and other dependencies requiring Node 20+.
Devasy
left a comment
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.
LGTM
Implemented pull-to-refresh functionality with haptic feedback on the mobile app's main list screens (
HomeScreenandGroupDetailsScreen). This improves the user experience by allowing users to easily refresh data without losing context, and provides tactile feedback for the action. I also separated the initial loading state from the refreshing state to prevent the list from disappearing during a refresh.PR created automatically by Jules for task 12130360773097016983 started by @Devasy23
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.