TLN Add missing js translation strings to `src/en.json` file

pschiffe commented 3 months ago

Description

I went through the source code in the client/src dir and addded all missing translation strings to the client/lang/src/en.json file.

Manual testing steps

Issues

1689

Pull request checklist

[x] The target branch is correct
- See picking the right version
[x] All commits are relevant to the purpose of the PR (e.g. no debug statements, unrelated refactoring, or arbitrary linting)
- Small amounts of additional linting are usually okay, but if it makes it hard to concentrate on the relevant changes, ask for the unrelated changes to be reverted, and submitted as a separate PR.
[x] The commit messages follow our commit message guidelines
[x] The PR follows our contribution guidelines
[x] Code changes follow our coding conventions
[x] This change is covered with tests (or tests aren't necessary for this change)
[x] Any relevant User Help/Developer documentation is updated; for impactful changes, information is added to the changelog for the intended release
[x] CI is green

pschiffe commented 3 months ago

I have one more question. I also found the following string IDs - where do they belong?

Boolean.ANY
CampaignAdmin.DELETECAMPAIGN
CampaignAdmin.LOADING
CampaignAdmin.NO_RECORDS
PopoverOptionSet.CLEAR
PopoverOptionSet.NO_RESULTS
PopoverOptionSet.SEARCH_PLACEHOLDER
PopoverOptionSetToggle.TOGGLE

and

SilverStripe\\Admin\\FileStatusIcon.TRACKED_FORM_UPLOAD_RESTRICTED
SilverStripe\\Admin\\FileStatusIcon.TRACKED_FORM_UPLOAD_UNRESTRICTED
SilverStripe\\Admin\\FileStatusIcon.ACCESS_RESTRICTED

pschiffe commented 3 months ago

Thanks for taking a look and sorry for my lousiness :see_no_evil:

All the string IDs were found in the client/src folder. Is it OK to have cross module translation dependencies like.. will it work?

Also, if the string ID is SilverStripe\\Admin\\FileStatusIcon.ACCESS_RESTRICTED in the javascript file, will it be read from the translation yaml, or should it be also added to the client/lang/src/en.json file?

pschiffe commented 3 months ago

Hm, this file probably also requires to rebuild the bundle. Please help me to sort out the missing string IDs and I'll update the PR :pray:

 git diff found modified files when it should not have:
M js/bundle.js
sha1sum of files that are different:
a790abc90e872e05680c06101c0adfc34df9c5fa  client/dist/js/bundle.js

GuySartorelli commented 3 months ago

All the string IDs were found in the client/src folder.

If they're not already declared by other modules, then they will need to be added to the json file.

The CampaignAdmin ones almost certainly are specific to the campaign admin module for example... it's suspicious that they're here.

Maybe open a new separate issue about the ones you're not sure about, and link to where each one is in the code, so we can discuss where they belong without delaying merging this PR which is good to go as-is.

Is it OK to have cross module translation dependencies like.. will it work?

If the module is installed, it will pull the translations from that module. If the module is not installed, it will fall back to the default string which is declared in the js.

In short: yes ;p

Also, if the string ID is SilverStripe\\Admin\\FileStatusIcon.ACCESS_RESTRICTED in the javascript file, will it be read from the translation yaml, or should it be also added to the client/lang/src/en.json file?

The yaml file for PHP is entirely separate. Unfortunately.

GuySartorelli commented 3 months ago

The JS build failure here is unrelated to the changes in this PR.

pschiffe commented 3 months ago

@GuySartorelli I've updated the PR and added all missing string IDs except the CampaignAdmin ones, which should be in the campaign admin module I think. I didn't find the others, the Boolean and PopoverOptionSet anywhere else, so I think they are OK here.

I'm happy with this PR now.

silverstripe / silverstripe-admin