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

Fix use of setSpendingCap locale string to properly use our i18n interpolation system #16234

Closed
danjm opened this issue Oct 19, 2022 · 0 comments · Fixed by #16347
Closed

Fix use of setSpendingCap locale string to properly use our i18n interpolation system #16234

danjm opened this issue Oct 19, 2022 · 0 comments · Fixed by #16347
Assignees
Labels
area-transactions area-translation Issues relating to translating the app into various languages. team-confirmations-secure-ux DEPRECATED: please use "team-confirmations" label instead type-bug
Milestone

Comments

@danjm
Copy link
Contributor

danjm commented Oct 19, 2022

Currently, in token-allowance.js we have this code:

<Box marginBottom={5}>
        <Typography
          variant={TYPOGRAPHY.H3}
          fontWeight={FONT_WEIGHT.BOLD}
          align={TEXT_ALIGN.CENTER}
        >
          {isFirstPage ? t('setSpendingCap') : t('reviewSpendingCap')}
        </Typography>
      </Box>
      <Box>
        <ContractTokenValues
          tokenName={tokenSymbol}
          address={tokenAddress}
          chainId={fullTxData.chainId}
          rpcPrefs={rpcPrefs}
        />
      </Box>

setSpendingCap is defined in the locale file as follows:

  "setSpendingCap": {
    "message": "Set a spending cap for your"
  },

This is a problem because this won't be able to be translated to a language that would place the contract token values somewhere other than the end of the string.

That is why we have a system of replacable symbols in the translation files. Take, for example, spendLimitRequestBy:

  "spendLimitRequestedBy": {
    "message": "Spend limit requested by $1",
    "description": "Origin of the site requesting the spend limit"
  },

Which gets used as follows (in edit-approval-permission.component.js):

{t('spendLimitRequestedBy', [origin])}

The second parameter passed to the translation function can be a whole react component.

We should refactor the definition of setSpendingCap and its use in token-allowance.js to follow this pattern.

Also, another case of this is introduced here: https://github.com/MetaMask/metamask-extension/pull/16157/files#diff-7509d7529ea1cbc57ef79713676f7d124d9d5a446053f94836876c4cd3f3ccd6R2941, that should be fixed as well

@bschorchit bschorchit added area-transactions team-confirmations-secure-ux DEPRECATED: please use "team-confirmations" label instead area-translation Issues relating to translating the app into various languages. labels Oct 27, 2022
@adnansahovic adnansahovic self-assigned this Nov 1, 2022
@hilvmason hilvmason added this to the v10.23.0 milestone Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-transactions area-translation Issues relating to translating the app into various languages. team-confirmations-secure-ux DEPRECATED: please use "team-confirmations" label instead type-bug
Projects
None yet
4 participants