dev/translation#90 address translation issue on membership #31299
Conversation
|
🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷 Introduction for new contributors...
Quick links for reviewers...
|
|
The issue associated with the Pull Request can be viewed at https://lab.civicrm.org/dev/translation/-/issues/90 |
| @@ -116,13 +116,9 @@ | |||
| {if !empty($auto_renew)} {* Auto-renew membership confirmation *} | |||
| {crmRegion name="contribution-thankyou-recur-membership"} | |||
| <br /> | |||
| {if !$installments || $installments > 1} | |||
There was a problem hiding this comment.
only thought here @eileenmcnaughton I wonder is if $installments is always being assigned to the template or not
There was a problem hiding this comment.
@seamuslee001 yeah - I'm confused - I'm thinking it maybe is not - but note that if it is NOT assigned there is still text rendered - it's just that there is
<strong>{ts 1=$frequency_unit}This membership will be renewed automatically every %1.{/ts}</strong>
vs the per-unit version
& it seems like the per unit version is generally preferred
There was a problem hiding this comment.
There is also a problem with
<strong>{ts 1=$frequency_interval 2=$frequency_unit}This membership will be renewed automatically every %1 %2(s).{/ts}</strong>
that seems to be pushed out - I don't know whether you can use the plural whatsit & have day translate to days in English - that seems like it has been left out of scope over a few iterations
There was a problem hiding this comment.
If I read this right, the every %1 %2(s) that is still there only affects when a limit is set on the number of instalments, which presumably is not used much.
Ideally it should use the same pattern as for unlimited instalments, with the if/elseif for day/week/month/year (but that's outside the scope of this PR).
There was a problem hiding this comment.
@mlutfy I pushed in an addition fix for that - if it looks good I suggest we merge this to the rc but only back port the original patch to 5.78
There was a problem hiding this comment.
@eileenmcnaughton Looks good to me, thanks! I added merge-ready based on the feedback from Samuele on Gitlab (he had also worked on the initial fixes around this).
The (s) pattern doesn't work for translation
mlutfy
added
the
merge ready
|
I tested a few scenarios and seems all OK. It was on Standalone, with PHP notices enabled, latest Smarty, etc. Before:
After:
Contribution Page ThankYou with a number of instalments:
Contribution Page ThankYou with no limit of instalments:
|
Overview
A brief description of the pull request. Keep technical jargon to a minimum. Hyperlink relevant discussions.
Before
What is the old user-interface or technical-contract (as appropriate)?
For optimal clarity, include a concrete example such as a screenshot, GIF (LICEcap, SilentCast), or code-snippet.
After
What changed? What is new old user-interface or technical-contract?
For optimal clarity, include a concrete example such as a screenshot, GIF (LICEcap, SilentCast), or code-snippet.
Technical Details
If the PR involves technical details/changes/considerations which would not be manifest to a casual developer skimming the above sections, please describe the details here.
Comments
Anything else you would like the reviewer to note