Problem/Motivation

The "Result summary" view area plugin has a non-translatable description.

screenshot of the non-translatable description

Steps to reproduce

  1. Switch drupal to non-English language
  2. Go to the Content view for example (it can be any view)
  3. Add a "Result summary" into header
  4. You will see that the description is only partly transltated

Issue fork drupal-3476898

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

chizh273 created an issue. See original summary.

chizh273 changed the visibility of the branch 3476898-the-result-summary to hidden.

chizh273 changed the visibility of the branch 3476898-the-result-summary to active.

chizh273’s picture

Assigned: chizh273 » Unassigned
Status: Active » Needs review
mero.s’s picture

Status: Needs review » Reviewed & tested by the community

Looks good for me.

kleiton_rodrigues’s picture

StatusFileSize
new23.94 KB

LGFM ++
3476898

'@start -- ' . $this->t('the initial record number in the set'),
        '@end -- ' . $this->t('the last record number in the set'),
        '@total -- ' . $this->t('the total records in the set'),
        '@label -- ' . $this->t('the human-readable name of the view'),
        '@per_page -- ' . $this->t('the number of items per page'),
        '@current_page -- ' . $this->t('the current page number'),
        '@current_record_count -- ' . $this->t('the current page record count'),
        '@page_count -- ' . $this->t('the total page count'),
quietone’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs manual testing

Even though this is straightforward, there should be manual testing to prove this works as expected. Adding tag.

brandonlira’s picture

anapaulagoetze’s picture

StatusFileSize
new187.88 KB

Hi,
I've been working on the manual test to validate that this feature works as expected, and I believe everything is functioning correctly.
Please check the evidence attached below for confirmation.

1. Prepare Content
1.1 Go to Content.
1.2Create a new Article to be displayed on the View.

2. Add and Set New Language
2.1Go to /admin/config/regional/language.
2.1.1 If this page does not exist, install the “Translation” module.
2.2 Click “+ Add language”.
2.3 After adding, set that language as the default and Save.

3. Edit the View
3.1Go to Structure > Views.
3.2 Find and edit the view displaying the content.
3.3 In the Header section:
3.4 Click Add > select Result summary.
3.5 Click Configure next to it.

4. Check Translation
4.1 Confirm if the description and tag text are already translated.
4.2 If they are not translated automatically:

5. Add Custom Translation
5.1 Go back to /admin/config/regional/language.
5.2 Click on the Translate percentage number.
5.2.1 You will be redirected to a page like (/admin/config/regional/translate?langcode=pt-br.)
5.3 Search for the exact phrase/tag text used in the result summary.
5.4 Add the correct translation for the selected tag and Save.

6. Recheck the View
6.1 Return to the View edit page.
6.2 Click on the Result summary element (the one you added to the header).
6.3 Confirm that the tag text now appears correctly translated.

Evidence

xjm’s picture

Issue summary: View changes
StatusFileSize
new78.29 KB

Thanks @anapaulagoetze! Excellent writeup.

I followed the same testing steps with French, and I noticed that different variations of the same strings already exist. For example:
Translating 'the number of items per page'

The current string has:
the number of items per page

But core already has this existing string:
The number of items per page.

If we update the merge request to match this (capitalize it and add a period), then the existing string translation will just start working here, without translators needing to do additional work. This would also better match our content standards. The same is also true for at least the total page count if we change it to The total page count.

So, I think we should also update the merge request to capitalize all the strings and add periods to the end of them. Thanks!

brandonlira’s picture

Great, I'll take a look at that. Thanks @xjm for the guidance.

brandonlira’s picture

Hi @xjm, I’ve updated the MR !9611 with capitalised strings and periods as suggested.
It’s ready for review!
Let me know if anything else is needed.
Thanks!

brandonlira’s picture

Status: Needs work » Needs review
himanshu raj’s picture

StatusFileSize
new93.37 KB
new19.05 KB

Thanks @anapaulagoetze

Hi, I ran the same test on a clean Drupal 11.0 site with English installed and French set as the default language . After enabling Interface Translation and adding Result summary to the view header, the strings showed up in French automatically no manual translations required.
I also scanned the string list and found only the capitalised version (“The number of items per page.”), with no lowercase duplicate. Everything looks consistent and working as expected.
Screenshot of the French UI is attached for reference.

xjm’s picture

StatusFileSize
new129.74 KB

I tested manually, and confirmed that the two strings that already exist in core are now automatically translated, and that the rest follow our content style guidelines. Nice work!

As an aside, I learned that the Russian translation for "header" is "шапка", which means "hat", and I find that absolutely delightful.

@himanshu raj, your screenshot in #17 seems to be without the patch applied?

I think this is ready, but leaving NR for another reviewer to confirm to give myself the option of committing it.

himanshu raj’s picture

StatusFileSize
new93.37 KB
new19.05 KB
new259.28 KB
new238.01 KB

Hi @brandonlira, I manually applied the code from the referenced merge request/PR !9611 to the core module on a local copy of Drupal and cleared caches
The updated code compiled without errors and the result which i'm getting is in the ss below.

xjm’s picture

Thanks @himanshu raj. For next time, please embed your screenshots in your comment (see mine above for an example), and indicate which of your screenshots are before the MR and which are after.

Also, we don't need to know that the code compiled without errors or need screenshots of your IDE; the automated testing tells us that. 🙂 thanks!

uesli’s picture

Hi, @xjm and @brandonlira

I can perform manual tests if still needed!

xjm’s picture

Thanks @uesli! I think we're good on manual testing now since @himanshu raj's tenth screenshot appears to be with the MR applied and a confirmation of my result. The last thing we need for this to be RTBC is a general review (code review, confirmation that the changes meet Drupal style guidelines and translation best practices, etc.) Thanks!

himanshu raj’s picture

Thank you @xjm for the feedback! I’ll keep all these points in mind for future . Appreciate the guidance!

xjm’s picture

Issue tags: -Needs manual testing

Sorry I keep trying and failing to remove the tag; crossposts and d.o timeouts. 😀

nexusnovaz’s picture

Status: Needs review » Reviewed & tested by the community

Code looks good to me. Nice and simple.

  • xjm committed 37ec2666 on 11.x
    Issue #3476898 by chizh273, brandonlira, himanshu raj, xjm,...
xjm’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Fixed
StatusFileSize
new349.89 KB

Reviewed locally with git diff --staged --color-words just to confirm that there weren't any changes other than the added t() calls, the capitalization, and the periods:
git diff output of the MR

Since it adds new translatable strings for the other translations, I've committed it to 11.x only. Thanks everyone for working on this and congratulations @anapaulagoetze on your first core issue credit!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.