[TASK-14647] Fix/links feedback changes#1240
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds two-stage cancel-link UI flow and timing to Send Success and Transaction Details views, shifting loading to button-level and adding unmount cleanup. Tweaks UnsupportedBrowserModal CTA layout and copy button padding. Introduces optional ctaClassName prop to ActionModal and passes it from UnsupportedBrowserModal. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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 |
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/Send/link/views/Success.link.send.view.tsx (1)
104-114: Use TRANSACTIONS constant for queryKey (avoid cache misses).Replace hard-coded
queryKey: ['transactions']withqueryKey: [TRANSACTIONS]and import theTRANSACTIONSconstant to match other files.
- src/components/Send/link/views/Success.link.send.view.tsx — queryKey at ~line 106
- src/components/Send/link/views/Initial.link.send.view.tsx — queryKey at ~line 57
🧹 Nitpick comments (4)
src/components/TransactionDetails/TransactionDetailsReceipt.tsx (2)
66-66: De-duplicate Cancel Link statuses; use shared constants for i18n/typo-safety.Replace string literals with a shared constant to prevent drift across files and ease localization.
Apply this diff in this file:
-const [cancelLinkText, setCancelLinkText] = useState<'Cancelling' | 'Cancelled' | 'Cancel link'>('Cancel link') +const [cancelLinkText, setCancelLinkText] = useState<CancelLinkText>(CANCEL_LINK_TEXT.IDLE)-disabled={isLoading || cancelLinkText === 'Cancelled'} +disabled={isLoading || cancelLinkText === CANCEL_LINK_TEXT.DONE}-setCancelLinkText('Cancelling') +setCancelLinkText(CANCEL_LINK_TEXT.LOADING)- setCancelLinkText('Cancelled') + setCancelLinkText(CANCEL_LINK_TEXT.DONE)- setCancelLinkText('Cancel link') + setCancelLinkText(CANCEL_LINK_TEXT.IDLE)Add these shared defs (new file or existing constants module):
// src/constants/ui.consts.ts export const CANCEL_LINK_TEXT = { IDLE: 'Cancel link', LOADING: 'Cancelling', DONE: 'Cancelled', } as const export type CancelLinkText = typeof CANCEL_LINK_TEXT[keyof typeof CANCEL_LINK_TEXT]Also applies to: 812-812, 1049-1049, 1063-1064, 1074-1074
1049-1049: Guard against setState after unmount; clear timers.The 3s timeout + async then 2s delay can fire after unmount, causing React warnings. Track/clear timeouts and gate state updates.
Apply this minimal patch in this block:
- setCancelLinkText(CANCEL_LINK_TEXT.LOADING) + setCancelLinkText(CANCEL_LINK_TEXT.LOADING)- .then(async () => { - setIsLoading(false) - setCancelLinkText(CANCEL_LINK_TEXT.DONE) - await new Promise((resolve) => setTimeout(resolve, 2000)) - onClose() - }) + .then(async () => { + if (isMountedRef.current) setIsLoading(false) + if (isMountedRef.current) setCancelLinkText(CANCEL_LINK_TEXT.DONE) + await sleepSafe(2000, isMountedRef) + if (isMountedRef.current) onClose() + })- setCancelLinkText(CANCEL_LINK_TEXT.IDLE) + if (isMountedRef.current) setCancelLinkText(CANCEL_LINK_TEXT.IDLE)Add helpers near the top of the component:
// mount guards and safe sleep const isMountedRef = React.useRef(true) useEffect(() => { return () => { isMountedRef.current = false } }, []) const sleepSafe = (ms: number, ref: React.MutableRefObject<boolean>) => new Promise<void>((resolve) => { const id = setTimeout(() => resolve(), ms) if (!ref.current) { clearTimeout(id) resolve() } })Also applies to: 1062-1065, 1074-1074
src/components/Send/link/views/Success.link.send.view.tsx (2)
30-31: Use shared CANCEL_LINK_TEXT for consistency with TransactionDetailsReceipt.Prevents string drift and simplifies translation.
Apply this diff:
-const [cancelLinkText, setCancelLinkText] = useState<'Cancelling' | 'Cancelled' | 'Cancel link'>('Cancel link') +const [cancelLinkText, setCancelLinkText] = useState<CancelLinkText>(CANCEL_LINK_TEXT.IDLE)-disabled={isLoading || cancelLinkText === 'Cancelled'} +disabled={isLoading || cancelLinkText === CANCEL_LINK_TEXT.DONE}-<span>{cancelLinkText}</span> +<span>{cancelLinkText}</span>-setCancelLinkText('Cancelling') +setCancelLinkText(CANCEL_LINK_TEXT.LOADING)- .then(async () => { - setIsLoading(false) - setCancelLinkText('Cancelled') - await new Promise((resolve) => setTimeout(resolve, 2000)) + .then(async () => { + if (isMountedRef.current) setIsLoading(false) + if (isMountedRef.current) setCancelLinkText(CANCEL_LINK_TEXT.DONE) + await sleepSafe(2000, isMountedRef) router.push('/home') })- setCancelLinkText('Cancel link') + if (isMountedRef.current) setCancelLinkText(CANCEL_LINK_TEXT.IDLE)Also add the same helpers as suggested in the receipt component:
// mount guards and safe sleep const isMountedRef = useRef(true) useEffect(() => () => { isMountedRef.current = false }, []) const sleepSafe = (ms: number, ref: React.MutableRefObject<boolean>) => new Promise<void>((resolve) => { const id = setTimeout(() => resolve(), ms) if (!ref.current) { clearTimeout(id); resolve() } })Also applies to: 71-72, 82-82, 97-97, 109-112, 120-120
32-37: Unmount cleanup only resets redux; also cancel pending timers.Add timer cleanup alongside resetSendFlow to avoid late UI updates post-unmount.
Add:
const timeouts: number[] = [] const pushTimeout = (id: number) => timeouts.push(id) useEffect(() => { return () => { dispatch(sendFlowActions.resetSendFlow()) timeouts.forEach(clearTimeout) } }, [dispatch])And whenever you call setTimeout, wrap as: pushTimeout(window.setTimeout(...)).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/app/(setup)/setup/page.tsx(1 hunks)src/components/Global/UnsupportedBrowserModal/index.tsx(3 hunks)src/components/Send/link/views/Success.link.send.view.tsx(6 hunks)src/components/TransactionDetails/TransactionDetailsReceipt.tsx(6 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Zishan-7
PR: peanutprotocol/peanut-ui#1072
File: src/app/(setup)/setup/page.tsx:173-175
Timestamp: 2025-08-07T12:53:50.946Z
Learning: In the peanut-ui setup flow at `src/app/(setup)/setup/page.tsx`, when handling unsupported scenarios, both device not supported and browser not supported cases should show the same "Unsupported browser" message using the `UnsupportedBrowserModal` component, rather than having distinct messaging for each scenario.
📚 Learning: 2025-08-07T12:53:50.946Z
Learnt from: Zishan-7
PR: peanutprotocol/peanut-ui#1072
File: src/app/(setup)/setup/page.tsx:173-175
Timestamp: 2025-08-07T12:53:50.946Z
Learning: In the peanut-ui setup flow at `src/app/(setup)/setup/page.tsx`, when handling unsupported scenarios, both device not supported and browser not supported cases should show the same "Unsupported browser" message using the `UnsupportedBrowserModal` component, rather than having distinct messaging for each scenario.
Applied to files:
src/app/(setup)/setup/page.tsxsrc/components/Global/UnsupportedBrowserModal/index.tsx
🧬 Code graph analysis (1)
src/components/Send/link/views/Success.link.send.view.tsx (1)
src/redux/slices/send-flow-slice.ts (1)
sendFlowActions(101-101)
🪛 Biome (2.1.2)
src/app/(setup)/setup/page.tsx
[error] 173-173: Unexpected constant condition.
(lint/correctness/noConstantCondition)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (2)
src/components/Global/UnsupportedBrowserModal/index.tsx (2)
104-104: LGTM: Button padding improvement.The addition of
sm:py-3provides better vertical spacing on small screens, improving the button's touch target and visual balance.
130-130: LGTM: Consistent CTA layout.The
ctaClassName="flex-col sm:flex-col"ensures CTAs are stacked vertically on all screen sizes, providing a consistent layout that works well for the modal's copy-and-paste workflow.
This PR also fixes TASK-14695