Conversation
…to export screen Co-authored-by: Yash <Yash094@users.noreply.github.com>
|
Cursor Agent can help with this pull request. Just |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 3377f1a The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis PR adds an initialScreen prop and a screen option to the wallet details modal/hook, allowing the modal to open directly at a specified screen (notably Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
size-limit report 📦
|
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (60.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #8698 +/- ##
==========================================
- Coverage 52.74% 52.73% -0.02%
==========================================
Files 934 934
Lines 62965 62968 +3
Branches 4136 4138 +2
==========================================
- Hits 33211 33204 -7
- Misses 29654 29666 +12
+ Partials 100 98 -2
🚀 New features to boost your workflow:
|
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/thirdweb/src/react/web/ui/ConnectWallet/Details.tsx`:
- Line 1830: Add unit tests that cover the new screen propagation through the
ConnectWallet UI by asserting that when open({ screen: "export" }) is invoked
the UI lands on the Export Private Key view and that calling open() without a
screen starts at "main"; specifically, write tests that render the component
which uses initialScreen={props.screen} (the Details.tsx wiring), call the open
helper/prop used by your component with and without a screen value, and assert
the rendered output contains the Export Private Key content for the "export"
case and the main screen content for the omission case.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8ad4a910-e7b9-4c50-8ab2-45878839b72b
📒 Files selected for processing (2)
.changeset/wallet-details-export-screen.mdpackages/thirdweb/src/react/web/ui/ConnectWallet/Details.tsx
| showTestnetFaucet: props.showTestnetFaucet, | ||
| }} | ||
| displayBalanceToken={props.displayBalanceToken} | ||
| initialScreen={props.screen} |
There was a problem hiding this comment.
Add regression tests for the new screen propagation path.
initialScreen={props.screen} is the core wiring for this feature, but the patch currently has no coverage on this line. Please add a test that verifies:
open({ screen: "export" })lands on Export Private Key- omitting
screenstill starts at"main"
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 1830-1830: packages/thirdweb/src/react/web/ui/ConnectWallet/Details.tsx#L1830
Added line #L1830 was not covered by tests
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/thirdweb/src/react/web/ui/ConnectWallet/Details.tsx` at line 1830,
Add unit tests that cover the new screen propagation through the ConnectWallet
UI by asserting that when open({ screen: "export" }) is invoked the UI lands on
the Export Private Key view and that calling open() without a screen starts at
"main"; specifically, write tests that render the component which uses
initialScreen={props.screen} (the Details.tsx wiring), call the open helper/prop
used by your component with and without a screen value, and assert the rendered
output contains the Export Private Key content for the "export" case and the
main screen content for the omission case.
Co-authored-by: Yash <Yash094@users.noreply.github.com>
3 participants
Slack Thread
PR-Codex overview
This PR introduces the ability to specify an initial screen when opening the
useWalletDetailsModal, allowing direct navigation to the "Export Private Key" screen.Detailed summary
initialScreenprop touseWalletDetailsModal.screenstate initialization to useprops.initialScreen.screenstate to include "export".screenprop inUseWalletDetailsModalOptions.Summary by CodeRabbit