- Issue created by @joachim namyslo
- 🇮🇪Ireland lostcarpark
Interesting. I didn't realise that functionality existed.
On my dev site, the header and link are not displaying, so I think it is a bit broken. It's possible because the header has a format of "rich_html", which doesn't exist on a default D10 install.
I think putting the link in the header of the view is at best a bit hacky, and the sort of thing you can get away with in a view on a website, but not one that's part of a module.
I would suggest moving the link out of the view and into the render array returned by the ScheduledPublishListing controller. Obviously this would include moving the
<div>
tag outside the translation string.Is that what you have in mind?
- 🇩🇪Germany joachim namyslo Kulmbach 🇩🇪 🇪🇺
Yes, because otherwise the string will remains Englisch in over 100 langusges
- 🇮🇪Ireland lostcarpark
MR !16 adds the link into the controller's render array. This means only the link text will be in the translation string.
I haven't taken it out of the view yet, because it's likely that a new view export will be generated for 📌 Provided view config does not work as expected - Edit and Delete links broken. Needs review .
I would like to add a test to verify the new link.
- Status changed to Needs work
about 1 year ago 7:43pm 5 September 2023 - 🇮🇪Ireland lostcarpark
I was holding off on changing the view definition as I thought there might be a new export of the view. There might still be, but it could be a while off, and if I can achieve all the needed fixes by just changing the affected elements, that seems preferable.
I've changed the view definition YML to remove the header, but that only affects new installations, so I've also added an update script that loads the view, replaces the header with an empty array, and saves it.
I've added a test to verify the bulk update link is present on the view page. The test doesn't care where the link is coming from, as long as it's present.
I've also added a test of the update script. It verifies that the link test is present in the view definition before the update, and that it's removed after.
The tests currently only cover the functionality of the bulk update link. I'd like to add more tests of the view functionality, but I feel that's outside the scope of this issue. There's an argument for adding a translation test, but I don't think that's strictly necessary.
This change fixes the reported issue by moving the bulk update link out of the view, and directly into the page controller. As a result, only the link label text needs to be translated, rather than the whole HTML block. Aside from resolving the issue with importing the HTML elements, it removes the risk of a translator accidentally introducing errors in the HTML when translating the text, and removes for the string to be retranslated if changes are made to the HTML in future.
- Status changed to Needs review
about 1 year ago 8:04am 15 September 2023 - 🇮🇪Ireland lostcarpark
CI tests are currently failing for D9 because the test loads a saved database and runs the update script on it, and it fails because it's loading a D10 database against a D9 codebase. I'm reluctant to make too much accommodation for D9 as it's going end of life in a couple of months. Seeing if I can tell CI to skip the test for D9.
- 🇮🇪Ireland lostcarpark
Added an exclusion for the update test to the CI for D9 only. Tests are now passing.
- 🇮🇪Ireland lostcarpark
I have carried out a test of importing the German translations for the updated code. I updated the .PO file attached to the issue to remove the HTML from the affected translation string, and verified that the link is correctly translated after the import.
I'm attaching the updated .PO file.
- 🇺🇸United States cindy_codediaries
I attempted to translate node of article content type from English to German with moderation state of draft to publish and I received the following warning: Deprecated function: strip_tags(): Passing null to parameter #1 ($string) of type string is deprecated in Drupal\comment\CommentForm->buildEntity() (line 322 of core/modules/comment/src/CommentForm.php).
- 🇮🇪Ireland lostcarpark
Hi Cindy,
Could you outline the steps you followed, please?
I'm curious what triggered an error in the comment form. Were you posting a comment?
Thanks for your help. - 🇺🇸United States cindy_codediaries
What triggered these steps was the comment field was empty when I attempted to translate node of article content type. The error message disappeared when I added a comment and published the article.
- 🇮🇪Ireland lostcarpark
Thanks for that information.
That may be a separate bug, but this issue is not related to translating the article.
The issue here has to do with translation of the user interface, and in particular importing the translation strings.
To test, you need to do the following:
- Make sure the "Interface Translation" module is enabled.
- Enable a second language (for the sample files attached, use German).
- Navigate to Config->Region and Language->User Interface Translation (/admin/config/regional/translate).
- Select the Import tab.
- Attempt to import the PO file attached to the original issue, and verify it fails.
- Import the PO file attached to #11 and verify it loads successfully.
- Navigate to Content, then select the Scheduled Publish tab (/admin/content/scheduled-publish).
- Note the position of the "Add bulk scheduled publishing entries" button. (Note: you must have scheduled at least one item for this to be displayed.)
- Switch to language to German (you can do this by inserting "/de" between the domain and the page path in the URL).
- Verify the button text is translated.
Hope this helps you to review.
If you think there's an issue with translating a scheduled article, could you open a separate issue?
- 🇮🇪Ireland lostcarpark
I have added a check that the view header hasn't been modified from the default before removing it, in case the site is using a custom header.
I will add this to the test cases shortly.
- 🇺🇸United States cindy_codediaries
I followed the steps listed on comment #15. It appears the .po file in comment #11 when imported loads successfully. When I switch to the German language it translate some of the button text on the page
-
lostcarpark →
committed b6a9bc48 on 8.x-3.x
Issue #3384944 by lostcarpark, cindy_codediaries: SID 2776211 contains...
-
lostcarpark →
committed b6a9bc48 on 8.x-3.x
- Status changed to Fixed
10 months ago 1:02am 6 January 2024 Automatically closed - issue fixed for 2 weeks with no activity.