Problem/Motivation
I spent two hours figuring out how to update translation strings for contributed modules. I followed these steps:
- Enabled Interface Translation module
- Went to admin/config/regional/translate
- Searched for the strings I wanted to translate using the filter -- no luck
- Paged through the list of strings -- no luck
- Found that the list of strings changed in length depending of when Interface Translation module -- a rabbit trail
- Google search on "drupal 8 t() string translation" -- lots of information but none pointing to admin/reports/translations
- Read https://www.drupal.org/docs/8/api/translation-api/overview a few times -- no luck
- Found a footnote about the only strings available in admin/config/regional/translate are core
- Found admin/reports/translations -- finally!
I know this is sad -- especially to those working with these functions daily -- but it is true.
Proposed resolution
Add a note to the top of admin/config/regional/translate. Something like: String translation for contributed modules can be found at admin/reports/translations. This small change will help the next person using this function for the first time.
Remaining tasks
Get it committed.
Comment | File | Size | Author |
---|---|---|---|
#64 | Afterpatch.png | 135.4 KB | DishaKatariya |
#64 | Beforepatch.png | 122.71 KB | DishaKatariya |
#61 | 2849929-61.patch | 2.15 KB | Rinku Jacob 13 |
#59 | AfterPatchSite.png | 444.21 KB | priyanka.sahni |
#58 | BeforePatchSite.png | 418.67 KB | priyanka.sahni |
Comments
Comment #2
cilefen CreditAttribution: cilefen commentedComment #4
Sutharsan CreditAttribution: Sutharsan at LimoenGroen commentedComment #5
chishah92 CreditAttribution: chishah92 at Valuebound commentedComment #6
chishah92 CreditAttribution: chishah92 at Valuebound commentedAdded the note in a patch.
Comment #7
ronaldtebrake CreditAttribution: ronaldtebrake as a volunteer and for Open Social commentedHi chishah92,
Thanks for taking this on, this still needs some work.
It's not the best idea in using the link as-is in the text. Because it does not allow you to change it in a later release without requiring translators to fix their work as well. Even for possibly static links, there might be changes, so it is best to remove the target from the URL.
You should however keep the link markup in the text, so that translators are aware of what is happening, and the link text is also there to translate in the sentence flow. This lets them move the link around, if the language at hand requires it, move out text from the link or vice versa.
See for example line 164 of locale.module:
You should be able to use the same idea, use a placeholder like
:update
, and make sure you let Drupal know render instead of the placeholder[':update' => \Drupal::url('locale.translate_status')]
For more background information please check:
https://www.drupal.org/docs/7/api/localization-api/dynamic-or-static-lin...
Comment #8
jeetendrakumar CreditAttribution: jeetendrakumar as a volunteer and at HyTechPro.com commentedComment #9
jeetendrakumar CreditAttribution: jeetendrakumar as a volunteer and at HyTechPro.com commentedDid changes as per suggestion in #7.
Comment #10
jeetendrakumar CreditAttribution: jeetendrakumar as a volunteer and at HyTechPro.com commentedComment #11
ronaldtebrake CreditAttribution: ronaldtebrake as a volunteer and for Open Social commentedThanks, just updated the patch with a little nitpick. There was a redundant space in the anchor.
Comment #12
ronaldtebrake CreditAttribution: ronaldtebrake as a volunteer and for Open Social commentedWoopsie, was a little too fast too furious on that one. Back to needs review :).
Comment #13
chishah92 CreditAttribution: chishah92 at Valuebound commentedComment #14
chishah92 CreditAttribution: chishah92 at Valuebound commentedHave fixed the redundant space issue.
Comment #15
Sutharsan CreditAttribution: Sutharsan at LimoenGroen commentedThis string does not explain what I can expect on the page it links to. "String translation" does not contain useful info. Considering the context of the page this message is on, I would mention the ability to mass import translations of contributed modules.
Comment #16
snehi CreditAttribution: snehi as a volunteer and at Publicis Sapient for Publicis Sapient commentedTried as per comment.
Comment #17
Sutharsan CreditAttribution: Sutharsan at LimoenGroen commentedThis line should not change in this issue. This issue is about adding extra help text. Extending the scope of an issue will slow the issue down. If it is a mistake, remove the change. If it is intentionally, remove the change and create a new issue.
I don't think that 'bulk' or 'mass' is relevant in this context. When I read the help text, it describes this page is for importing custom translations. The new sentence can connect to that message with the following:
"Standard translations for contributed modules can be imported at ..."
Page titles should start with a capital: "Available translation updates"
Comment #18
Sutharsan CreditAttribution: Sutharsan at LimoenGroen commentedComment #19
Sutharsan CreditAttribution: Sutharsan at LimoenGroen commentedI
It is not logical to add the new line at this position. The last sentence has not relation with the newly added. The new line should be placed last.
Comment #20
mohit1604 CreditAttribution: mohit1604 at Google Summer of Code commentedI am working on this !
Comment #21
mohit1604 CreditAttribution: mohit1604 at Google Summer of Code commentedThanks Sutharsan ! Did changes as per comment #17
Comment #22
Sutharsan CreditAttribution: Sutharsan at LimoenGroen commentedNote that you have added the extra string to a different page than the previous patches. On the positive side, you _have_ added it to the page mentioned in the issue summary. But now the text may no longer match the context. Lets rethink this ...
Comment #23
Sutharsan CreditAttribution: Sutharsan at LimoenGroen commentedI agree with the first committers that the Import page is a better place for the link, as importing is a more intuitive place. And further these is also a reference to the automatic import functionality there. I propose to append it to the existing string as:
Let's agree on the location and string first, before we make more patches.
Comment #24
ifrikGood idea to agree on the wording first.
You can leave out the "once enabled" because you already say that the translations are downloaded as part of the enabling. And you might just as well mention that the import can also happen automatically.
The Interface translation settings page (admin/config/regional/translate/settings) already has a wording for this.
Combining that with the proposed wording in #23, would be
Comment #25
Sutharsan CreditAttribution: Sutharsan at LimoenGroen commented@ifrik, super! I agree with your proposed text.
Comment #26
kiruba karan CreditAttribution: kiruba karan at Drupal Partners for Innoppl Technologies Pvt. Ltd commented@Sutharsan
I created the patch based on the proposed text given by @ifrik. Please check.
Comment #27
Sutharsan CreditAttribution: Sutharsan at LimoenGroen commentedThe text is as agreed. Patch is clean.
Comment #28
Sutharsan CreditAttribution: Sutharsan at LimoenGroen commentedBoth the summary and titel neefs to be updated, they are both out of date. Using the issue summary template is preferred. Updating will help the core maintainer to get the right information right away.
Comment #30
larowlanCan we get this array split onto multiple lines as per coding standards for arrays
also, still needs issue summary update.
thanks
Comment #31
shubhangi1995Comment #32
mohit1604 CreditAttribution: mohit1604 at Google Summer of Code commentedMade changes as per #30, please review it. Thank you :)
Comment #33
zymbian CreditAttribution: zymbian as a volunteer commentedPatch #32 looks good. Marking RTBC.
Comment #35
zymbian CreditAttribution: zymbian as a volunteer commentedComment #36
Sutharsan CreditAttribution: Sutharsan at LimoenGroen commentedIssue summary still needs update. See #28
Comment #37
mohit1604 CreditAttribution: mohit1604 at Google Summer of Code commentedUpdated issues summary and title.
Comment #38
mohit1604 CreditAttribution: mohit1604 at Google Summer of Code commentedComment #39
alexpottReading the final text and the issue summary seems that the original problem faced by the user is bot being addressed. From the issue summary.
The new text makes no mention of where and how to update the translated strings.
Comment #45
Abhijith S CreditAttribution: Abhijith S as a volunteer and at Zyxware Technologies commentedPatch #32 can't be applied on 9.2.x.Needs reroll.
Comment #46
Abhijith S CreditAttribution: Abhijith S as a volunteer and at Zyxware Technologies commentedRerolled patch #32 for 9.2.x.Please check.
Comment #47
kleiton_rodrigues CreditAttribution: kleiton_rodrigues at CI&T commentedI m work on Issue #2849929
Comment #48
kleiton_rodrigues CreditAttribution: kleiton_rodrigues at CI&T commentedLooks good!
+1 RTBC
Comment #49
alexpottUpdating issue credit.
I was wrong in #39 the new text says
and links to admin/reports/translations
Comment #50
alexpott@kleiton_rodrigues thank you for looking into this issue
Posting screenshots of your codebase or CLI does not advance the issue, since the automated testing infrastructure tells us whether the patch applies correctly.
Posting a screenshot of the new help text appearing is useful so I've decided to keep the credit box ticked but in future it'd be great to see an rtbc comment with more information. For example, detailing whether you've tested all the links and whether the fix actually fixes the problem outlined in the issue summary. For example, in #39 I reported that the fix didn't actually address the problem faced by the reporter, I was wrong, but it would have been good if the rtbc comment could have addressed this. See for information about how credit is awarded.
Comment #52
renatogIn this case below, "language" is the URL that's right?!
So please could you update the token name to ':language-url'?
Is better to be clear in the code due only "language" is generic.
E.g.:
From:
':language' => Url::fromRoute('entity.configurable_language.collection')->toString(),
To:
':language-url' => Url::fromRoute('entity.configurable_language.collection')->toString(),
And from:
<a title="Languages" href=":language">languages</a>
To:
<a title="Languages" href=":language-url">languages</a>
Comment #53
ayushmishra206 CreditAttribution: ayushmishra206 at OpenSense Labs commentedMade the suggested change in #52. Please review.
Comment #56
priyanka.sahni CreditAttribution: priyanka.sahni at Srijan | A Material+ Company for Drupal India Association commentedVerified and tested by applying the patch #53 on Drupal 9.2.x. Patch was successfully applied.
Steps to Replicate -
1. Go to the codebase.
2. Go to modules -> locale.
3. Go to locale.module -> search for case 'locale.translate_import': .
4. Verify the text updated.
Before Patch -
After Patch -
Comment #57
priyanka.sahni CreditAttribution: priyanka.sahni at Srijan | A Material+ Company for Drupal India Association commentedComment #58
priyanka.sahni CreditAttribution: priyanka.sahni at Srijan | A Material+ Company for Drupal India Association commentedComment #59
priyanka.sahni CreditAttribution: priyanka.sahni at Srijan | A Material+ Company for Drupal India Association commentedComment #61
Rinku Jacob 13 CreditAttribution: Rinku Jacob 13 at Srijan | A Material+ Company for Drupal India Association commentedRe-rolled patch #53 for 9.5.x.
Comment #62
VinmayiSwamy CreditAttribution: VinmayiSwamy as a volunteer and at Srijan | A Material+ Company for Drupal India Association commentedVerified re-rolled patch #61 for 9.5.x. Failed to apply.
Comment #63
DishaKatariya CreditAttribution: DishaKatariya at QED42 for Drupal India Association commentedComment #64
DishaKatariya CreditAttribution: DishaKatariya at QED42 for Drupal India Association commentedVerified and tested #61 Patch applied successfully on Drupal 9.5.x-dev and looks good to me.
Testing Steps:
1. Go to the codebase.
2. Go to modules -> locale.
3. Go to locale.module -> search for case 'locale.translate_import': .
4. Verify the text updated.
Testing Results-
The Interface translation settings page (http://localhost/drupal-9.5.x/admin/config/regional/translate/import) the text is updated
Please refer attached screenshots.
Can be a move to RTBC.
Comment #65
xjmThanks @priyanka.sahni and @DishaKatariya for your manual testing. Note that it is never necessary or appropriate to attach screenshots of the code in your IDE, nor of your terminal window. This was also explained in #50. Please read the previous comments on the issue before posting your own.
I also recommend reviewing the contributor task instructions for creating a screenshot, particularly the following points.
Since the screenshots provided do illustrate the string change, I will grant issue credit for them, but please make cropped screenshots in small browser windows in the future to focus on the part of the UI with the change.
I don't think this string should say "automatic updates". That's an upcoming module that will automatically update the core codebase. It's also getting to be a lot of words, so we should focus on removing unnecessary words from the text so that there's not a UX regression. I would suggest:
We should probably also reduce the text in the following paragraphs to reduce the risk that people will skip over it because it's a wall of text. There's nearly an entire sentence we can eliminate, and we can also make it easier to scan with an ordered list:
And then in the last paragraph there's not much we can improve, but we can at least get rid of the words "note that":
I apologize if this feels like scope creep, but the overall amount of text on a page impacts usability, so reducing the overall text is one way to help this through the usability gate.
And please, crop your screenshots and ONLY post screenshots of the Drupal site user interface.
Thanks!