Copy prompt v2 #2512
Copy prompt v2 #2512
Conversation
|
This PR was not deployed automatically as @atharvadeosthale does not have access to the Railway project. In order to get automatic PR deploys, please add @atharvadeosthale to your workspace on Railway. |
ConsoleProject ID: Tip You can use Avatars API to generate QR code for any text or URLs. |
WalkthroughAdds an LLM-assisted starter-kit banner and supporting logic to platform creation flows. New files: Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (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.
Actionable comments posted: 3
🧹 Nitpick comments (7)
src/routes/(console)/project-[region]-[project]/overview/platforms/createReactNative.svelte (1)
71-75: Consider consistent prop naming.The
promptConfigCodevariable is passed toLlmBannerasconfigCode, while other platform files directly use a variable namedconfigCode. Consider renaming this toconfigCodefor consistency across the codebase.Apply this diff:
- const promptConfigCode = ` + const configCode = ` const client = new Client() .setProject("${projectId}") .setEndpoint("${sdk.forProject(page.params.region, page.params.project).client.config.endpoint}") `;Then update line 260:
<LlmBanner platform="reactnative" - configCode={promptConfigCode} + {configCode} {alreadyExistsInstructions} openers={['cursor']} />src/routes/(console)/project-[region]-[project]/overview/platforms/createWeb.svelte (1)
166-212: Consider standardizing the banner configuration approach.The Web platform uses a
LLMPromptConfigobject approach while other platforms (Android, Apple, Flutter, React Native) use individual props (platform,configCode,alreadyExistsInstructions). This creates an inconsistency across the codebase.Consider either:
- Updating other platforms to use the
LLMPromptConfigapproach, or- Updating Web to use individual props like other platforms
The
LLMPromptConfigapproach appears more maintainable and type-safe, so option 1 might be preferable.src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte (3)
28-33: Consider user-friendly error handling instead of throwing.The derived config throws an error if required props are missing. This could crash the component and show a developer-facing error to users. Consider either:
- Rendering a fallback UI with a user-friendly message, or
- Using console.error and returning a minimal valid config
Example:
const config = $derived.by(() => { if (customConfig) return customConfig; if (platform && configCode) return buildPlatformConfig(platform, configCode, alreadyExistsInstructions); - throw new Error('LlmBanner: must provide either config OR (platform + configCode)'); + console.error('LlmBanner: must provide either config OR (platform + configCode)'); + return null; });Then add a guard in the template:
{#if showAlert && config} <!-- existing content --> {/if}
135-135: Use kebab-case for aria attributes.The
ariaLabelprop should bearia-labelfollowing HTML attribute naming conventions. While Svelte may handle the conversion, using the standard kebab-case is clearer.Apply this diff:
- ariaLabel="Open action menu" + aria-label="Open action menu"
190-244: Reduce reliance on !important.The styles contain multiple
!importantdeclarations which suggest CSS specificity issues. Consider using more specific selectors or scoped styles instead of forcing precedence with!important. This makes the styles more maintainable and easier to override when needed.src/routes/(console)/project-[region]-[project]/overview/platforms/store.ts (2)
112-120: Consider narrowing the platformKey parameter type.The
platformKeyparameter is currently typed asstring, but it could be narrowed to the keys ofplatformConfigsfor better type safety at compile time.Apply this diff to narrow the type:
export function buildPlatformConfig( - platformKey: string, + platformKey: keyof typeof platformConfigs, configCode: string, alreadyExistsInstructions: string ): LLMPromptConfig {
122-131: Optional: Use object property shorthand.Line 124 can use ES6 shorthand syntax for cleaner code.
Apply this diff:
return { title: config.title, - alreadyExistsInstructions: alreadyExistsInstructions, + alreadyExistsInstructions, cloneCommand: `git clone https://github.com/appwrite/${config.repoName}\ncd ${config.repoName}`,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
package.json(1 hunks)src/lib/actions/analytics.ts(1 hunks)src/routes/(console)/project-[region]-[project]/overview/components/CursorIconLarge.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/overview/platforms/createAndroid.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/overview/platforms/createApple.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/overview/platforms/createFlutter.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/overview/platforms/createReactNative.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/overview/platforms/createWeb.svelte(3 hunks)src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/overview/platforms/store.ts(1 hunks)
🔇 Additional comments (13)
package.json (1)
27-27: Dependency updates verified and valid.Both commit hashes (85d1b43) for
@appwrite.io/pink-icons-svelteand@appwrite.io/pink-svelteare reachable and successfully resolve in the lockfile. The dependencies are compatible with Svelte 5.25.3 and with each other (pink-svelte correctly depends on pink-icons-svelte 2.0.0-RC.1).The llmBanner component exists and uses icons from the updated pink-icons-svelte library. The codebase extensively uses both updated dependencies across hundreds of files with no import breakage detected. The lockfile confirms successful resolution and no version conflicts.
src/routes/(console)/project-[region]-[project]/overview/platforms/createAndroid.svelte (1)
200-204: LGTM!The LlmBanner integration follows the established pattern and correctly passes the necessary props.
src/routes/(console)/project-[region]-[project]/overview/platforms/createReactNative.svelte (1)
42-62: LGTM!The setup instructions are well-structured and correctly use template literals to interpolate the dynamic projectId and endpoint values.
src/routes/(console)/project-[region]-[project]/overview/platforms/createApple.svelte (1)
73-78: Good refactor to const.Changing
platformsfromlettoconstis appropriate since the object is never reassigned. This makes the code's intent clearer.src/routes/(console)/project-[region]-[project]/overview/platforms/createFlutter.svelte (2)
42-62: LGTM!The setup instructions are well-structured and correctly use template literals to interpolate dynamic values. The Flutter SDK integration guidance is clear and comprehensive.
308-312: LGTM!The LlmBanner integration follows the established pattern across other platform creation flows.
src/lib/actions/analytics.ts (1)
198-201: LGTM!The new analytics event identifiers follow the established naming convention and align with the LLM banner feature's interaction tracking needs. The trailing comma addition is also a good practice.
src/routes/(console)/project-[region]-[project]/overview/platforms/createWeb.svelte (1)
356-356: LGTM!The LlmBanner integration correctly uses the config object and includes both 'cursor' and 'lovable' openers, providing users with multiple AI tool options.
src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte (2)
87-98: LGTM!The copyPrompt function correctly implements clipboard copying with proper analytics tracking and user feedback via notifications.
51-83: Well-structured opener configuration.The
openersConfigobject provides a clean, extensible pattern for supporting multiple AI tools. The URL generation logic correctly encodes prompts and tracks analytics events.src/routes/(console)/project-[region]-[project]/overview/platforms/store.ts (3)
20-29: LGTM: Well-structured type definition.The LLMPromptConfig type is comprehensive and includes all necessary fields for prompt generation across different platforms.
35-57: LGTM: Clear prompt generation logic.The function generates a well-structured prompt with appropriate fallback handling (line 44) and clear section delineation.
69-110: Web platform is intentionally configured separately—no missing configuration.The web platform already exists and is properly implemented, but uses a different configuration approach than the other platforms. The
createWeb.sveltecomponent defines its ownllmConfigdirectly (lines 166-212) rather than relying on theplatformConfigsmapping. This is by design, not an oversight, since web platforms have different setup requirements than mobile frameworks.No action required.
| import Dark from '../assets/dark/cursor-ai.svg'; | ||
| </script> | ||
|
|
||
| <img src={$app.themeInUse === 'dark' ? Dark : Light} width="20" height="20" alt="" /> |
There was a problem hiding this comment.
Add descriptive alt text for accessibility.
The img element has an empty alt attribute. Consider adding descriptive alt text like "Cursor AI" or "Cursor" to improve accessibility for screen-reader users.
Apply this diff:
-<img src={$app.themeInUse === 'dark' ? Dark : Light} width="20" height="20" alt="" />
+<img src={$app.themeInUse === 'dark' ? Dark : Light} width="20" height="20" alt="Cursor" />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <img src={$app.themeInUse === 'dark' ? Dark : Light} width="20" height="20" alt="" /> | |
| <img src={$app.themeInUse === 'dark' ? Dark : Light} width="20" height="20" alt="Cursor" /> |
🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/overview/components/CursorIconLarge.svelte
around line 7, the <img> tag currently has an empty alt attribute; replace
alt="" with a descriptive string such as alt="Cursor AI" (or "Cursor" if
preferred) so screen readers receive meaningful context, keeping the rest of the
tag (theme-based src, width, height) unchanged.
There was a problem hiding this comment.
Thank you for thanking me! Your feedback helps me improve my assistance. I'm always here to help with your code reviews and questions!
(\ /)
( ^_^ )
o(")_(")o
src/routes/(console)/project-[region]-[project]/overview/platforms/createAndroid.svelte
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/overview/platforms/createApple.svelte
Outdated
Show resolved
Hide resolved
…orms/createApple.svelte Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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/(console)/project-[region]-[project]/overview/platforms/createApple.svelte(3 hunks)
🔇 Additional comments (3)
src/routes/(console)/project-[region]-[project]/overview/platforms/createApple.svelte (3)
31-31: LGTM!The import is correctly structured and the component is properly utilized in the template.
73-78: Good immutability improvement.Changing from
lettoconstprevents accidental mutation and follows best practices since this object is never reassigned.
228-232: LGTM!The
LlmBannercomponent is properly integrated with appropriate props and logical placement within the Clone starter flow.
src/routes/(console)/project-[region]-[project]/overview/platforms/createApple.svelte
Outdated
Show resolved
Hide resolved
| {/if} | ||
|
|
||
| <style lang="scss"> | ||
| :global(.btn-no-right-radius), |
There was a problem hiding this comment.
Can we please avoid or reduce the use of global classes here? They tend to introduce unintended side effects
There was a problem hiding this comment.
Strangely things were not working if I didn't use global, I'll try again
| variant="m-500" | ||
| >{o.label}</Typography.Text> | ||
| <Typography.Text | ||
| variant="m-400" |
There was a problem hiding this comment.
m-400 is the default, we can omit this
| color: var(--fgcolor-neutral-primary) !important; | ||
| } | ||
| & :global(p) { |
There was a problem hiding this comment.
Typography colors are not working?
| import Dark from '../assets/dark/cursor-ai.svg'; | ||
| </script> | ||
|
|
||
| <img src={$app.themeInUse === 'dark' ? Dark : Light} width="20" height="20" alt="" /> |
Summary by CodeRabbit
New Features
Chores