[jules] style: Add haptic feedback to all mobile buttons #276
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: Devasy23 <110348311+Devasy23@users.noreply.github.com>
Contributor
|
👋 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. |
✅ Deploy Preview for split-but-wiser canceled.
|
Contributor
WalkthroughAdds a mobile haptics system: Haptic wrappers for react-native-paper components, a haptic utilities module, integration into mobile screens (pull-to-refresh haptics, separate isRefreshing state), and updates to .Jules docs marking the haptics TODO completed. No public API signature breaks. Changes
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. No actionable comments were generated in the recent review. 🎉 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
mobile/screens/FriendsScreen.js (1)
98-166:⚠️ Potential issue | 🔴 CriticalBuild-breaking JSX tag mismatch:
<HapticListAccordion>closed with</List.Accordion>.Line 98 opens
<HapticListAccordion>but Line 166 closes with</List.Accordion>. This causes the pipeline SyntaxError.🐛 Fix the closing tag
- </List.Accordion> + </HapticListAccordion>mobile/screens/AddExpenseScreen.js (1)
373-397: 🧹 Nitpick | 🔵 TrivialNote:
HapticMenuadds no haptic feedback itself.Per the snippet,
HapticMenuis a pure passthrough (<Menu {...props}>{children}</Menu>). Haptic feedback on menu open comes only from theHapticButtonanchor. The individualHapticMenu.Itempresses won't trigger haptics unless that wrapper also includes it — worth confirming this matches the intended behavior across the app.
🤖 Fix all issues with AI agents
In @.Jules/changelog.md:
- Line 12: The changelog line omitted HapticAppbar and its subcomponents; update
the list to include HapticAppbar and its subcomponents (specifically
HapticAppbarBackAction and HapticAppbarAction) so the entry names all wrapper
components used across screens are documented alongside HapticButton,
HapticIconButton, HapticFAB, HapticCard, HapticList, HapticCheckbox, HapticMenu,
and HapticSegmentedButtons.
In `@mobile/components/ui/HapticAppbar.js`:
- Around line 1-21: Multiple Haptic* wrapper files (e.g., HapticAppbarAction and
HapticAppbarBackAction) duplicate the same handlePress logic; extract a shared
utility (e.g., create mobile/components/ui/hapticUtils.js exporting
withHapticPress and createHapticWrapper) that calls Haptics.impactAsync and then
delegates to the original onPress, and refactor HapticAppbarAction and
HapticAppbarBackAction to use createHapticWrapper(WrappedComponent) (or pass
onPress through withHapticPress) so the components become thin wrappers that
reuse the centralized haptic behavior and set a clear displayName.
In `@mobile/components/ui/HapticButton.js`:
- Around line 1-14: Create a shared HOC named withHapticFeedback that wraps a
component's press-like handler (default prop name 'onPress') to call
Haptics.impactAsync before delegating to the original handler; implement it
using React.forwardRef and useCallback and accept options { pressProp='onPress',
onlyWhenHandler=false } to support components with different handler prop names
or only-fire-if-handler semantics. Replace each individual wrappers
(HapticButton, HapticIconButton, HapticFAB, HapticCheckbox, HapticMenu,
HapticCard, HapticList, HapticAppbar, HapticSegmentedButtons) to simply export
withHapticFeedback(WrappedComponent, options) — e.g., for SegmentedButtons pass
pressProp:'onValueChange' and for components that should only haptic when a
handler exists pass onlyWhenHandler:true — and ensure the HOC sets a descriptive
displayName for debugging.
In `@mobile/components/ui/HapticList.js`:
- Around line 5-22: Haptic feedback is currently inconsistent: HapticListItem
only triggers haptics when onPress exists but HapticListAccordion always fires;
change HapticListAccordion to mirror the guarded pattern used in HapticListItem
(i.e., only call Haptics.impactAsync when onPress is defined) and ensure other
components (HapticCard, HapticButton, HapticIconButton, HapticFAB) follow the
same convention so non-interactive items don't produce haptics; locate and
update the onPress handlers in HapticListAccordion (and the listed Haptic*
components) to check if onPress is truthy before invoking Haptics and passing an
onPress handler to the underlying List.Accordion/List.Item.
In `@mobile/screens/FriendsScreen.js`:
- Around line 12-14: The file imports expo-haptics directly but only uses it in
onRefresh while other haptics go through wrappers (HapticIconButton,
HapticListAccordion); consolidate by moving the pull-to-refresh haptic into your
existing haptic utilities: add a named helper (e.g., triggerPullRefreshHaptic)
exported from the same module that provides
HapticIconButton/HapticListAccordion, replace the direct Haptics call in
onRefresh with that helper, and then remove the direct import of expo-haptics
from this file so all haptics are centralized through the wrapper utilities.
mobile/components/ui/HapticButton.js
Outdated
Comment on lines
1
to
14
| import React from 'react'; | ||
| import { Button } from 'react-native-paper'; | ||
| import * as Haptics from 'expo-haptics'; | ||
|
|
||
| const HapticButton = ({ onPress, ...props }) => { | ||
| const handlePress = (e) => { | ||
| Haptics.impactAsync(Haptics.ImpactFeedbackStyle.Light); | ||
| if (onPress) onPress(e); | ||
| }; | ||
|
|
||
| return <Button onPress={handlePress} {...props} />; | ||
| }; | ||
|
|
||
| export default HapticButton; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract a shared withHapticFeedback HOC to eliminate duplication across ~10 nearly identical wrapper files.
Every Haptic* wrapper (HapticButton, HapticIconButton, HapticFAB, HapticCheckbox, HapticMenu, HapticCard, HapticList, HapticAppbar, HapticSegmentedButtons) repeats the same 3-line handlePress pattern. This is a textbook DRY violation — a single higher-order component or utility would centralize the logic and make it trivial to adjust feedback style, add platform checks, or respect user accessibility settings (e.g., "Reduce Motion") in one place.
♻️ Proposed: shared utility + simplified wrappers
Create mobile/components/ui/withHapticFeedback.js:
import React, { useCallback } from 'react';
import * as Haptics from 'expo-haptics';
/**
* HOC that wraps a component's `onPress` (or a custom handler prop) with haptic feedback.
* `@param` {React.ComponentType} WrappedComponent
* `@param` {object} [options]
* `@param` {string} [options.pressProp='onPress'] - the prop name to intercept
* `@param` {boolean} [options.onlyWhenHandler=false] - skip haptic if no handler provided
*/
export default function withHapticFeedback(
WrappedComponent,
{ pressProp = 'onPress', onlyWhenHandler = false } = {}
) {
const HapticWrapper = React.forwardRef(({ [pressProp]: handler, ...props }, ref) => {
const handleInteraction = useCallback(
(...args) => {
if (!onlyWhenHandler || handler) {
Haptics.impactAsync(Haptics.ImpactFeedbackStyle.Light);
}
handler?.(...args);
},
[handler]
);
return <WrappedComponent ref={ref} {...props} {...{ [pressProp]: handleInteraction }} />;
});
HapticWrapper.displayName = `Haptic(${WrappedComponent.displayName || WrappedComponent.name || 'Component'})`;
return HapticWrapper;
}Then each wrapper becomes a one-liner, e.g. HapticButton.js:
import { Button } from 'react-native-paper';
import withHapticFeedback from './withHapticFeedback';
export default withHapticFeedback(Button);And for components like Card/List that should only fire when a handler exists:
import { Card } from 'react-native-paper';
import withHapticFeedback from './withHapticFeedback';
export default withHapticFeedback(Card, { onlyWhenHandler: true });For SegmentedButtons with a different prop name:
import { SegmentedButtons } from 'react-native-paper';
import withHapticFeedback from './withHapticFeedback';
export default withHapticFeedback(SegmentedButtons, { pressProp: 'onValueChange' });🤖 Prompt for AI Agents
In `@mobile/components/ui/HapticButton.js` around lines 1 - 14, Create a shared
HOC named withHapticFeedback that wraps a component's press-like handler
(default prop name 'onPress') to call Haptics.impactAsync before delegating to
the original handler; implement it using React.forwardRef and useCallback and
accept options { pressProp='onPress', onlyWhenHandler=false } to support
components with different handler prop names or only-fire-if-handler semantics.
Replace each individual wrappers (HapticButton, HapticIconButton, HapticFAB,
HapticCheckbox, HapticMenu, HapticCard, HapticList, HapticAppbar,
HapticSegmentedButtons) to simply export withHapticFeedback(WrappedComponent,
options) — e.g., for SegmentedButtons pass pressProp:'onValueChange' and for
components that should only haptic when a handler exists pass
onlyWhenHandler:true — and ensure the HOC sets a descriptive displayName for
debugging.
Co-authored-by: Devasy23 <110348311+Devasy23@users.noreply.github.com>
Owner
Author
|
@jules In @.Jules/changelog.md:
In
In
In
In
|
Contributor
Thank you for the detailed feedback. I have implemented all the requested changes:
|
Co-authored-by: Devasy23 <110348311+Devasy23@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@mobile/components/ui/HapticMenu.js`:
- Around line 7-9: HapticMenu currently drops any passed ref because it's a
plain arrow component; update it to accept and forward refs and apply the
existing Haptic wrapper pattern: convert HapticMenu to use
React.forwardRef((props, ref) => <Menu ref={ref}
{...props}>{props.children}</Menu>) and then export the forwarded component
wrapped with withHapticFeedback (i.e., export default
withHapticFeedback(HapticMenuForwarded)) so refs passed to <HapticMenu> are
forwarded to Menu and haptic behavior is preserved.
In `@mobile/components/ui/hapticUtils.js`:
- Around line 54-56: The function triggerPullRefreshHaptic is marked async and
awaits Haptics.impactAsync, which is inconsistent with handlePress that fires
Haptics.impactAsync without awaiting; change triggerPullRefreshHaptic to be
fire-and-forget for consistency by removing the async/await (make it a plain
function that calls Haptics.impactAsync without awaiting) or, alternatively,
update handlePress to await Haptics.impactAsync — pick the fire-and-forget
approach and update the triggerPullRefreshHaptic implementation (referencing
triggerPullRefreshHaptic and handlePress) so both call patterns match.
In `@mobile/screens/FriendsScreen.js`:
- Around line 62-67: The onRefresh handler unnecessarily awaits
triggerPullRefreshHaptic before starting the network call; change it so the
haptic is fired without blocking fetchData (e.g., call
triggerPullRefreshHaptic() without await or start both concurrently with
Promise.all), keeping setIsRefreshing(true) at the top and awaiting
fetchData(false) to completion, and ensure any errors from
triggerPullRefreshHaptic are either ignored or handled separately; modify the
onRefresh function accordingly (references: onRefresh, triggerPullRefreshHaptic,
fetchData).
Comment on lines
+54
to
+56
| export const triggerPullRefreshHaptic = async () => { | ||
| await Haptics.impactAsync(Haptics.ImpactFeedbackStyle.Light); | ||
| }; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor inconsistency: triggerPullRefreshHaptic awaits the haptic call while handlePress does not.
In handlePress (Line 22), Haptics.impactAsync is fire-and-forget, which is fine for UI responsiveness. Here the call is awaited, meaning the caller's await triggerPullRefreshHaptic() will block on the haptic completing. This is unlikely to cause a real problem but is worth noting for consistency — if a caller doesn't await it, the async keyword is just unnecessary overhead.
🤖 Prompt for AI Agents
In `@mobile/components/ui/hapticUtils.js` around lines 54 - 56, The function
triggerPullRefreshHaptic is marked async and awaits Haptics.impactAsync, which
is inconsistent with handlePress that fires Haptics.impactAsync without
awaiting; change triggerPullRefreshHaptic to be fire-and-forget for consistency
by removing the async/await (make it a plain function that calls
Haptics.impactAsync without awaiting) or, alternatively, update handlePress to
await Haptics.impactAsync — pick the fire-and-forget approach and update the
triggerPullRefreshHaptic implementation (referencing triggerPullRefreshHaptic
and handlePress) so both call patterns match.
Comment on lines
62
to
67
| const onRefresh = async () => { | ||
| setIsRefreshing(true); | ||
| await Haptics.impactAsync(Haptics.ImpactFeedbackStyle.Light); | ||
| await triggerPullRefreshHaptic(); | ||
| await fetchData(false); | ||
| setIsRefreshing(false); | ||
| }; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
await on haptic before fetch is a minor latency concern.
triggerPullRefreshHaptic is awaited before fetchData starts. The haptic call is fast (~ms), but there's no reason to serialize these — the haptic and the network fetch are independent.
Suggested improvement
const onRefresh = async () => {
setIsRefreshing(true);
- await triggerPullRefreshHaptic();
- await fetchData(false);
+ triggerPullRefreshHaptic(); // fire-and-forget; no need to await
+ await fetchData(false);
setIsRefreshing(false);
};🤖 Prompt for AI Agents
In `@mobile/screens/FriendsScreen.js` around lines 62 - 67, The onRefresh handler
unnecessarily awaits triggerPullRefreshHaptic before starting the network call;
change it so the haptic is fired without blocking fetchData (e.g., call
triggerPullRefreshHaptic() without await or start both concurrently with
Promise.all), keeping setIsRefreshing(true) at the top and awaiting
fetchData(false) to completion, and ensure any errors from
triggerPullRefreshHaptic are either ignored or handled separately; modify the
onRefresh function accordingly (references: onRefresh, triggerPullRefreshHaptic,
fetchData).
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implemented a comprehensive haptic feedback system for the mobile app by creating wrapper components for React Native Paper UI elements. Replaced standard components with Haptic versions across all screens to provide consistent tactile feedback.
PR created automatically by Jules for task 14230723313919841411 started by @Devasy23
Summary by CodeRabbit
New Features
Accessibility
UX Improvements