-
-
Notifications
You must be signed in to change notification settings - Fork 366
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
fix: hide other language #11455
fix: hide other language #11455
Conversation
WalkthroughThis pull request removes the dynamic language selection functionality throughout the application. The changes include the removal of language selection components and related event handlers in the navigation, wallet asset menu, and profile dropdown. The dedicated language management store has been deleted, and the locale-setting method in the preferences store now forces a fixed English locale. Additionally, end-to-end tests for language switching have been removed. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as Component
participant P as Preferences Store
participant I as $i18n
U->>C: Initiate locale change (previously via language UI)
C->>P: Call setUserLocale(userLang)
P->>P: Override userLang to "en"
P->>I: Set $i18n.locale.value to "en"
P->>P: Update state.userLocale to "en"
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (28)
💤 Files with no reviewable changes (24)
✅ Files skipped from review due to trivial changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
✅ Deploy Preview for koda-canary ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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 (3)
stores/preferences.ts (1)
46-72
: Consider removing unused language definitionsThe
langsFlags
array is still present even though language selection has been disabled. Since the application now only supports English, this array is no longer being used for its original purpose.Consider removing or deprecating this array to avoid confusion and maintain code cleanliness:
-export const langsFlags = [ - { - value: 'en', - flag: '🇬🇧', - label: 'English', - }, - { - value: 'de', - flag: '🇩🇪', - label: 'Deutsch', - }, - { - value: 'es', - flag: '🇪🇸', - label: 'Español', - }, - { - value: 'fr', - flag: '🇫🇷', - label: 'Français', - }, - { - value: 'hi', - flag: '🇮🇳', - label: 'हिंदी', - }, -]If there's a future plan to re-enable multi-language support, you might want to add a TODO comment instead.
components/common/ConnectWallet/WalletAssetMenu.vue (2)
96-98
: Remove unused language selector CSSThe CSS for the language selector still exists even though the component has been removed from the template.
Remove the unused CSS for better code maintainability:
.wallet-asset-footer { border-top: 1px solid grey; justify-content: space-between; @apply bulma-mobile:justify-center; - .language-selector { - @apply bulma-mobile:my-8 bulma-mobile:mx-0; - } & > * { @apply cursor-pointer flex gap-1; &:hover { @apply text-text-color; } }
109-111
: Remove unnecessary dropdown stylingThe deep selector style for dropdown menu is likely related to the removed language selector and may no longer be needed.
Consider removing this CSS rule if it was specifically for positioning the language dropdown:
- /* manually center dropdown menu, because no props "position" to center it */ - :deep(.o-drop__menu) { - @apply bulma-tablet:translate-x-[50px] - }Verify this doesn't affect other dropdowns before removing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
components/Navbar.vue
(0 hunks)components/common/ConnectWallet/WalletAssetMenu.vue
(1 hunks)components/navbar/ProfileDropdown.vue
(1 hunks)stores/lang.ts
(0 hunks)stores/preferences.ts
(1 hunks)tests/e2e/language.spec.ts
(0 hunks)
💤 Files with no reviewable changes (3)
- components/Navbar.vue
- tests/e2e/language.spec.ts
- stores/lang.ts
⏰ Context from checks skipped due to timeout of 90000ms (16)
- GitHub Check: Redirect rules
- GitHub Check: Header rules
- GitHub Check: Pages changed
- GitHub Check: shard (10, 10)
- GitHub Check: shard (3, 10)
- GitHub Check: build-cf
- GitHub Check: runner / eslint
- GitHub Check: shard (9, 10)
- GitHub Check: shard (6, 10)
- GitHub Check: shard (8, 10)
- GitHub Check: shard (4, 10)
- GitHub Check: shard (7, 10)
- GitHub Check: shard (5, 10)
- GitHub Check: shard (1, 10)
- GitHub Check: build
- GitHub Check: shard (2, 10)
🔇 Additional comments (4)
stores/preferences.ts (1)
271-272
: Language selection locked to EnglishThe modifications to the
setUserLocale
method now force the locale to 'en' (English) regardless of what the user selects, effectively disabling multi-language support across the application.This change aligns with the PR objective to "fix: hide other language" and ensures consistency throughout the codebase by removing the language selection feature.
components/common/ConnectWallet/WalletAssetMenu.vue (1)
43-43
: Import cleanup for removed language selectorThe import statement has been updated to only include
NeoIcon
, removing unused imports for the removed language selection dropdown.components/navbar/ProfileDropdown.vue (2)
55-55
: Import cleanup for removed language iconThe import statement has been updated to remove
NeoIcon
since it was previously used for the language selector that has now been removed.
1-80
:❓ Verification inconclusive
Verify user profile dropdown functionality
The language selector functionality has been removed from the profile dropdown. Ensure that all remaining dropdown functionality (color mode switcher and cookies settings) continues to work as expected.
🏁 Script executed:
#!/bin/bash # Description: Check if there are any remaining language selection related code in other components # Look for any remaining language selection related code echo "Checking for any remaining language selection UI components:" rg -i "language(.*)selector" --type vue # Check for any remaining language icon references echo "Checking for any remaining language icon references:" rg -i "globe(.*)icon" --type vue # Check for any remaining language selection related methods echo "Checking for any remaining language toggle methods:" rg -i "toggle(.*)language" --type vue echo "Checking for language selection in navbar components:" rg -i "language" components/navbar/Length of output: 712
Below is a new shell script to re-run the verification using a file glob instead of the unrecognized Vue file type. This should help confirm that no language selection-related references remain in the code:
#!/bin/bash echo "Checking for any remaining language selection UI components in .vue files:" rg -i "language(.*)selector" -g "*.vue" echo "Checking for any remaining language icon references in .vue files:" rg -i "globe(.*)icon" -g "*.vue" echo "Checking for any remaining language selection related methods in .vue files:" rg -i "toggle(.*)language" -g "*.vue" echo "Checking for language selection within navbar components in .vue files:" rg -i "language" -g "*.vue" components/navbar/Dropdown Functionality Verification: Language Selection Removal Confirmed
- Confirm that no language selector code remains in any Vue component.
- Verify manually that the remaining dropdown items—the Color Mode switcher and the Cookies settings button—are functioning as expected.
shouldn't we also remove the other translation files ? |
It's doable to delete the other translation files, but we may need them in the future. wdyt |
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/preferences.ts (2)
269-275
: Use more descriptive unused parameter name and consider refactoring for clarity.The parameter has been renamed from
locale: string
to_: string
to indicate it's unused, which is a good practice. However, a more descriptive name like_locale: string
would better preserve the semantic meaning while still indicating it's unused.Consider applying these changes for improved clarity:
- setUserLocale(_: string) { + setUserLocale(_locale: string) { const { $i18n } = useNuxtApp() // only supports en for now const locale = 'en' $i18n.locale.value = locale this.userLocale = locale },Additionally, since this appears to be a temporary solution based on the comment "only supports en for now", it might be helpful to add a more explicit TODO comment if there are plans to restore multi-language support in the future.
46-72
: Consider removing unused language configuration.The
langsFlags
array is still present in the code, even though the language selection functionality has been removed from the application as indicated in the PR summary. This could be confusing for future developers and might lead to inconsistent behavior if language selection is partially restored.If other languages are no longer supported, consider removing this array entirely or adding a clear comment explaining why it's being preserved:
-export const langsFlags = [ - { - value: 'en', - flag: '🇬🇧', - label: 'English', - }, - { - value: 'de', - flag: '🇩🇪', - label: 'Deutsch', - }, - { - value: 'es', - flag: '🇪🇸', - label: 'Español', - }, - { - value: 'fr', - flag: '🇫🇷', - label: 'Français', - }, - { - value: 'hi', - flag: '🇮🇳', - label: 'हिंदी', - }, -]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
stores/preferences.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (15)
- GitHub Check: Redirect rules
- GitHub Check: Header rules
- GitHub Check: Pages changed
- GitHub Check: shard (6, 10)
- GitHub Check: shard (8, 10)
- GitHub Check: shard (7, 10)
- GitHub Check: shard (10, 10)
- GitHub Check: shard (9, 10)
- GitHub Check: build-cf
- GitHub Check: shard (4, 10)
- GitHub Check: build
- GitHub Check: shard (5, 10)
- GitHub Check: shard (1, 10)
- GitHub Check: shard (2, 10)
- GitHub Check: shard (3, 10)
🔇 Additional comments (2)
stores/preferences.ts (2)
273-274
: Implementation matches the suggested fix from the previous review.The implementation of setting both
$i18n.locale.value
andthis.userLocale
to the same value matches the suggestion from Sonar mentioned in the previous review.
272-274
:❓ Verification inconclusive
Verify all translation-related files have been properly addressed.
As mentioned in the PR comments, there was a question about "removal of other translation files." This change only affects the runtime behavior by forcing the English locale, but doesn't address the existence of other translation files in the repository that might no longer be used.
🏁 Script executed:
#!/bin/bash # Description: Look for potentially unused translation files in the repository # Find locale/translation files with two-letter language codes echo "Searching for translation files..." fd -e json 'en\.json|de\.json|fr\.json|es\.json|hi\.json' --type f # Look for imports of i18n or translation-related code echo "Checking for i18n imports across the codebase..." rg 'import.*i18n' -l # Look for uses of translation functions in the codebase echo "Checking for translation function usage..." rg '\$t\(' -lLength of output: 12500
Translation Files Status Review
The changes in
stores/preferences.ts
now force the runtime locale to English, but our investigation confirms that multiple translation files (e.g.,i18n/locales/en.json
,i18n/locales/de.json
,i18n/locales/es.json
,i18n/locales/fr.json
,i18n/locales/hi.json
) are still present and that translation function calls (e.g.,$t(
) remain in widespread use across many components.
- The forced locale change affects runtime behavior only.
- Translation files for various languages are still part of the repository.
- Numerous components continue to rely on translation functions.
Action: Please confirm whether the retention of these translation files is intentional or if a cleanup/removal process should be initiated in a subsequent change.
I'd delete them we can always go back to this pr and recover them, @preschian wdyt |
Oh, this one is okay for me. We can revert it once we need to in the future |
|
Updated |
Thank you for your contribution to the Koda - Generative Art Marketplace.
👇 __ Let's make a quick check before the contribution.
PR Type
Needs Design check
Needs QA check
Context
Screenshot 📸
Summary by CodeRabbit