Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

MM-20169 - Fixed escape characters used in FormattedMessage #4241

Closed
wants to merge 6 commits into from
Closed

MM-20169 - Fixed escape characters used in FormattedMessage #4241

wants to merge 6 commits into from

Conversation

marianunez
Copy link
Member

Summary

react-intl that was recently upgraded in master and in v3 they changed there escape characterto ' which was causing issues in formatting messages that were using that character around formatted values.

Ticket Link

Fixes MM-20169

@marianunez marianunez self-assigned this Nov 15, 2019
@marianunez marianunez added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Nov 15, 2019
@marianunez
Copy link
Member Author

@enahum An escape character was added to the en.json file. The other translation files should not be updated in this case either?

enahum
enahum suggested changes Nov 15, 2019
View reviewed changes
i18n/en.json Show resolved Hide resolved
i18n/en.json Show resolved Hide resolved
enahum
enahum approved these changes Nov 15, 2019
View reviewed changes
Copy link
Contributor

@enahum enahum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code looks good, you need to update the snapshots

@amyblais amyblais added this to the v5.18.0 milestone Nov 18, 2019
@amyblais amyblais added the CherryPick/Approved Meant for the quality or patch release tracked in the milestone label Nov 18, 2019
@marianunez marianunez removed their assignment Nov 18, 2019
@mickmister mickmister removed the 2: Dev Review Requires review by a core commiter label Nov 18, 2019
@marianunez
Copy link
Member Author

/update-branch

@marianunez marianunez added the Do Not Merge Should not be merged until this label is removed label Nov 20, 2019
@marianunez
Copy link
Member Author

marianunez commented Nov 20, 2019 •
edited
Loading

Given that the react-intl upgrade seems that it will be reverted in this PR: #4255 not sure if this should be merged. Adding the Do not Merge for now

@marianunez
Copy link
Member Author

/update-branch

@saturninoabril saturninoabril requested review from saturninoabril and removed request for prapti November 27, 2019 18:26
@saturninoabril
Copy link
Member

@marianunez Are these changes still valid? I'm seeing duplicate single quote per this change.

@marianunez
Copy link
Member Author

marianunez commented Nov 27, 2019 •
edited
Loading

@marianunez Are these changes still valid? I'm seeing duplicate single quote per this change.

@saturninoabril Given that the react-intl upgrade has been reverted in #4255 then it is no longer relevant for v5.18.

I guess we can close this PR and reopen after the upgrade is done later?

@enahum Would you know if there is a place where I can leave this noted for the future upgrade?

@marianunez marianunez removed this from the v5.18.0 milestone Nov 27, 2019
@enahum
Copy link
Contributor

enahum commented Nov 27, 2019

@marianunez probably in the ticket? not sure really

@marianunez
Copy link
Member Author

Closing this PR and noted in the upgrade react-intl ticket MM-17555 for future reference to reopen when the actual upgrade is done.

@marianunez marianunez closed this Nov 27, 2019
@hanzei hanzei removed 3: QA Review Requires review by a QA tester CherryPick/Approved Meant for the quality or patch release tracked in the milestone Do Not Merge Should not be merged until this label is removed labels Feb 20, 2020
@metanerd metanerd deleted the MM-20169 branch May 19, 2020 07:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@enahum enahum enahum approved these changes

@mickmister mickmister mickmister approved these changes

@saturninoabril saturninoabril Awaiting requested review from saturninoabril

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@marianunez @saturninoabril @enahum @mickmister @hanzei @amyblais @mattermod