Replace language chooser with EthnoLib implementation#1481
Replace language chooser with EthnoLib implementation#1481
Conversation
📝 WalkthroughWalkthroughThe changes integrate a new modal-based language selection component ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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)
📝 Coding Plan
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 Tip CodeRabbit can use Trivy to scan for security misconfigurations and secrets in Infrastructure as Code files.Add a .trivyignore file to your project to customize which findings Trivy reports. |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
vite.config.ts (1)
7-14: Consider adding a comment explaining why these packages are excluded.The
optimizeDeps.excludeconfiguration is appropriate for Svelte component libraries that may not work well with Vite's esbuild-based pre-bundling. A brief comment would help future maintainers understand why these specific packages need special handling.📝 Suggested documentation
export default defineConfig({ + // Exclude `@ethnolib` packages from pre-bundling as they are Svelte component libraries + // that need to be processed by the Svelte compiler rather than esbuild optimizeDeps: { exclude: [ '@ethnolib/language-chooser-svelte-daisyui',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vite.config.ts` around lines 7 - 14, Add a brief explanatory comment above the optimizeDeps.exclude configuration that states why these packages are being excluded (e.g., they are Svelte component/controller/state libraries that break Vite's esbuild pre-bundling or require special handling), and mention the specific packages listed ('@ethnolib/language-chooser-svelte-daisyui', '@ethnolib/language-chooser-controller', '@ethnolib/find-language', '@ethnolib/state-management-svelte') so future maintainers understand the rationale for optimizeDeps.exclude.src/routes/(authenticated)/projects/new/[id=idNumber]/+page.svelte (1)
86-92: Button lacks visual affordance for required field.The button shows the placeholder text
m.project_languageCode()when empty, but there's norequiredattribute or visual indicator that language selection is mandatory. The submit button is disabled until$form.Language.lengthis truthy (line 140), but users may not understand why they can't submit.Consider adding a visual cue similar to the Name field's
required: truetreatment.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`(authenticated)/projects/new/[id=idNumber]/+page.svelte around lines 86 - 92, The language-picker button (onclick={() => showLanguageModal?.()}, displaying {$form.Language || m.project_languageCode()}) lacks a required-field affordance even though form submission is gated by $form.Language.length; update the button to surface that it is required by adding an accessible and visual cue: set aria-required="true" and add the same required visual treatment/class used by the Name field (e.g., the required:true styling or an explicit "required" CSS class and an asterisk in the label text), and ensure any client-side validation still checks $form.Language before enabling the submit button so behavior matches the new UI affordance.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/routes/`(authenticated)/projects/new/[id=idNumber]/+page.svelte:
- Around line 86-92: The language-picker button (onclick={() =>
showLanguageModal?.()}, displaying {$form.Language || m.project_languageCode()})
lacks a required-field affordance even though form submission is gated by
$form.Language.length; update the button to surface that it is required by
adding an accessible and visual cue: set aria-required="true" and add the same
required visual treatment/class used by the Name field (e.g., the required:true
styling or an explicit "required" CSS class and an asterisk in the label text),
and ensure any client-side validation still checks $form.Language before
enabling the submit button so behavior matches the new UI affordance.
In `@vite.config.ts`:
- Around line 7-14: Add a brief explanatory comment above the
optimizeDeps.exclude configuration that states why these packages are being
excluded (e.g., they are Svelte component/controller/state libraries that break
Vite's esbuild pre-bundling or require special handling), and mention the
specific packages listed ('@ethnolib/language-chooser-svelte-daisyui',
'@ethnolib/language-chooser-controller', '@ethnolib/find-language',
'@ethnolib/state-management-svelte') so future maintainers understand the
rationale for optimizeDeps.exclude.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5269a2ce-7adc-4a41-a981-9b60693eeb25
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
package.jsonsrc/routes/(authenticated)/projects/new/[id=idNumber]/+page.sveltevite.config.ts
Summary by CodeRabbit
New Features
Chores