Feat : move GitHub button above #2580
Conversation
Console (appwrite/console)Project ID: Tip Storage files get ClamAV malware scanning and encryption by default |
|
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. WalkthroughThe login and register pages relocate the GitHub OAuth button to the top of the form (rendered only when Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (5)**/*.{ts,tsx,js,jsx,svelte}📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/routes/**/*.svelte📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,jsx,svelte,json}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.svelte📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/routes/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (3)📚 Learning: 2025-11-19T11:22:42.553ZApplied to files:
📚 Learning: 2025-11-25T03:15:27.539ZApplied to files:
📚 Learning: 2025-09-08T13:20:47.308ZApplied to files:
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 |
![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
🧹 Nitpick comments (1)
src/routes/(public)/(guest)/register/+page.svelte (1)
108-115: Consider preventing double-clicks in the GitHub OAuth flow.The
onGithubLoginfunction doesn't setdisabled = truebefore initiating the OAuth session, unlike the regularregisterfunction (line 54). This could allow users to trigger multiple OAuth flows if they click rapidly.Apply this diff to prevent double-clicks:
function onGithubLogin() { + disabled = true; sdk.forConsole.account.createOAuth2Session({ provider: OAuthProvider.Github, success: window.location.origin, failure: window.location.origin, scopes: ['read:user', 'user:email'] }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/routes/(public)/(guest)/login/+page.svelte(2 hunks)src/routes/(public)/(guest)/register/+page.svelte(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-08T13:20:47.308Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2316
File: src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte:29-29
Timestamp: 2025-09-08T13:20:47.308Z
Learning: The Form.svelte component in the Appwrite console creates a FormContext with isSubmitting as writable(false) and expects consumers to work with Svelte writable stores, not plain booleans.
Applied to files:
src/routes/(public)/(guest)/register/+page.sveltesrc/routes/(public)/(guest)/login/+page.svelte
⏰ 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
🔇 Additional comments (3)
src/routes/(public)/(guest)/login/+page.svelte (2)
97-108: LGTM! Clean implementation of cloud-specific GitHub OAuth login.The conditional rendering, button configuration, and OAuth flow integration are well-implemented. The button properly reuses the existing
disabledstate and theonGithubLoginhandler correctly constructs redirect URLs with query parameters.
138-142: LGTM! Appropriate use of global styles.The global scope is necessary here for Svelte, and the class name is specific enough to avoid conflicts. Using the CSS custom property with a fallback ensures consistent spacing with the design system.
src/routes/(public)/(guest)/register/+page.svelte (1)
186-190: LGTM! Global styling approach is appropriate for cross-page consistency.The
:global()scope for.auth-provider-buttonensures consistent spacing across authentication pages, as this pattern is mirrored in the login page.
There was a problem hiding this comment.
Pull request overview
This PR relocates the GitHub authentication buttons to appear above the form fields instead of below them on both the login and registration pages. The change is intended to improve the user experience by presenting alternative authentication options earlier in the flow.
Key Changes:
- Moved GitHub OAuth button from bottom to top of login form
- Moved GitHub OAuth button from bottom to top of registration form
- Added consistent styling for auth provider buttons with
margin-bottomspacing
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/routes/(public)/(guest)/register/+page.svelte |
Relocated GitHub sign-up button above form fields and added CSS styling for the button |
src/routes/(public)/(guest)/login/+page.svelte |
Relocated GitHub sign-in button above form fields and added CSS styling for the button |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| :global(.auth-provider-button) { | ||
| margin-bottom: var(--gap-s, 8px); | ||
| } |
There was a problem hiding this comment.
maybe lets just wrap the button for now in a div with this style. don't wanna add more globals. we anyway plan to remove class support from Pink because it makes us use :global. Same for the other one.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/(public)/(guest)/register/+page.svelte (1)
108-123: URL-encode query parameters and prevent rapid re-clicks.Two issues in the OAuth flow:
Query parameter values are not URL-encoded when building the success URL. Special characters in campaign codes or promo codes could break the redirect or be misinterpreted.
The function doesn't set
disabled = trueafter initiating the OAuth flow, allowing users to trigger multiple concurrent OAuth requests if they rapidly click the button.🔎 Proposed fix
function onGithubLogin() { + disabled = true; let successUrl = window.location.origin; if (page.url.searchParams.has('code')) { - successUrl += `?code=${page.url.searchParams.get('code')}`; + successUrl += `?code=${encodeURIComponent(page.url.searchParams.get('code'))}`; } else if (page.url.searchParams.has('campaign')) { - successUrl += `?campaign=${page.url.searchParams.get('campaign')}`; + successUrl += `?campaign=${encodeURIComponent(page.url.searchParams.get('campaign'))}`; } sdk.forConsole.account.createOAuth2Session({ provider: OAuthProvider.Github, success: successUrl, failure: window.location.origin, scopes: ['read:user', 'user:email'] }); }
♻️ Duplicate comments (1)
src/routes/(public)/(guest)/register/+page.svelte (1)
135-143: Enforce terms acceptance for GitHub OAuth signup.The GitHub signup button only checks
{disabled}but does not require terms acceptance. This allows users to sign up via GitHub OAuth without agreeing to the Privacy Policy and Terms of Use, unlike the email registration flow (line 179) which correctly enforcesdisabled={disabled || !terms}.🔎 Proposed fix
{#if isCloud} <div class="auth-provider-wrapper"> - <Button secondary fullWidth on:click={onGithubLogin} {disabled}> + <Button secondary fullWidth on:click={onGithubLogin} disabled={disabled || !terms}> <span class="icon-github" aria-hidden="true"></span> <span class="text">Sign up with GitHub</span> </Button> </div> <span class="with-separators eyebrow-heading-3">or</span> {/if}
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/routes/(public)/(guest)/login/+page.sveltesrc/routes/(public)/(guest)/register/+page.svelte
🚧 Files skipped from review as they are similar to previous changes (1)
- src/routes/(public)/(guest)/login/+page.svelte
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(public)/(guest)/register/+page.svelte
src/routes/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Files:
src/routes/(public)/(guest)/register/+page.svelte
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(public)/(guest)/register/+page.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/routes/(public)/(guest)/register/+page.svelte
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(public)/(guest)/register/+page.svelte
🧠 Learnings (3)
📚 Learning: 2025-11-19T11:22:42.553Z
Learnt from: atharvadeosthale
Repo: appwrite/console PR: 2512
File: src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte:51-83
Timestamp: 2025-11-19T11:22:42.553Z
Learning: In src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte, the Lovable integration URL format `https://lovable.dev/` with `autosubmit` and `prompt` as query parameters (set via searchParams) is correct and functional.
Applied to files:
src/routes/(public)/(guest)/register/+page.svelte
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to src/routes/**/*.svelte : Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Applied to files:
src/routes/(public)/(guest)/register/+page.svelte
📚 Learning: 2025-09-08T13:20:47.308Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2316
File: src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte:29-29
Timestamp: 2025-09-08T13:20:47.308Z
Learning: The Form.svelte component in the Appwrite console creates a FormContext with isSubmitting as writable(false) and expects consumers to work with Svelte writable stores, not plain booleans.
Applied to files:
src/routes/(public)/(guest)/register/+page.svelte
🔇 Additional comments (1)
src/routes/(public)/(guest)/register/+page.svelte (1)
190-195: LGTM!The style correctly uses design system spacing variables and provides appropriate visual separation for the OAuth button.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/(public)/(guest)/register/+page.svelte (1)
108-123: Setdisabled = trueafter initiating the OAuth flow to prevent rapid re-clicks.The function correctly preserves campaign and coupon query parameters. However, setting
disabled = trueimmediately after callingcreateOAuth2Sessionprevents users from rapidly clicking the GitHub button multiple times before the redirect completes.🔎 Proposed fix
function onGithubLogin() { let successUrl = window.location.origin; if (page.url.searchParams.has('code')) { successUrl += `?code=${page.url.searchParams.get('code')}`; } else if (page.url.searchParams.has('campaign')) { successUrl += `?campaign=${page.url.searchParams.get('campaign')}`; } sdk.forConsole.account.createOAuth2Session({ provider: OAuthProvider.Github, success: successUrl, failure: window.location.origin, scopes: ['read:user', 'user:email'] }); + disabled = true; }
♻️ Duplicate comments (1)
src/routes/(public)/(guest)/register/+page.svelte (1)
135-143: GitHub sign-up button must enforce terms agreement.The GitHub sign-up button only checks
{disabled}but does not verify that the user has agreed to the terms. This allows users to bypass the terms and conditions requirement by signing up via GitHub, creating a compliance risk. The email sign-up button at line 179 correctly enforcesdisabled={disabled || !terms}.🔎 Proposed fix
{#if isCloud} <div style:margin-bottom="var(--gap-s, 8px)"> - <Button secondary fullWidth on:click={onGithubLogin} {disabled}> + <Button secondary fullWidth on:click={onGithubLogin} disabled={disabled || !terms}> <span class="icon-github" aria-hidden="true"></span> <span class="text">Sign up with GitHub</span> </Button> </div> <span class="with-separators eyebrow-heading-3">or</span> {/if}
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/routes/(public)/(guest)/login/+page.sveltesrc/routes/(public)/(guest)/register/+page.svelte
🚧 Files skipped from review as they are similar to previous changes (1)
- src/routes/(public)/(guest)/login/+page.svelte
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(public)/(guest)/register/+page.svelte
src/routes/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Files:
src/routes/(public)/(guest)/register/+page.svelte
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(public)/(guest)/register/+page.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/routes/(public)/(guest)/register/+page.svelte
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(public)/(guest)/register/+page.svelte
🧠 Learnings (3)
📚 Learning: 2025-11-19T11:22:42.553Z
Learnt from: atharvadeosthale
Repo: appwrite/console PR: 2512
File: src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte:51-83
Timestamp: 2025-11-19T11:22:42.553Z
Learning: In src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte, the Lovable integration URL format `https://lovable.dev/` with `autosubmit` and `prompt` as query parameters (set via searchParams) is correct and functional.
Applied to files:
src/routes/(public)/(guest)/register/+page.svelte
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to src/routes/**/*.svelte : Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Applied to files:
src/routes/(public)/(guest)/register/+page.svelte
📚 Learning: 2025-09-08T13:20:47.308Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2316
File: src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte:29-29
Timestamp: 2025-09-08T13:20:47.308Z
Learning: The Form.svelte component in the Appwrite console creates a FormContext with isSubmitting as writable(false) and expects consumers to work with Svelte writable stores, not plain booleans.
Applied to files:
src/routes/(public)/(guest)/register/+page.svelte
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
✏️ Tip: You can customize this high-level summary in your review settings.