feat: add hidden email input to password reset form for password manager#2415
feat: add hidden email input to password reset form for password manager#2415HarshMN2345 wants to merge 2 commits intomainfrom
Conversation
…ger compatibility
ConsoleProject ID: Sites (2)
Note Appwrite has a Discord community with over 16 000 members. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughA 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 (2)
🚧 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). (2)
🔇 Additional comments (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.
Pull Request Overview
This PR adds a hidden email input field to the password reset form to improve password manager compatibility. Password managers typically need both username/email and password fields to properly detect and save credentials.
- Adds a hidden email input field with proper accessibility attributes
- Extracts email from URL parameters for the hidden field
- Enables autocomplete on password inputs
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| autocomplete={true} | ||
| bind:value={password} /> | ||
| <InputPassword | ||
| label="Confirm password" | ||
| placeholder="Confirm password" | ||
| id="confirm-password" | ||
| required={true} | ||
| autocomplete={true} |
There was a problem hiding this comment.
The autocomplete attribute should be a string value, not a boolean. For password fields, use 'new-password' for the new password field to indicate this is a password creation scenario.
| autocomplete={true} | |
| bind:value={password} /> | |
| <InputPassword | |
| label="Confirm password" | |
| placeholder="Confirm password" | |
| id="confirm-password" | |
| required={true} | |
| autocomplete={true} | |
| autocomplete="new-password" | |
| bind:value={password} /> | |
| <InputPassword | |
| label="Confirm password" | |
| placeholder="Confirm password" | |
| id="confirm-password" | |
| required={true} | |
| autocomplete="new-password" |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/(public)/recover/+page.svelte(2 hunks)
⏰ 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). (2)
- GitHub Check: e2e
- GitHub Check: build
8324eb5 to
563b2bd
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| <input | ||
| type="email" | ||
| name="email" | ||
| value={userEmail} | ||
| style="position: absolute; left: -9999px; opacity: 0; pointer-events: none;" | ||
| tabindex="-1" | ||
| autocomplete="username" | ||
| aria-hidden="true" /> |
There was a problem hiding this comment.
[nitpick] Using inline styles for hiding elements is not recommended. Consider using CSS classes or data attributes for better maintainability and consistency with the project's styling approach.
| autofocus={autofocus || undefined} | ||
| autocomplete={autocomplete ? 'on' : 'off'} | ||
| autocomplete={typeof autocomplete === 'string' | ||
| ? (autocomplete as HTMLInputElement['autocomplete']) |
There was a problem hiding this comment.
[nitpick] The type assertion as HTMLInputElement['autocomplete'] is unnecessary since TypeScript can infer the type when autocomplete is a string. Consider removing the type assertion for cleaner code.
| ? (autocomplete as HTMLInputElement['autocomplete']) | |
| ? autocomplete |
There was a problem hiding this comment.
@HarshMN2345 type casting don't work in svelte context.
| onMount(() => { | ||
| userId = page.url.searchParams.get('userId'); | ||
| secret = page.url.searchParams.get('secret'); | ||
| userEmail = page.url.searchParams.get('email') || ''; |
There was a problem hiding this comment.
does the recovery email email include userEmail? I doubt so 🤔
| autofocus={autofocus || undefined} | ||
| autocomplete={autocomplete ? 'on' : 'off'} | ||
| autocomplete={typeof autocomplete === 'string' | ||
| ? (autocomplete as HTMLInputElement['autocomplete']) |
There was a problem hiding this comment.
@HarshMN2345 type casting don't work in svelte context.

What does this PR do?
(Provide a description of what this PR does.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)
Summary by CodeRabbit