-
Notifications
You must be signed in to change notification settings - Fork 1
feat(language): language switcher #172
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: release/v0.2
Are you sure you want to change the base?
feat(language): language switcher #172
Conversation
WalkthroughA language switcher feature was introduced by adding new Vue components, a JSON asset mapping language codes to flag emojis, and updating the footer layout to include the switcher. Additional changes include improving route path handling to ensure a non-empty value and explicitly setting the document charset in the Nuxt configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LanguageSwitcher
participant LanguageSwitcherInner
participant UiSelect
participant i18nUtils
participant SessionContext
User->>LanguageSwitcher: Render component
LanguageSwitcher->>LanguageSwitcherInner: Render inner component
LanguageSwitcherInner->>i18nUtils: Fetch available languages (onMounted)
i18nUtils-->>LanguageSwitcherInner: Return list of languages
LanguageSwitcherInner->>SessionContext: Get current language
User->>UiSelect: Open dropdown and select language
UiSelect->>LanguageSwitcherInner: onUpdate(selectedLang)
LanguageSwitcherInner->>i18nUtils: changeLanguage(selectedLang)
i18nUtils-->>LanguageSwitcherInner: Return redirect URL
LanguageSwitcherInner->>User: Redirect browser to new URL
sequenceDiagram
participant LayoutFooterInner
participant LanguageSwitcher
LayoutFooterInner->>LanguageSwitcher: Render in "language-switcher" slot
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
components/LanguageSwitcherInner.vue (1)
7-8
: Top-levelawait
may degrade TTI on CSR & block SSR renderingThe component does a blocking
await getAvailableLanguages()
before the template is evaluated.
If the request is slow, both CSR initial render and SSR streaming (Nuxt) are delayed.
Consider moving the call to an asynconMounted
/useAsyncData
(Nuxt) hook or usingPromise.all
withoutawait
to let the component render a skeleton immediately.components/LanguageSwitcher.vue (1)
1-3
: Component is a no-logic passthrough – consider inlining
LanguageSwitcher.vue
only re-exportsLanguageSwitcherInner
without adding props, provide/inject, or layout.
Eliminating the extra component avoids one level in the vnode tree:-<template> - <LanguageSwitcherInner /> -</template> +<!-- Remove this file and use <LanguageSwitcherInner /> directly -->Nice-to-have, but keeps the tree flat.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
components/LanguageSwitcher.vue
(1 hunks)components/LanguageSwitcherInner.vue
(1 hunks)components/layout/footer/LayoutFooterInner.vue
(1 hunks)pages/[...all].vue
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: review-app
🔇 Additional comments (3)
components/LanguageSwitcherInner.vue (1)
28-32
: Pass the primitive, not the ref, todefault-value
UiSelect
most likely expects a primitive.
Passing the ref itself works only if the component unwraps refs (Vue will not do that automatically for props).
IfUiSelect
is not set up for ref-based props, use.value
:-<UiSelect :default-value="selectedLanguage" @update:model-value="onUpdate"> +<UiSelect :default-value="selectedLanguage.value" @update:model-value="onUpdate">pages/[...all].vue (1)
18-20
: LGTM – fallback ‘/’ guards against empty pathThe additional
|| '/'
correctly prevents downstream 404s when the stripped path is empty.
No further issues spotted.components/layout/footer/LayoutFooterInner.vue (1)
52-56
:LanguageSwitcher
not imported – relies on global/auto-import
LanguageSwitcher
is referenced in the template but isn’t imported in the<script setup>
.
If the project stops auto-importing components in the future, this silently breaks SSR.
Add an explicit import (tree-shakable):<script setup lang="ts"> import type { Schemas } from '@shopware/api-client/api-types'; +import LanguageSwitcher from '~/components/LanguageSwitcher.vue';
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
stores/LocaleStore.ts (2)
4-6
: Simplify the wrapper and propagate the promise.
loadAvailableLanguages
does nothing beyond awaiting another async function.
Dropping theasync/await
removes an unnecessary micro-task and lets callers await the underlying promise (and catch errors) directly.- const loadAvailableLanguages = async () => { - await getAvailableLanguages(); - }; + const loadAvailableLanguages = () => getAvailableLanguages();
8-10
: Consider whether a Pinia store is needed here.This store has no state or getters—it only exposes a pass-through action to a composable.
Unless future state is planned, components could calluseInternationalization().getAvailableLanguages()
directly and avoid an extra abstraction layer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/LanguageSwitcherInner.vue
(1 hunks)stores/LocaleStore.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/LanguageSwitcherInner.vue
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: review-app
🔇 Additional comments (1)
stores/LocaleStore.ts (1)
1-3
: Verify thatdefineStore
anduseInternationalization
are auto-imported.There are no explicit imports for
defineStore
oruseInternationalization
.
If your build relies on an Auto-Import plugin (e.g. unplugin-auto-import) this is fine; otherwise the file will not compile.
Summary by CodeRabbit