Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
This is a followup to #1927584: Add support for the Twig {% trans %} tag extension
Comment | File | Size | Author |
---|---|---|---|
#17 | 2047227-aggregator-summary-items-before.png | 18.88 KB | star-szr |
#17 | 2047227-aggregator-summary-items-after.png | 23.15 KB | star-szr |
#17 | 2047227-datetime-wrapper-before.png | 49.6 KB | star-szr |
#17 | 2047227-datetime-wrapper-after.png | 52.14 KB | star-szr |
#17 | 2047227-forum-submitted-before.png | 17.46 KB | star-szr |
Comments
Comment #1
SeanKelly CreditAttribution: SeanKelly commentedI'm having a whack at this.
Comment #2
markhalliwellJust 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.
Comment #3
SeanKelly CreditAttribution: SeanKelly commentedUpdated all five of the templates that used the old syntax.
Comment #5
markhalliwellI 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:
There is an extra space that was added to the end of the line.
The tests are failing because this is an example of needing to filter the token before using
{% trans %}
. Do something like:Comment #6
star-szrChanging translation strings is out of scope for this issue, I definitely agree with the second and third points though. Thanks for reviewing @Mark Carver!
Comment #7
star-szrMaking the two changes to the patch would be a good novice task.
Comment #8
StephaneQComment #9
star-szrThanks @StephaneQ, the changes look great!
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.Comment #10
StephaneQHere's an updated patch.
Comment #11
star-szrThis feel off my radar a while, I will do the final testing and review but this looks great to me :)
Comment #12
richardj CreditAttribution: richardj commentedTo 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?
Comment #13
richardj CreditAttribution: richardj commentedComment #14
richardj CreditAttribution: richardj commentedApplied #12 to a patch.
Comment #15
star-szrThanks @richardj! Working on manually testing this, so far so good.
Comment #16
star-szrGot interrupted a bit by travel but I'm about halfway through testing this, will post the results when I'm done.
Comment #17
star-szrEverything checks out, looks great.
aggregator-summary-items.html.twig
datetime-wrapper.html.twig
forum-submitted.html.twig
locale-translation-last-check.html.twig
locale-translation-update-info.html.twig
Comment #18
catchCommitted/pushed to 8.x, thanks!