-
Notifications
You must be signed in to change notification settings - Fork 5.4k
feat: updated wallet ready page PR - 2 #36839
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
Conversation
…ive-app/react-canvas`
…neProperty` call
Co-authored-by: Ganesh Suresh Patra <gpatra1996@gmail.com>
…ive-app/react-canvas`
…neProperty` call
ui/contexts/rive-wasm/index.tsx
Outdated
| const newArrayBuffer = await response.arrayBuffer(); | ||
| setUrlBufferCache(url, newArrayBuffer); | ||
| return newArrayBuffer; | ||
| }, [isWasmReady, url, setUrlBufferCache]); |
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.
Bug: Stale Cache Risk From Missing Dependency Re-Evaluation
The useRiveWasmFile hook is missing the urlBufferMap dependency in its useAsyncResult dependencies array (line 117). The hook reads urlBufferMap[url] on line 106 to check for a cached buffer, but urlBufferMap is not listed in the dependency array. This means the hook won't re-execute when the cache is updated by other components, potentially causing stale cache reads.
ui/contexts/rive-wasm/index.tsx
Outdated
| await RuntimeLoader.awaitInstance(); | ||
| setIsWasmReady(true); | ||
| return true; | ||
| }, [isWasmReady, setIsWasmReady]); |
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.
Bug: Infinite loop in wasm readiness hook
Infinite loop in useRiveWasmReady hook. When isWasmReady is true, the async function checks if (isWasmReady) and calls setIsWasmReady(true). Since isWasmReady is in the dependency array [isWasmReady, setIsWasmReady], this causes useAsyncResult to re-execute the async function, which again calls setIsWasmReady(true), creating an infinite loop. The check should either return early without calling setState, or the dependency array should not include isWasmReady if it's only used as an early exit condition.
| }, | ||
| "seedPhraseReviewDetails2": { | ||
| "message": "Geben Sie sie niemals an Dritte weiter." | ||
| "description": "The $1 is the bolded text 'Secret Recovery Phrase'" |
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.
Bug: Locale message still shows obsolete parameter placeholder
The locale message "seedPhraseReviewDetails" still contains "$2" in the message text, but the description was updated to indicate it only takes "$1" (the bolded text 'Secret Recovery Phrase'). The English version was updated to combine both messages into one, removing the second parameter, but the German locale still has the old format with "$2" that no longer exists. This will cause the message to display "$2" literally instead of the intended text. The same issue likely exists in other locale files (pt, vi, hi, fr, es, zh_CN, tr, ja, ko, tl, ga, en_GB).
Builds ready [410ae61]
UI Startup Metrics (1220 ± 84 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [5523186]
UI Startup Metrics (1258 ± 119 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
| if (!context) { | ||
| throw new Error('useRiveWasm must be used within RiveWasmProvider'); | ||
| } | ||
| return context; |
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.
Bug: Default context should be undefined to trigger validation
The useRiveWasmContext validation check if (!context) will never throw an error because the RiveWasmContext is created with a default value (an object with default properties). When useContext is called outside of a provider, it returns this default object instead of undefined, so the validation check always passes. This means the hook can be used outside the provider without any error, potentially causing silent bugs. The context should be created with undefined as the default value (like createContext<ContextType | undefined>(undefined)) for the validation check to work correctly.
Builds ready [139d3b5]
UI Startup Metrics (1227 ± 82 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚀 Bundle size reduced!]
|
Builds ready [31f1143]
UI Startup Metrics (1223 ± 86 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚀 Bundle size reduced!]
|
LeVinhGithub
left a comment
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.
re-approve
Description
In this pull request, we have updated the wallet-ready page. Specifically, we have replaced the "Remind Me Later" UI with the content on the wallet-ready page. Changes include removing all previous content and replacing it with a simple title—Your wallet is ready. Additionally, we introduced a Rive animation loader and positioned the "Manage Settings" text button below the "Done" button.
Jira Link: https://consensyssoftware.atlassian.net/browse/SL-54
Figma Link: https://www.figma.com/design/pViOUcmjwhEzFsdrwknpNc/Onboarding?node-id=16495-182046&m=dev
Changelog
CHANGELOG entry: we have updated the wallet-ready page
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Screen.Recording.2025-10-14.at.5.45.55.PM.mov
Screen.Recording.2025-10-15.at.1.22.21.PM.mov
Screen.Recording.2025-10-15.at.1.22.38.PM.mov
Fixed Word Wrap in Review SRP UI:
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Reworks the wallet-ready page with a new Rive animation and streamlined UI, introduces a shared Rive WASM context, and consolidates SRP copy/locales; updates headers, styles, and tests accordingly.
creation-successfulto show title"Your wallet is ready!", a Rive animation (WalletReadyAnimation), a primaryDonebutton, and a text "Manage default settings" button below.ONBOARDING_COMPLETION_ROUTEwhen arriving from SRP backup/reminder.ui/contexts/rive-wasmwith hooks (useRiveWasmReady,useRiveWasmFile) and provider; wraps app inRiveWasmProvider(ui/pages/index.js).fox-appear-animation.tsxandmetamask-wordmark-animation.tsxto use the shared context/file-loading; removesui/pages/onboarding-flow/rive-wasm/*init.wallet-ready-animation.tsxand related styles (index.scss).seedPhraseReviewDetailsand removesseedPhraseReviewDetails2across locales.walletReady*reminder/social strings; keepsyourWalletIsReadyandyourWalletIsReadyFromReminder.Written by Cursor Bugbot for commit 31f1143. This will update automatically on new commits. Configure here.