💄 app: improve freeze card ui#995
Conversation
WalkthroughA patch release for the Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Card as Card Component
participant Sheet as CardFreezeSheet
participant API as Card API
participant CardStatus as CardStatus Component
participant Overlay as Card Overlay
User->>Card: Tap freeze toggle
Card->>Card: Haptic feedback
Card->>Sheet: Open with onConfirm callback
Sheet->>User: Display freeze confirmation
User->>Sheet: Tap confirm
Sheet->>API: changeCardStatus("FROZEN")
API->>API: Optimistic state update
Card->>Card: Update displayStatus
CardStatus->>CardStatus: Derive frozen flag
Overlay->>Overlay: Show dimming + snowflake
CardStatus->>User: Display unfreeze button
API-->>Card: Mutation settled
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
🦋 Changeset detectedLatest commit: 7082d26 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a card freeze confirmation flow, a new shared Switch component, and UI updates to display a frozen state overlay on the card. Key changes include the addition of the CardFreezeSheet component, haptic feedback on card actions, and the integration of freeze controls into the home card status view. Feedback focuses on potential layout overflow in the new Switch component, optimizing the frozen overlay structure for better performance, and ensuring smooth animation transitions for the logo when the card status changes.
| {frozen ? null : ( | ||
| <Exa | ||
| width={50} | ||
| height={20} | ||
| style={styles.exa} | ||
| {...(Platform.OS === "web" ? undefined : { shouldRasterizeIOS: true })} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
The Exa logo is placed inside AnimatePresence but lacks animation properties (enterStyle, exitStyle). Consequently, it will abruptly appear or disappear instead of performing a smooth transition. Wrapping it in an animated View with appropriate styles will ensure a consistent user experience.
| {frozen ? null : ( | |
| <Exa | |
| width={50} | |
| height={20} | |
| style={styles.exa} | |
| {...(Platform.OS === "web" ? undefined : { shouldRasterizeIOS: true })} | |
| /> | |
| )} | |
| {frozen ? null : ( | |
| <View | |
| key="exa-logo" | |
| style={styles.exa} | |
| animation="default" | |
| enterStyle={{ opacity: 0 }} | |
| exitStyle={{ opacity: 0 }} | |
| > | |
| <Exa | |
| width={50} | |
| height={20} | |
| {...(Platform.OS === "web" ? undefined : { shouldRasterizeIOS: true })} | |
| /> | |
| </View> | |
| )} |
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Prevent Tests Dashboard |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/card/Card.tsx (1)
205-215:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFreeze/unfreeze failures are silent.
setCardStatuserrors are only reported to telemetry from the call sites, so a failed freeze/unfreeze request snaps the optimistic UI back with no user-facing feedback. For a card-control action, that is easy to misread as success. Add anonErrortoast/haptic here and then drop the per-call.catch(reportError)handlers to avoid double-reporting. This same mutation pattern is mirrored insrc/components/home/CardStatus.tsx.💡 Suggested mutation-level handling
const { mutateAsync: changeCardStatus, isPending: isSettingCardStatus, variables: optimisticCardStatus, } = useMutation({ mutationKey: ["card", "status"], mutationFn: setCardStatus, + onError: (error) => { + reportError(error); + toast.show(t("An error occurred. Please try again later."), { + native: true, + duration: 1000, + burntOptions: { haptic: "error", preset: "error" }, + }); + }, onSettled: async () => { await queryClient.invalidateQueries({ queryKey: ["card", "details"] }); }, });
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cbe1d518-1666-4e69-9670-44accbf7d070
📒 Files selected for processing (14)
.changeset/cool-rockets-raise.md.changeset/frank-snails-move.md.changeset/six-breads-shout.md.changeset/social-icons-retire.md.changeset/spotty-rules-camp.md.changeset/tangy-deer-design.md.maestro/subflows/activateCard.yamlsrc/components/card/Card.tsxsrc/components/card/CardFreezeSheet.tsxsrc/components/card/exa-card/CardContents.tsxsrc/components/home/CardStatus.tsxsrc/components/shared/Switch.tsxsrc/i18n/es.jsonsrc/i18n/pt.json
| - assertVisible: Freeze your card? | ||
| - tapOn: Freeze card | ||
| - assertNotVisible: Freeze your card? | ||
| - waitForAnimationToEnd | ||
| - tapOn: Freeze card |
There was a problem hiding this comment.
Make freeze/unfreeze assertions deterministic to avoid false positives.
tapOn: Freeze card is reused for both the sheet CTA and the card row, and the second tap can be ignored if isSettingCardStatus is still true (src/components/card/Card.tsx, onPress guard). This can pass the flow without actually validating unfreeze.
Proposed flow hardening
- assertVisible: Freeze your card?
-- tapOn: Freeze card
+- tapOn:
+ text: Freeze card
+ below: Freeze your card?
- assertNotVisible: Freeze your card?
- waitForAnimationToEnd
- tapOn: Freeze card
+- waitForAnimationToEnd
+- tapOn: Freeze card
+- assertVisible: Freeze your card?
+- tapOn: Cancel📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - assertVisible: Freeze your card? | |
| - tapOn: Freeze card | |
| - assertNotVisible: Freeze your card? | |
| - waitForAnimationToEnd | |
| - tapOn: Freeze card | |
| - assertVisible: Freeze your card? | |
| - tapOn: | |
| text: Freeze card | |
| below: Freeze your card? | |
| - assertNotVisible: Freeze your card? | |
| - waitForAnimationToEnd | |
| - tapOn: Freeze card | |
| - waitForAnimationToEnd | |
| - tapOn: Freeze card | |
| - assertVisible: Freeze your card? | |
| - tapOn: Cancel |
1 participant
Summary by CodeRabbit
New Features
Internationalization