✨ misc app improvements #705
Conversation
🦋 Changeset detectedLatest commit: 6839afd 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 |
WalkthroughAdds mount/unmount animations to GettingStarted and CardStatus, refactors onboarding step logic to accept hasKYC/isDeployed (removing external step mutation), centralizes richer KYC query defaults (retry and meta.suppressError), removes some local suppressions, and adds two patch changeset files. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Home / GettingStarted / CardStatus
participant Animator as AnimatePresence
participant QC as QueryClient
participant API as getKYCStatus API
participant Chain as Blockchain
UI->>Animator: mount GettingStarted / CardStatus (keyed)
Animator->>UI: apply enterStyle (opacity/translateY)
UI->>QC: request ["kyc","status"] (useQuery)
UI->>Chain: request bytecode for account (useBytecode)
QC->>API: call getKYCStatus
API-->>QC: success / error
alt success
QC-->>UI: return KYC status
else error
QC->>QC: retry up to 3 unless expected(error)
QC-->>UI: propagate or suppress per meta.suppressError
end
UI->>UI: compute hasKYC/isDeployed -> useOnboardingSteps({hasKYC,isDeployed})
Animator->>UI: apply exitStyle on unmount
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 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 |
Summary of ChangesHello @dieguezguille, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces several user experience improvements and bug fixes across the mobile application. It specifically addresses a gesture handler warning by ensuring callbacks are executed on the main React Native thread, and enhances the visual fluidity of the home screen by adding subtle entrance and exit animations to key components like the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![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 primarily focuses on enhancing the user experience with animations and fixing a worklet gesture warning. It introduces entry and exit animations to the CardStatus and GettingStarted components on the home screen by utilizing AnimatePresence from tamagui and applying specific animation properties. The BenefitCard component is updated to fix a worklet gesture warning by importing scheduleOnRN and wrapping the onPress callback within a worklet. Additionally, the KYC status query in Home.tsx is refined to prevent retries for specific API errors and the rendering condition for the GettingStarted component is changed from !isPendingKYC to isKYCFetched.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #705 +/- ##
==========================================
+ Coverage 65.27% 65.74% +0.47%
==========================================
Files 190 190
Lines 5996 5994 -2
Branches 1734 1734
==========================================
+ Hits 3914 3941 +27
+ Misses 1903 1873 -30
- Partials 179 180 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
cruzdanilo
force-pushed
the
fixes
branch
2 times, most recently
from
acd0f14 to
fcb7829
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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)
src/components/home/Home.tsx (1)
107-116: 🧹 Nitpick | 🔵 TrivialConsider extracting the shared error predicate.
The same error condition (
APIErrorwith specifictextvalues) is duplicated in bothretryandsuppressError. Extracting this into a helper function would reduce duplication and ensure both checks stay in sync.♻️ Proposed refactor
+function isExpectedKYCError(error: unknown): boolean { + return ( + error instanceof APIError && + (error.text === "no kyc" || error.text === "not started" || error.text === "bad kyc") + ); +} + const { data: KYCStatus, isFetched: isKYCFetched, refetch: refetchKYCStatus, } = useQuery({ queryKey: ["kyc", "status"], queryFn: async () => getKYCStatus(), - retry: (_, error) => - !( - error instanceof APIError && - (error.text === "no kyc" || error.text === "not started" || error.text === "bad kyc") - ), - meta: { - suppressError: (error) => - error instanceof APIError && - (error.text === "no kyc" || error.text === "not started" || error.text === "bad kyc"), - }, + retry: (_, error) => !isExpectedKYCError(error), + meta: { suppressError: isExpectedKYCError }, });
🤖 Fix all issues with AI agents
In `@src/components/home/Home.tsx`:
- Around line 201-203: The exit animation isn't triggered because GettingStarted
sometimes returns null while still mounted; change the mount condition to the
parent by rendering <GettingStarted .../> only when it should be shown (e.g.,
use isKYCFetched && (isKYCApproved === false || !bytecode) or a new boolean like
shouldShowGettingStarted) inside the AnimatePresence wrapper and remove the
internal early "return null" path from the GettingStarted component (so
GettingStarted no longer mounts and unmounts silently); update references to
props isDeployed/hasKYC (or isKYCApproved/bytecode) to compute the show/hide
logic in Home.tsx and keep GettingStarted responsible only for its animated
content.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
src/components/home/Home.tsx
Outdated
| )} | ||
| {!isPendingKYC && <GettingStarted isDeployed={!!bytecode} hasKYC={isKYCApproved} />} | ||
| <AnimatePresence> | ||
| {isKYCFetched && <GettingStarted isDeployed={!!bytecode} hasKYC={isKYCApproved} />} |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/home/Home.tsx (1)
107-116: 🧹 Nitpick | 🔵 TrivialConsider extracting the error check to reduce duplication.
The same error text check (
"no kyc","not started","bad kyc") is duplicated betweenretryandsuppressError. Extract a helper predicate for consistency and maintainability.♻️ Proposed refactor
+const isKYCPendingError = (error: unknown) => + error instanceof APIError && + (error.text === "no kyc" || error.text === "not started" || error.text === "bad kyc"); + const { data: KYCStatus, isFetched: isKYCFetched, refetch: refetchKYCStatus, } = useQuery({ queryKey: ["kyc", "status"], queryFn: async () => getKYCStatus(), - retry: (_, error) => - !( - error instanceof APIError && - (error.text === "no kyc" || error.text === "not started" || error.text === "bad kyc") - ), - meta: { - suppressError: (error) => - error instanceof APIError && - (error.text === "no kyc" || error.text === "not started" || error.text === "bad kyc"), - }, + retry: (_, error) => !isKYCPendingError(error), + meta: { suppressError: isKYCPendingError }, });
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/components/getting-started/GettingStarted.tsx`:
- Around line 26-38: The useOnboardingState hook (which calls useBytecode and
useQuery) is being invoked twice causing redundant hooks; update GettingStarted
to call useOnboardingState() once and pass the resulting { hasKYC, isDeployed }
into CurrentStep as props, then remove the duplicate useOnboardingState()
invocation from CurrentStep and change CurrentStep to accept these two props
instead of calling the hook itself; ensure any references to
useBytecode/useQuery remain only inside useOnboardingState and update prop
types/signature for CurrentStep accordingly.
- Line 29: The variable name returned from useQuery is using PascalCase
(KYCStatus); rename it to camelCase (kycStatus) in the destructuring of useQuery
in GettingStarted.tsx (const { data: kycStatus } = useQuery(...)) and update
every usage/reference in this file (including any JSX, conditionals, or
callbacks) to use kycStatus so it follows JS/TS naming conventions and remains
consistent with getKYCStatus and useQuery.
Summary by CodeRabbit
New Features
Bug Fixes
Chores