Skip to content

Conversation

@timowolf
Copy link
Member

@timowolf timowolf commented Nov 25, 2025

…t translate pipe

Element components shall not depend on concrete translation library. Instead, they shall use the Element translate facade.

Describe in detail what your merge request does and why. Add relevant
screenshots and reference related issues via Closes #XY or Related to #XY.


Summary by CodeRabbit

  • Chores

    • Updated translation implementation and type references to a new translation package/namespace; no behavioral or public API changes.
  • No user-facing changes

    • This release does not change functionality or visible behavior.

✏️ Tip: You can customize this high-level summary in your review settings.

…t translate pipe

Element components shall not depend on concrete translation
library. Instead, they shall use the Element translate facade.
@timowolf timowolf added the bug Something isn't working label Nov 25, 2025
@coderabbitai
Copy link

coderabbitai bot commented Nov 25, 2025

Walkthrough

Replaced a component's translation pipe import from TranslatePipe (ngx-translate) to SiTranslatePipe (@siemens/element-translate-ng) and updated multiple API golden files to switch TranslatableString type references from the translate-types namespace to the root @siemens/element-translate-ng namespace. No runtime behavior changes.

Changes

Cohort / File(s) Summary
Component — translation pipe swap
projects/element-ng/application-header/si-account-details.component.ts
Replaced import TranslatePipe (from @ngx-translate/core) with SiTranslatePipe (from @siemens/element-translate-ng) and updated component imports array accordingly.
API goldens — application-header types
api-goldens/element-ng/application-header/index.api.md
Replaced references of _siemens_element_translate_ng_translate_types.TranslatableString with _siemens_element_translate_ng.TranslatableString across multiple public component property signatures (SiApplicationHeaderComponent, SiHeaderCollapsibleActionsComponent, SiLaunchpadFactoryComponent).
API goldens — navbar types
api-goldens/element-ng/navbar/index.api.md
Replaced references of _siemens_element_translate_ng_translate_types.TranslatableString with _siemens_element_translate_ng.TranslatableString across multiple public input/property signatures of SiNavbarPrimaryComponent.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Review the component import change for correct module/provider usage and compatibility with existing templates.
  • Verify API golden updates correctly reflect the new namespace and no unintended type regressions were introduced.
  • Ensure package/module resolution for @siemens/element-translate-ng is present in the project and build passes.

Suggested reviewers

  • spike-rabbit

Poem

🐰 I hopped through code with gentle paws,
Swapped pipes and namespace without a cause,
Translations snug in a new home today,
The header hums — I’m gone to play! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(application-header): replace ngx-translate dependency with element...' is directly related to the main change in the changeset, which replaces TranslatePipe from ngx-translate/core with SiTranslatePipe from element-translate-ng.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/remove-ngx-translate-dependency

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 489ff2a and ab644fa.

📒 Files selected for processing (2)
  • api-goldens/element-ng/application-header/index.api.md (6 hunks)
  • api-goldens/element-ng/navbar/index.api.md (3 hunks)
🔇 Additional comments (2)
api-goldens/element-ng/application-header/index.api.md (1)

18-18: Excellent! All verification checks have been completed. The review comment's claims are fully supported by the evidence.


TranslatableString namespace migration verified—all components successfully migrated to Element translate facade with complete backward compatibility.

The namespace updates are confirmed consistent across all affected components (SiApplicationHeaderComponent, SiHeaderCollapsibleActionsComponent, and SiLaunchpadFactoryComponent). All input signals accepting translatable strings now correctly reference _siemens_element_translate_ng.TranslatableString instead of the old namespace variant.

Verification Results:

  1. ✅ All component source files use SiTranslatePipe from @siemens/element-translate-ng instead of ngx-translate's TranslatePipe
  2. ✅ No ngx-translate imports remain in the application-header module
  3. ✅ All API golden file lines (18, 97, 111, 162, 238, 242, 255-258) consistently reference the new _siemens_element_translate_ng.TranslatableString namespace
  4. ✅ Public API surface maintains backward-compatibility—TranslatableString is available to consumers through component type signatures
api-goldens/element-ng/navbar/index.api.md (1)

21-21: Perfect! I've now completed the verification of the review comment. Let me summarize my findings:

Verification Summary:

  1. Source component updated: si-navbar-primary.component.ts correctly imports and uses SiTranslatePipe from @siemens/element-translate-ng/translate (line 40) instead of ngx-translate.

  2. Golden file reflects migration: The api-goldens/element-ng/navbar/index.api.md file shows the correct namespace migration:

    • Line 21: import * as _siemens_element_translate_ng from '@siemens/element-translate-ng'; (new)
    • Type signatures use _siemens_element_translate_ng.TranslatableString (new namespace)
    • The old _siemens_element_translate_ng_translate_types namespace is absent
  3. API Extractor properly configured: The tooling (tools/api-goldens/) is set up to:

    • Run API Extractor on built component artifacts (dist/@siemens/element-ng)
    • Auto-generate .api.md golden files from source components
    • Compare or accept changes via npm scripts (api-goldens:test, api-goldens:accept, api-goldens:build-accept)

Golden files are correctly auto-generated from updated source components and properly reflect the ngx-translate removal in favor of the Element translate facade.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@timowolf timowolf added this to the 48.x milestone Nov 25, 2025
@timowolf
Copy link
Member Author

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Nov 25, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@github-actions
Copy link

@github-actions
Copy link

Code Coverage

@timowolf timowolf marked this pull request as ready for review November 25, 2025 21:18
@timowolf timowolf requested a review from a team as a code owner November 25, 2025 21:18
@timowolf
Copy link
Member Author

@spike-rabbit Maybe you can give me a hint why golden APIs had to be rebuild

@chintankavathia
Copy link
Member

Once this is merged. We should merge this one #1086 to avoid such thing in future.

Copy link
Member

@fh1ch fh1ch left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@timowolf timowolf merged commit d3b7316 into main Nov 26, 2025
12 checks passed
@timowolf timowolf deleted the fix/remove-ngx-translate-dependency branch November 26, 2025 05:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working released

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants