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

Properly tags Plural-able strings for translation, and removes unnecessary i18n #9897

Merged
merged 3 commits into from
Apr 23, 2021

Conversation

AlexSCorey
Copy link
Member

SUMMARY

This resolves #9891.

It also begings the process of remove i18n._(t string) in places in favor of this syntax

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME
  • UI
AWX VERSION

ADDITIONAL INFORMATION

Screen Shot 2021-04-13 at 11 59 43 AM

Screen Shot 2021-04-13 at 11 59 55 AM

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@unlikelyzero
Copy link

@AlexSCorey can you run the visual tests and functional tests?

<span>{`: ${itemsUnableToDelete}`}</span>
</>
) : (
i18n._(
Copy link
Member

Choose a reason for hiding this comment

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

can this just be t`` now?

Copy link
Contributor

@nixocio nixocio Apr 13, 2021

Choose a reason for hiding this comment

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

If usage of t is not going to completely remove i18n._( we will have 2 standards for translation syntax. It there any benefit in using one over the other?

Copy link
Member Author

Choose a reason for hiding this comment

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

In most cases we will be able to remove withI18n() HOC and also i18n that is being injected into the components in favor of simply using t

Copy link
Member

@keithjgrant keithjgrant left a comment

Choose a reason for hiding this comment

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

🎉

<Plural
value={selected.length}
one="This execution environment is currently being used by other resources. Are you sure you want to delete it?"
other="Deleting these execution environemnts could impact other resources that rely on them. Are you sure you want to delete anyway?"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
other="Deleting these execution environemnts could impact other resources that rely on them. Are you sure you want to delete anyway?"
other="Deleting these execution environments could impact other resources that rely on them. Are you sure you want to delete anyway?"

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@tiagodread tiagodread self-assigned this Apr 19, 2021
@mabashian
Copy link
Member

@AlexSCorey looks like this one is gonna need a rebase

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

Copy link
Contributor

@tiagodread tiagodread left a comment

Choose a reason for hiding this comment

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

@AlexSCorey I'm facing the same problem in different places (dev or production build) like:

image

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

Copy link
Contributor

@tiagodread tiagodread left a comment

Choose a reason for hiding this comment

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

I could not reproduce the problem anymore in a manual exploration testing, the e2e tests are 🟢

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@tiagodread
Copy link
Contributor

regate

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit a744f0d into ansible:devel Apr 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Delete warning showing some plural ploblems
7 participants