-
Notifications
You must be signed in to change notification settings - Fork 33
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
i18n: Apply translation scheme. #100
Conversation
* Cleanup english translations * Remove non english ones.
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.
- Apply translation scheme to english translations
What does it mean ?
Thanks to refactor the keys, some questions though.
"results.sortBy.relevancy": "Relevancy", | ||
"search.field.any.placeholder": "Search datasets, services and maps ...", | ||
"search.field.sortBy": "Sort by", | ||
"search.loading": "Loading ..." |
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.
how did you change the order ? Did you use the npm script to extract the translations ?
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.
JSON sort plugin.
- Apply translation scheme to english translations
What does it mean ?
We discussed to apply a common pattern/scheme to translation keys - like in
https://github.com/geonetwork/geonetwork-ui/blob/master/apps/datafeeder/src/assets/i18n/en.json or https://github.com/geonetwork/geonetwork-microservices/blob/main/modules/services/ogc-api-records/service/src/main/resources/messages/api.properties
no?
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.
ok, I was confused by the term "schema"
@Component({ | ||
selector: 'search-sort-by', | ||
templateUrl: './sort-by.component.html', | ||
}) | ||
export class SortByComponent implements OnInit { | ||
choices = [ | ||
{ | ||
label: 'relevancy', | ||
label: 'results.sortBy.relevancy', |
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.
how those strings will be reconized to be extracte d?
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.
In this particular case, I don't think we can rely on the extractor script - at some point this will not be hardcoded and we may have a list or config for this ... but I can restore the marker and use the npm script if it makes more sense ...
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.
Well, I think it's better to keep the translation keys being extracted. It's a good practice to let the script update the file.
at some point this will not be hardcoded and we may have a list or config for this
What do you mean exactly ?
If we update en.json
manually to add foreign keys, it's the same effort to add them as markers in a dedicated file.
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.
Also if we use npm run i18n:extract
{
"search.field.any.placeholder": "Search datasets, services and maps ...",
"results.layout.selectOne": "Results layout",
"search.field.sortBy": "Sort by",
"records": "records",
"record.action.view": "viewable",
"record.action.download": "downloadable",
"record.more.details": "Read more",
"results.records.hits.found=0.help": "Suggestions: <ul class='list-disc list-inside'><li>Try other words</li><li>Specify fewer words</li></ul>",
"results.records.hits.found": "{hits, plural, =0{No documents match the specified search.} one{} other{{hits} records found.}}",
"search.loading": "Loading ...",
"results.sortBy.relevancy": "Relevancy",
"results.sortBy.dateStamp": "Last updates",
"results.sortBy.popularity": "Popularity",
"dropFile": ""
}
which looks to be incomplete. Should we then add markers for all facets stuffs ? Web component translation like nav.back
are also missing.
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.
Does it work well for the datafeeder app ?
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.
Does it work well for the datafeeder app ?
Not sure they are using, nobody read the manual ! :)
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.
Should we then add markers for all facets stuffs ?
IMO yes
Web component translation like
nav.back
are also missing.
Maybe it's just the path config in the package.json script
It helps to see some translations are missing, or some key are not well formatted, I keep thinking it's a good practice but I won't force it you find it painfull to maintain.
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.
Let's see if it works. Markers added. Format consistent with prettier. Web component path fixed.
Note that for the moment, it is always english and there is no mecanism to detect or change the langage. |
…h proper format. Sort for readability.
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.
Thanks for the changes, LG !
Next steps will be: