Fix for GH-1371 and for handling $$, which Edge does not handle properly. #194
Conversation
zarembsky
commented
Sep 17, 2018
|
| "message": "While Ghostery is free, you can choose to support us through a small subscription of $DOLLARS$ per month in exchange for cool perks, such as color themes, priority help service, and more. Join our mission and subscribe!", | ||
| "placeholders": { | ||
| "dollars": { | ||
| "content": "$1" |
christophertino
Sep 18, 2018
Member
Should this be $2?
Should this be $2?
IAmThePan
Sep 21, 2018
Contributor
@christophertino No, it should be $1 because Serge is passing in $2 into the translation function: t('subscribe_pitch', '@2') and $1 in the placeholder content is for when you will fill that space with a passed variable.
@christophertino No, it should be $1 because Serge is passing in $2 into the translation function: t('subscribe_pitch', '@2') and $1 in the placeholder content is for when you will fill that space with a passed variable.
IAmThePan
Sep 21, 2018
Contributor
@zarembsky There is another escaped $ in hub_supporter_price:
The $$ in between $SUP_START$ and $SUP_END$ is an escaped $.
Not sure if all escaped $ need to be fixed...
@zarembsky There is another escaped $ in hub_supporter_price:
The $$ in between $SUP_START$ and $SUP_END$ is an escaped $.
Not sure if all escaped $ need to be fixed...
zarembsky
Sep 21, 2018
•
Author
Contributor
Yes, same thing. I missed it. Did not look at the hub.
Yes, same thing. I missed it. Did not look at the hub.
zarembsky
Sep 22, 2018
Author
Contributor
$$ in Hub Supporter tab is fixed. Could not find any more similar cases.
$$ in Hub Supporter tab is fixed. Could not find any more similar cases.
| @@ -1775,6 +1775,9 @@ | |||
| }, | |||
| "span_end": { | |||
| "content": "</span>" | |||
| }, | |||
| "dollar_sign": { | |||
| "content": "$1" | |||
jsignanini
Sep 26, 2018
Member
@zarembsky since the dollar sign is static content, can we just pass "content": "$" here? This way we don't have to remember to pass it in when we use this string.
@zarembsky since the dollar sign is static content, can we just pass "content": "$" here? This way we don't have to remember to pass it in when we use this string.
| "message": "While Ghostery is free, you can choose to support us through a small subscription of $DOLLARS$ per month in exchange for cool perks, such as color themes, priority help service, and more. Join our mission and subscribe!", | ||
| "placeholders": { | ||
| "dollars": { | ||
| "content": "$1" |
zarembsky
Oct 2, 2018
Author
Contributor
This suggestion does not work for Edge.
This suggestion does not work for Edge.
| @@ -63,7 +63,7 @@ const SupporterView = (props) => { | |||
| {_renderButton(isSupporter, true)} | |||
| <div | |||
| className="SupporterView__headingCost SupporterView--addSideMargin flex-container align-middle" | |||
| dangerouslySetInnerHTML={{ __html: t('hub_supporter_price') }} | |||
| dangerouslySetInnerHTML={{ __html: t('hub_supporter_price', '$') }} | |||
jsignanini
Sep 26, 2018
Member
@zarembsky using innerHTML to set the html content will trigger a review complaint form Firefox... can we figure out another way of doing this?
@zarembsky using innerHTML to set the html content will trigger a review complaint form Firefox... can we figure out another way of doing this?
zarembsky
Oct 2, 2018
Author
Contributor
We have dangerouslySetInnerHTML all over our code.
We have dangerouslySetInnerHTML all over our code.
