Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CRM-21149 ts fix for "(including yourself)" on the Event Registration Form #10942

Merged
merged 1 commit into from
Oct 8, 2017

Conversation

agileware
Copy link
Contributor

Overview

On Event Registration Form, cannot target the text "(including yourself)" using word replacement because the parenthesis is outside the ts markers

See https://issues.civicrm.org/jira/browse/CRM-21149

Before

"including yourself" can be translated, but cannot translate "(including yourself)"

After

Can translate "(including yourself)"

Technical Details

Moves location of ts tags

Comments

Have submitted PR for civicrm/l10n#17 as well

@seamuslee001
Copy link
Contributor

@mlutfy do you have any comments on this from an internationalisation perspective?

@eileenmcnaughton
Copy link
Contributor

@mlutfy ping - this probably just needs a quick comment from you

@jcorlew
Copy link

jcorlew commented Sep 27, 2017

Jenkins please retest this

@jcorlew
Copy link

jcorlew commented Sep 27, 2017

Jenkins retest this

1 similar comment
@colemanw
Copy link
Member

Jenkins retest this

@jcorlew
Copy link

jcorlew commented Sep 27, 2017

Jenkins retest this please

1 similar comment
@colemanw
Copy link
Member

Jenkins retest this please

@jcorlew
Copy link

jcorlew commented Sep 27, 2017

I'll be testing this when the build completes

@alifrumin
Copy link
Contributor

Jenkins retest this please

@jcorlew
Copy link

jcorlew commented Sep 28, 2017

We tested this successfully
Test process:
Created a new site with dmaster
Created a new event "Sprint Test 1," enabled online registration and enabled "register multiple participants"
Made changes in Word Replacement (Administer --> Custom Data & Screens --> Word Replacement)
(including yourself) to change to parantheses

Before the patch the replacement did not take effect
After the patch the replacement worked

@mlutfy we believe this is ready to merge if you don't have any international concerns

@mlutfy
Copy link
Member

mlutfy commented Oct 8, 2017

This would break existing string overrides that people may have put in place, however:

  • the ts usage was indeed wrong, since I consider this to be typographic. Typography should always be included in the translation string. Different languages might handle or use parenthesis differently.
  • we change strings all the time, so "expect random breakage from overriden strings" is the current status quo.

Thank you for the patch & review!

@mlutfy mlutfy merged commit 33c3fad into civicrm:master Oct 8, 2017
@mlutfy mlutfy changed the title CIVICRM-441 CRM-21149 Update translatable string changing "including… CRM-21149 Update translatable string changing "including… Oct 8, 2017
@mlutfy mlutfy changed the title CRM-21149 Update translatable string changing "including… CRM-21149 ts fix for "(including yourself)" on the Event Registration Form Oct 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants