Skip to content

Conversation

ivan-kalachikov
Copy link
Contributor

@ivan-kalachikov ivan-kalachikov commented Oct 1, 2025

@ivan-kalachikov ivan-kalachikov marked this pull request as ready for review October 1, 2025 13:15
@ivan-kalachikov ivan-kalachikov requested a review from a team as a code owner October 1, 2025 13:15
@ivan-kalachikov ivan-kalachikov requested review from Andrew-Orlov, goldenmaya and yuskithedeveloper and removed request for a team October 1, 2025 13:15
};
if (cultureName !== currentLanguage.value?.cultureName) {
pinLocale(cultureName);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pinLocale(cultureName) is called twice with the same value. Is there a specific reason for this duplication?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice spot! deleted the second call


const pinedLocale = useLocalStorage<string | null>("pinedLocale", null);
const pinnedLocale = useLocalStorage<string | null>("pinnedLocale", null);
const previousCultureSlug = useSessionStorage<{ cultureName: string; slug: string }>("previousCultureSlug", {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to move pinnedLocale and previousCultureSlug inside the function useLanguages?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to keep it in module scope, chat gpt agrees)
basically if we move it inside the function it will behave the same way, no changes

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements multilingual SEO URLs functionality by replacing the previous two-letter language code approach with full culture names (e.g., "en-US" instead of "en"). The implementation adds support for localized permalinks that update dynamically when switching languages.

Key changes include:

  • Migration from twoLetterLanguageName to cultureName throughout the language system
  • Addition of localized URL management with updateLocalizedUrl() functionality
  • Enhanced locale resolution with support for short aliases and culture disambiguation
  • Implementation of previousCultureSlug tracking for cross-language navigation

Reviewed Changes

Copilot reviewed 17 out of 19 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
client-app/core/composables/useLanguages.ts Core refactoring to support culture names and localized URL management
client-app/shared/layout/components/language-selector/language-selector.vue Updated to use culture names instead of two-letter codes
client-app/shared/common/composables/useSlugInfo.ts Added culture-aware slug resolution using previous culture tracking
client-app/pages/product.vue Integrated localized URL updates for product pages
client-app/pages/catalog.vue Minor refactoring of destructuring order
client-app/shared/catalog/components/category.vue Integrated localized URL updates for category pages
client-app/pages/brand.vue Integrated localized URL updates for brand pages
client-app/pages/matcher/virto-pages/virto-pages.vue Integrated localized URL updates for CMS pages
client-app/core/composables/useLanguages.test.ts Comprehensive test suite for new language functionality
client-app/core/utilities/common/index.ts Added safeDecode utility and removed unused getBaseUrl function
client-app/app-runner.ts Updated app initialization to use new locale resolution system
client-app/modules/utils.ts Updated method name from mergeLocales to mergeLocalesMessages
client-app/shared/account/composables/useUser.ts Renamed contact locale property to use culture name
client-app/shared/layout/components/header/_internal/mobile-menu/mobile-menu.vue Updated property reference for supported languages
client-app/core/constants/locale.ts Added pt-br country mapping
README.md Added comprehensive documentation for the new language flow
Comments suppressed due to low confidence (1)

client-app/core/composables/useLanguages.ts:1

  • The pinLocale function is called twice - once on line 79 and again on line 86. The second call is redundant and should be removed.
import { useLocalStorage, useSessionStorage } from "@vueuse/core";

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link

sonarqubecloud bot commented Oct 3, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants