Comments

Assigned:Unassigned» SeanKelly

I'm having a whack at this.

Just be aware of the efforts in #1980004: [meta] Creating Dream Markup as well. Those might not get committed, but you might want to start tracking things like: #2047095: Remove $submitted from node templates in _this_ issue.

Assigned:SeanKelly» Unassigned
Status:Active» Needs review
Issue tags:+Needs manual testing
StatusFileSize
new3.15 KB
FAILED: [[SimpleTest]]: [MySQL] 57,497 pass(es), 2 fail(s), and 19 exception(s).
[ View ]

Updated all five of the templates that used the old syntax.

Status:Needs review» Needs work

The last submitted patch, 2047227-2.patch, failed testing.

+++ b/core/modules/aggregator/templates/aggregator-summary-items.html.twig
@@ -19,5 +19,5 @@
+  <a href="{{ source_url }}">{% trans %}More<span class="visually-hidden"> posts about {{ title|placeholder }}</span>{% endtrans %}</a>

I know the "visually-hidden" span was inside the translation before... but I'm not sure if I like putting HTML inside a translation string. I can be swayed either way here... but maybe just try something like:

{% set title %}<span class="visually-hidden"> posts about {{ title|placeholder }}</span>{% endset %}
<a href="{{ source_url }}">{% trans %}More{{ title }}{% endtrans %}</a>

+++ b/core/modules/datetime/templates/datetime-wrapper.html.twig
@@ -17,7 +17,7 @@
-    {{ '!title!required'|t({ '!title': title, '!required': required }) }}
+    {% trans %}{{ title|passthrough }}{{ required|passthrough }}{% endtrans %} ¶

There is an extra space that was added to the end of the line.

+++ b/core/modules/locale/templates/locale-translation-update-info.html.twig
@@ -21,7 +21,7 @@
-    <span class="text">{{ 'Updates for: @modules'|t({'@modules': modules|join(', ')}) }}</span>
+    <span class="text">{% trans %}Updates for: {{ modules|join(', ') }}{% endtrans %}</span>

The tests are failing because this is an example of needing to filter the token before using {% trans %}. Do something like:

{% set module_list = modules|join(', ') %}
<span class="text">{% trans %}Updates for: {{ module_list }}{% endtrans %}</span>

Changing translation strings is out of scope for this issue, I definitely agree with the second and third points though. Thanks for reviewing @Mark Carver!

Issue tags:+Novice

Making the two changes to the patch would be a good novice task.

Status:Needs work» Needs review
StatusFileSize
new1.37 KB
new3.19 KB
PASSED: [[SimpleTest]]: [MySQL] 57,593 pass(es).
[ View ]

Status:Needs review» Needs work

Thanks @StephaneQ, the changes look great!

+++ b/core/modules/forum/templates/forum-submitted.html.twig
@@ -17,7 +17,8 @@
+  <span class="submitted">
+    {% trans %}By {{ author|passthrough }} {{ time }} ago{% endtrans %}</span>

I suppose the wrapping on this one is a bit weird. The </span> should either be on the next line or this should all be on one line.

Status:Needs work» Needs review
StatusFileSize
new571 bytes
new3.18 KB
PASSED: [[SimpleTest]]: [MySQL] 58,052 pass(es).
[ View ]

Here's an updated patch.

Assigned:Unassigned» Cottser

This feel off my radar a while, I will do the final testing and review but this looks great to me :)

Status:Needs review» Needs work

To bump this, the following code piece, could it be placed on one line because it doesn't go over the max line length limit (80 characters), or are there code standards forbidding this?

{{ 'Last checked: @time ago'|t({'@time': time}) }}
+    {% trans %}
+      Last checked: {{ time }} ago
+    {% endtrans %}

Assigned:Cottser» richardj

Status:Needs work» Needs review
StatusFileSize
new3.17 KB
PASSED: [[SimpleTest]]: [MySQL] 58,653 pass(es).
[ View ]
new582 bytes

Applied #12 to a patch.

Assigned:richardj» Cottser

Thanks @richardj! Working on manually testing this, so far so good.

Got interrupted a bit by travel but I'm about halfway through testing this, will post the results when I'm done.

Everything checks out, looks great.

aggregator-summary-items.html.twig

2047227-aggregator-summary-items-before.png2047227-aggregator-summary-items-after.png

datetime-wrapper.html.twig

2047227-datetime-wrapper-before.png2047227-datetime-wrapper-after.png

forum-submitted.html.twig

2047227-forum-submitted-before.png2047227-forum-submitted-after.png

locale-translation-last-check.html.twig

2047227-locale-translation-last-check-before.png2047227-locale-translation-last-check-after.png

locale-translation-update-info.html.twig

2047227-locale-translation-update-info-before.png2047227-locale-translation-update-info-after.png

Status:Reviewed & tested by the community» Fixed

Committed/pushed to 8.x, thanks!

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