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

Allow localized messages to not use substitutions #8895

Merged

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Jul 2, 2020

The getMessage function in i18n-helper was assuming that any substitutions passed into the transaction function were used by the corresponding localized message. However, some messages are intentionally ignoring substitutions passed in. This was done to simplify the UI logic, so the same substitutions could be passed in for many different messages, even if some don't use them.

For example, transactionCancelSuccess is passed two substitutions but only uses the second one. transactionErrored is passed in two, but uses neither.

getMessage has been updated to no longer make that assumption. It will now only throw an error if the localized message expects a substitution that was not given. A given substitution that is unused results in no error.

The `getMessage` function in `i18n-helper` was assuming that any
substitutions passed into the transaction function were used by the
corresponding localized message. However, some messages are
intentionally ignoring substitutions passed in. This was done to
simplify the UI logic, so the same substitutions could be passed in for
many different messages, even if some don't use them.

For example, `transactionCancelSuccess` is passed two substitutions but
only uses the second one. `transactionErrored` is passed in two, but
uses neither.

`getMessage` has been updated to no longer make that assumption. It
will now only throw an error if the localized message expects a
substitution that was not given. A given substitution that is unused
results in no error.
@metamaskbot
Copy link
Collaborator

Builds ready [b08e2f2]
Page Load Metrics (660 ± 45 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint309847199
domContentLoaded3827526589345
load3847536609345
domInteractive3827516589345

@Gudahtt Gudahtt marked this pull request as ready for review July 2, 2020 21:52
@Gudahtt Gudahtt requested a review from a team as a code owner July 2, 2020 21:52
Copy link
Member

@rekmarks rekmarks left a comment

Choose a reason for hiding this comment

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

LGTM!

@Gudahtt Gudahtt merged commit 4855d1b into develop Jul 2, 2020
@Gudahtt Gudahtt deleted the allow-localized-messages-to-not-use-given-substitutions branch July 2, 2020 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants