-
-
Notifications
You must be signed in to change notification settings - Fork 33
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
Fix: show GiveBack Toast when token is eligible #4661
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes enhance the functionality of QR code donation components by modifying the Changes
Possibly related issues
Poem
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (6)
Files skipped from review due to trivial changes (2)
Files skipped from review as they are similar to previous changes (3)
Additional context usedBiome
Additional comments not posted (5)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
…giveth-dapps-v2 into feat/stellar_integration
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: 0
Outside diff range, codebase verification and nitpick comments (1)
src/components/views/donate/OnTime/SelectTokenModal/QRCodeDonation/QRDonationCard.tsx (1)
196-196
: Simplify the conditional expression.The ternary operator can be simplified by directly assigning the negated value of
isSignedIn && isEnabled
.Apply this diff to simplify the code:
-anonymous: isSignedIn && isEnabled ? false : true, +anonymous: !(isSignedIn && isEnabled),Tools
Biome
[error] 196-196: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
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
const stopTimerFun = startTimer?.(new Date(draftDonationData?.expiresAt)); | ||
setStopTimer(() => stopTimerFun); |
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!
The code change is approved. Setting the stopTimer
variable using the setStopTimer
function ensures that the timer can be stopped based on the qrDonationStatus
, enhancing the responsiveness of the component.
Please apply the following diff to fix the formatting issue:
-const stopTimerFun = startTimer?.(new Date(draftDonationData?.expiresAt));
+const stopTimerFun = startTimer?.(
+ new Date(draftDonationData?.expiresAt),
+);
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.
const stopTimerFun = startTimer?.(new Date(draftDonationData?.expiresAt)); | |
setStopTimer(() => stopTimerFun); | |
const stopTimerFun = startTimer?.( | |
new Date(draftDonationData?.expiresAt), | |
); | |
setStopTimer(() => stopTimerFun); |
Tools
GitHub Check: build
[failure] 79-79:
Replacenew·Date(draftDonationData?.expiresAt)
with⏎↹↹↹↹new·Date(draftDonationData?.expiresAt),⏎↹↹↹
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
|
||
const [tokenPrice, setTokenPrice] = useState(0); | ||
const [stopTimer, setStopTimer] = React.useState<void | (() => void)>(); |
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, but please address the static analysis hint.
The code change is approved. Defining stopTimer
as a state variable using the useState
hook enhances the control flow and allows the timer to be stopped based on the qrDonationStatus
.
Please apply the following diff to fix the union type based on the static analysis hint:
-const [stopTimer, setStopTimer] = React.useState<void | (() => void)>();
+const [stopTimer, setStopTimer] = React.useState<undefined | (() => void)>();
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.
const [stopTimer, setStopTimer] = React.useState<void | (() => void)>(); | |
const [stopTimer, setStopTimer] = React.useState<undefined | (() => void)>(); |
Tools
Biome
[error] 43-43: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
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.
Thanks @Meriem-BM LGTM
Summary by CodeRabbit