Skip to content

Conversation

@joaquim-verges
Copy link
Member

@joaquim-verges joaquim-verges commented Dec 18, 2024

Problem solved

Fixes TOOL-2785


PR-Codex overview

This PR focuses on enhancing the login functionality in the CustomLoginForm component by implementing email verification before connecting the wallet, and refactoring the login UI.

Detailed summary

  • Changed handleLogin to send an email verification code before connecting.
  • Updated the UI to separate login and verification screens.
  • Renamed CustomLoginForm to CustomLoginUi for clarity.
  • Removed unused imports and mutation logic related to custom auth.
  • Added state management for verificationCode and screen.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@vercel
Copy link

vercel bot commented Dec 18, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs-v2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 18, 2024 9:04pm
thirdweb_playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 18, 2024 9:04pm
2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
thirdweb-www ⬜️ Skipped (Inspect) Dec 18, 2024 9:04pm
wallet-ui ⬜️ Skipped (Inspect) Dec 18, 2024 9:04pm

@vercel vercel bot temporarily deployed to Preview – wallet-ui December 18, 2024 20:56 Inactive
@vercel vercel bot temporarily deployed to Preview – thirdweb-www December 18, 2024 20:56 Inactive
@github-actions github-actions bot added Playground Changes involving the Playground codebase. Portal Involves changes to the Portal (docs) codebase. labels Dec 18, 2024
Copy link
Member Author

joaquim-verges commented Dec 18, 2024


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • merge-queue - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@linear
Copy link

linear bot commented Dec 18, 2024

@joaquim-verges joaquim-verges marked this pull request as ready for review December 18, 2024 20:57
@joaquim-verges joaquim-verges requested review from a team and jakubkrehel as code owners December 18, 2024 20:57
Comment on lines +92 to 117
export function CustomLoginUi() {
const { connect, isConnecting, error } = useConnect();
const { mutate: loginWithCustomAuth } = useMutation({
mutationFn: async (email: string) => {
const wallet = await connect(async () => {
const wallet = inAppWallet();
await wallet.connect({
strategy: "auth_endpoint",
client,
// your own custom auth payload here
payload: JSON.stringify({
userId: email,
email,
}),
});
return wallet;
const preLogin = async (email: string) => {
// send email verification code
await preAuthenticate({
client,
strategy: "email",
email,
});
};
const handleLogin = async (email: string, verificationCode: string) => {
// verify email with verificationCode and connect
connect(async () => {
const wallet = inAppWallet();
await wallet.connect({
client,
strategy: "email",
email,
verificationCode,
});
return wallet;
}
});
const handleSubmit = (e: React.FormEvent) => {
e.preventDefault();
loginWithCustomAuth(email);
});
};
return (
<form onSubmit={handleSubmit}>
<div>
<label htmlFor="email">
Email Address
</label>
<input
type="email"
id="email"
value={email}
onChange={(e) => setEmail(e.target.value)}
placeholder="Enter your email"
required
/>
<button
type="submit"
disabled={isConnecting || !email}
>
{isConnecting ? "Submitting..." : "Submit"}
</button>
{error && <p>{error.message}</p>}
</div>
</form>
);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CustomLoginUi component is missing its UI implementation. While the authentication logic is correct, the component needs to return JSX that renders the email input, verification code input, and submit buttons that call preLogin() and handleLogin(). Consider adding a form similar to the one that was removed, but updated to handle the two-step email verification flow.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

Base automatically changed from docs_Reorganize_TypeScript_and_React_documentation_structure to main December 18, 2024 20:58
@codecov
Copy link

codecov bot commented Dec 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 53.03%. Comparing base (ecb4571) to head (13329e8).
Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5796      +/-   ##
==========================================
+ Coverage   52.99%   53.03%   +0.04%     
==========================================
  Files        1102     1101       -1     
  Lines       59107    59079      -28     
  Branches     4809     4812       +3     
==========================================
+ Hits        31323    31333      +10     
+ Misses      27066    27028      -38     
  Partials      718      718              
Flag Coverage Δ *Carryforward flag
legacy_packages 65.68% <ø> (ø) Carriedforward from ccb4873
packages 50.18% <ø> (+0.04%) ⬆️

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
packages/thirdweb/src/wallets/in-app/web/in-app.ts 60.00% <ø> (ø)

... and 5 files with indirect coverage changes

@joaquim-verges joaquim-verges merged commit 07ad025 into main Dec 18, 2024
32 of 35 checks passed
@joaquim-verges joaquim-verges deleted the feat_Add_email_verification_flow_to_in-app_wallet branch December 18, 2024 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Playground Changes involving the Playground codebase. Portal Involves changes to the Portal (docs) codebase.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants