Conversation
Move the configuration related to `Translations` that was previously stored in `Client` to a new `TranslationsConfig` object. This makes things more explicit and consistent with the other focussed configuration objects. BREAKING CHANGES: All elements that were in `Client` to manage the configuration of `Translations` have moved to `TranslationsConfig`: - `Client.defaultLanguage` to `TranslationsConfig.getDefaultLanguage` - `Client.setDefaultLanguage` to `TranslationsConfig.setDefaultLanguage` - `Client.language` to `TranslationsConfig.getLanguage` - `Client.setLanguage` to `TranslationsConfig.setLanguage` - `Client.languages` to `TranslationsConfig.getLanguages` - `Client.setLanguages` to `TranslationsConfig.setLanguages`
WalkthroughThis pull request reorganizes the internationalization configuration in maxGraph by relocating several properties and methods from the Client class to a dedicated TranslationsConfig module. The Client’s language-related functionalities are removed, and the version constant is moved to constants. Import paths for the Translations module have been revised across various files to reflect its new location. In addition, resource key initialization conditions have been updated to rely on TranslationsConfig.isEnabled(), and new tests and documentation entries have been added to support the changes and document migration steps. Changes
Sequence Diagram(s)sequenceDiagram
participant Comp as Client Component (e.g., Editor)
participant TC as TranslationsConfig
participant T as Translations Module
Comp->>TC: isEnabled()
alt Translations Enabled
Comp->>T: load language resources
else Translations Disabled
Comp->>Comp: Assign default/empty resource values
end
sequenceDiagram
participant Test as Test Suite
participant TC as TranslationsConfig
Test->>TC: Set custom language/config values
Test->>TC: resetTranslationsConfig()
Test->>Test: Verify configuration reverts to original values
Possibly related PRs
Suggested labels
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
packages/website/docs/usage/global-configuration.md (2)
21-21: LGTM! Fix the list indentation.The addition of
TranslationsConfigis well-documented. However, the indentation should be consistent with other list items.- - `TranslationsConfig` (since 0.16.0): for the configuration of `Translations`. +- `TranslationsConfig` (since 0.16.0): for the configuration of `Translations`.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
21-21: Unordered list indentation
Expected: 0; Actual: 2(MD007, ul-indent)
33-33: Fix the list indentation.The indentation should be consistent with other list items.
- - `resetTranslationsConfig` (since 0.16.0) +- `resetTranslationsConfig` (since 0.16.0)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
33-33: Unordered list indentation
Expected: 0; Actual: 2(MD007, ul-indent)
packages/core/src/view/Graph.ts (1)
386-388: Use ofTranslationsConfig.isEnabled()for resource resolution.This replacement ensures that translated resource keys appear only when translations are globally enabled. If an empty string is not desired as a fallback, consider displaying a user-friendly message or logging a warning.
packages/core/src/editor/Editor.ts (1)
314-320: Documentation updates for loading editor-specific resources.These lines clearly illustrate how to load built-in and Editor-specific translations. Ensure the snippet remains up to date with any further changes to the resource paths or configuration processes.
packages/core/src/i18n/config.ts (3)
24-38: Consider removing @ts-ignore by properly typing the original values.Instead of using @ts-ignore, consider initializing
originalValueswith proper typing:-// @ts-ignore the properties will be added dynamically when calling shallowCopy -const originalValues: TranslationsConfigValuesType = {}; +const originalValues: TranslationsConfigValuesType = { + defaultLanguage: '', + language: '', + languages: [] +};
94-112: Consider adding input validation for setLanguages.The languages setter should validate that the input array contains only valid language codes.
setLanguages(value: string[] | null | undefined): void { if (!isNullish(value)) { + // Validate that each language code follows ISO 639-1 format + const isValidLanguageCode = (code: string) => /^[a-z]{2}(-[A-Z]{2})?$/.test(code); + if (!value.every(isValidLanguageCode)) { + throw new Error('Invalid language code format. Expected ISO 639-1 format (e.g., "en", "en-US")'); + } values.languages = value; } }
114-129: Consider adding input validation for setDefaultLanguage.The default language setter should validate that the input is a valid language code.
setDefaultLanguage(value: string | undefined | null): void { + const newValue = !isNullish(value) ? value : 'en'; + // Validate that the language code follows ISO 639-1 format + if (!/^[a-z]{2}(-[A-Z]{2})?$/.test(newValue)) { + throw new Error('Invalid language code format. Expected ISO 639-1 format (e.g., "en", "en-US")'); + } - values.defaultLanguage = !isNullish(value) ? value : 'en'; + values.defaultLanguage = newValue; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
CHANGELOG.md(1 hunks)packages/core/__tests__/i18n/config.test.ts(1 hunks)packages/core/src/Client.ts(0 hunks)packages/core/src/editor/Editor.ts(5 hunks)packages/core/src/editor/EditorPopupMenu.ts(1 hunks)packages/core/src/gui/MaxForm.ts(1 hunks)packages/core/src/gui/MaxWindow.ts(1 hunks)packages/core/src/i18n/Translations.ts(5 hunks)packages/core/src/i18n/config.ts(1 hunks)packages/core/src/index.ts(1 hunks)packages/core/src/serialization/codecs/editor/EditorCodec.ts(1 hunks)packages/core/src/serialization/codecs/editor/EditorToolbarCodec.ts(1 hunks)packages/core/src/view/Graph.ts(2 hunks)packages/core/src/view/GraphSelectionModel.ts(2 hunks)packages/core/src/view/GraphView.ts(2 hunks)packages/core/src/view/geometry/node/StencilShape.ts(1 hunks)packages/core/src/view/handler/ElbowEdgeHandler.ts(2 hunks)packages/core/src/view/mixins/FoldingMixin.ts(2 hunks)packages/core/src/view/mixins/TooltipMixin.ts(1 hunks)packages/core/src/view/mixins/ValidationMixin.ts(1 hunks)packages/core/src/view/other/Multiplicity.ts(1 hunks)packages/html/.storybook/preview.ts(2 hunks)packages/ts-example-without-defaults/vite.config.js(1 hunks)packages/ts-example/vite.config.js(1 hunks)packages/website/docs/usage/global-configuration.md(2 hunks)packages/website/docs/usage/i18n.md(1 hunks)packages/website/docs/usage/migrate-from-mxgraph.md(2 hunks)
💤 Files with no reviewable changes (1)
- packages/core/src/Client.ts
✅ Files skipped from review due to trivial changes (11)
- packages/core/src/serialization/codecs/editor/EditorCodec.ts
- packages/ts-example-without-defaults/vite.config.js
- packages/ts-example/vite.config.js
- packages/core/src/gui/MaxForm.ts
- packages/core/src/view/mixins/ValidationMixin.ts
- packages/core/src/gui/MaxWindow.ts
- packages/core/src/editor/EditorPopupMenu.ts
- packages/core/src/view/geometry/node/StencilShape.ts
- packages/core/src/serialization/codecs/editor/EditorToolbarCodec.ts
- packages/core/src/view/mixins/TooltipMixin.ts
- packages/core/src/view/other/Multiplicity.ts
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
packages/website/docs/usage/global-configuration.md
21-21: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
33-33: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
🪛 LanguageTool
packages/website/docs/usage/migrate-from-mxgraph.md
[grammar] ~366-~366: Nouns are not usually modified by plural nouns. Is it possible that you meant to use the singular or possessive form here?
Context: ...y keep the property in Translations - languages properties are replaced by getters in `...
(PLURAL_MODIFIER)
CHANGELOG.md
[grammar] ~28-~28: This phrase is duplicated. You should probably use “to to” only once.
Context: ...nsConfig: - Client.defaultLanguagetoTranslationsConfig.getDefaultLanguage -Client.setDefaultLanguagetoTranslationsConfig.setDefaultLanguage -Client.languagetoTranslationsConfig.getLanguage -Client.setLanguagetoTranslationsConfig.setLanguage -C...
(PHRASE_REPETITION)
🔇 Additional comments (32)
packages/html/.storybook/preview.ts (1)
12-12: LGTM! The changes align with the PR objectives.The addition of
resetTranslationsConfigto the reset sequence ensures that translation configurations are properly reset between stories, maintaining consistency with other configuration resets.Also applies to: 38-38
packages/core/src/view/mixins/FoldingMixin.ts (1)
26-26: LGTM! The changes align with the PR objectives.The migration from
Client.languagetoTranslationsConfig.isEnabled()maintains the same functionality while aligning with the new centralized translations configuration approach.Also applies to: 66-66
packages/core/src/index.ts (1)
133-134: LGTM! The changes align with the PR objectives.The export path updates and new exports correctly reflect the reorganization of translation-related code into a dedicated i18n directory, making the new
TranslationsConfigavailable to consumers.packages/core/src/view/GraphView.ts (1)
48-48: LGTM! The changes align with the PR objectives.The migration from
Client.languagetoTranslationsConfig.isEnabled()for resource key initialization maintains the same functionality while aligning with the new centralized translations configuration approach.Also applies to: 124-124, 132-132
packages/core/__tests__/i18n/config.test.ts (2)
28-42: LGTM! Well-structured test implementation.The test follows good practices:
- Captures the original configuration state
- Modifies multiple configuration values
- Verifies that the reset restores the original state
1-15:⚠️ Potential issueUpdate the copyright year.
The copyright year is set to 2025, which is incorrect as it's beyond the current year.
-Copyright 2025-present The maxGraph project Contributors +Copyright 2024-present The maxGraph project ContributorsLikely an incorrect or invalid review comment.
packages/website/docs/usage/i18n.md (1)
21-27: LGTM! Clear resource file format documentation.The added documentation clearly illustrates the resource file format with a helpful example.
packages/core/src/view/GraphSelectionModel.ts (2)
26-26: LGTM! Clean import addition.The import of
TranslationsConfigis correctly placed with other imports.
81-81: LGTM! Improved initialization logic.The change from
Client.language !== 'none'toTranslationsConfig.isEnabled()makes the code more semantic and clearer in its intent.Also applies to: 89-89
packages/core/src/view/Graph.ts (2)
63-63: Solid import for translation config.Importing
TranslationsConfigaligns well with the new i18n approach for consistent configuration across the codebase.
396-398: Similar i18n resource initialization.Using
TranslationsConfig.isEnabled()keeps translation toggles consistent. The empty-string fallback is fine unless there's a requirement for default messaging.packages/core/src/view/handler/ElbowEdgeHandler.ts (2)
23-24: Updated imports to the new i18n modules.These updated paths and the additional
TranslationsConfigimport are consistent with the ongoing reorganization for improved i18n support.
60-62: Conditional resource assignment using i18n config.Good replacement from
Client.languagechecks toTranslationsConfig.isEnabled(). It ensures a central toggle controls the resource availability for the tooltip text.packages/core/src/editor/Editor.ts (9)
23-23: Refined import path for Translations module.Pointing to
../i18n/Translationspromotes better organization within the i18n directory structure.
59-59: Added import forTranslationsConfig.Adopting
TranslationsConfigfor Editor’s translation-related logic fosters consistency with other refactored modules.
459-459: askZoomResource now depends onTranslationsConfig.isEnabled().Switching to
TranslationsConfig.isEnabled()cleanly toggles the translation usage for the zoom dialog’s resource key.
469-469: Centralized toggle forlastSavedResource.This ensures the resource key is only active when translations are properly enabled. A good unified approach.
476-476: UpdatedcurrentFileResourcecondition.Same pattern: returning an empty string if translations are disabled. This maintains consistency with the other resource keys.
484-484: Conditionally assigningpropertiesResource.Relying on a single global toggle for translations helps avoid confusion when enabling or disabling i18n features.
492-492: Conditional check fortasksResource.Aligning the tasks window title resource with the rest of the i18n toggles ensures a cohesive user experience.
500-500: Conditional assignment forhelpResource.This is consistent with the new code style around i18n. Appears correct and well-scoped.
508-508: Outline window title i18n toggle.The empty-string fallback ensures no usage of the resource key if translations are disabled. Implementation looks good.
packages/core/src/i18n/config.ts (4)
1-19: LGTM!The copyright notice and imports are correctly implemented.
20-22: LGTM!The function correctly handles both browser and non-browser environments, with a proper fallback to 'en'.
40-42: LGTM!The reset function correctly restores the original configuration values.
47-92: LGTM!The language management methods are well-implemented with proper nullish checks and clear documentation.
packages/website/docs/usage/migrate-from-mxgraph.md (1)
366-367: LGTM!The documentation accurately reflects the relocation of language-related properties from
ClienttoTranslationsConfig.Also applies to: 380-381
🧰 Tools
🪛 LanguageTool
[grammar] ~366-~366: Nouns are not usually modified by plural nouns. Is it possible that you meant to use the singular or possessive form here?
Context: ...y keep the property inTranslations- languages properties are replaced by getters in `...(PLURAL_MODIFIER)
CHANGELOG.md (1)
27-33: LGTM!The changelog clearly documents the breaking changes related to the relocation of language-related properties from
ClienttoTranslationsConfig.🧰 Tools
🪛 LanguageTool
[grammar] ~28-~28: This phrase is duplicated. You should probably use “to to” only once.
Context: ...nsConfig: -Client.defaultLanguagetoTranslationsConfig.getDefaultLanguage-Client.setDefaultLanguagetoTranslationsConfig.setDefaultLanguage-Client.languagetoTranslationsConfig.getLanguage-Client.setLanguagetoTranslationsConfig.setLanguage-C...(PHRASE_REPETITION)
packages/core/src/i18n/Translations.ts (4)
20-23: LGTM!The imports are correctly updated to use the new module structure.
112-118: LGTM!The language support check is correctly updated to use TranslationsConfig.
148-165: LGTM!The special bundle logic is correctly updated to use TranslationsConfig for language settings.
190-190: LGTM!The language assignment is correctly updated to use TranslationsConfig with nullish coalescing.
|
Move the configuration related to
Translationsthat was previously stored inClientto a newTranslationsConfigobject.This makes things more explicit and consistent with the other focussed configuration objects.
BREAKING CHANGES: All elements that were in
Clientto manage the configuration ofTranslationshave moved toTranslationsConfig:Client.defaultLanguagetoTranslationsConfig.getDefaultLanguageClient.setDefaultLanguagetoTranslationsConfig.setDefaultLanguageClient.languagetoTranslationsConfig.getLanguageClient.setLanguagetoTranslationsConfig.setLanguageClient.languagestoTranslationsConfig.getLanguagesClient.setLanguagestoTranslationsConfig.setLanguagesNotes
Covers #688
Summary by CodeRabbit
New Features
Refactor
Documentation
TranslationsConfigandresetTranslationsConfigfunction.