Feature/cxspa 9278 fix cds translations #19939
Conversation
spartacus
|
||||||||||||||||||||||||||||||
| Project |
spartacus
|
| Branch Review |
feature/CXSPA-9278-fix-cds-translations
|
| Run status |
|
| Run duration | 14m 32s |
| Commit |
|
| Committer | lasoh |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
4
|
Pending
|
2
|
Skipped
|
0
|
Passing
|
125
|
Upgrade your plan to view test results. | |
| View all changes introduced in this branch ↗︎ | |
| @@ -1,6 +1,5 @@ | |||
| { | |||
| "cdsTrendingSearches": { | |||
| "trendingSearches": "Popularne wyszukiwania", | |||
| "ariaTrendingSearches": "wpisywanie popularnych wyszukiwań" | |||
There was a problem hiding this comment.
It feels unnatural to split the keys trendingSearches and ariaTrendingSearches into different files (cds.json and common.json).
So to keep them together, I'd suggest to even delete the cds-related i18n files, and move all keys both the trendingSearches and ariaTrendingSearches to the common.json file. Same for recentSearches.
What do you think about it?
--
Side note: A general issue we had, even before this PR: we had unnecesarily two i18n files for CDS, each having only 1-2 translation keys. Instead we could have just one cdc.json file. Why: when customer configures i18n to be lazy loaded, we would lazy load 2 separate files cdsTrendingSearches.json and cdsRecentSearches.json with just 1-2 keys each, which is suboptimal.
There was a problem hiding this comment.
I think this is a good solution, but on the other hand if we need to add something in these modules (texts) the translation structure would already be ready. We wouldn't have to restore them back. That's why I would leave it as it is in PR for the future.
If we decided to moge recentSearches & trendigSearches translation object to common.json i would prefere add objects to searchBox f.e.
searchBox:{ recentSearches:{ header: 'Recent searches', ariaLabel: 'Aria sth' } }
@Platonn what do You think about ?
There was a problem hiding this comment.
Perhaps I'm missing something obvious.
I just thought that trendingSearches and ariaTrendingSearches are tightly coupled.
PS. In the future, if we want to leave any i18n JSON file related to cds, I would not use the current 2 ones (but rather drop them now), and use one file e.g. cds.json instead.
There was a problem hiding this comment.
I moved the translations that were in many files of the cds module to one file. (They are used only in this module). However, due to the structure of the SearchComponent in common.json I left the translations only for the headers.
Fix cds translation