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

Feat: pt_BR localization #1756

Merged
merged 2 commits into from
Sep 23, 2019
Merged

Feat: pt_BR localization #1756

merged 2 commits into from
Sep 23, 2019

Conversation

thiagoeec
Copy link
Contributor

  • Add pt_BR localization for Axe messages

Reviewer checks

Required fields, to be filled out by PR reviewer(s)

  • Follows the commit message policy, appropriate for next version
  • Code is reviewed for security

- Add pt_BR localization for Axe messages
@thiagoeec thiagoeec requested a review from a team as a code owner August 7, 2019 09:54
@WilcoFiers
Copy link
Contributor

@thiagoeec Just wanted to drop you a quick message. Nobody on the axe-core team is familiar with Portuguese. We are looking for someone to help us review this pull request. This is a fantastically cool contribution, and we're very happy with it. Thank you so much for putting this together. We'll try to get this reviewed as soon as possible.

@thiagoeec
Copy link
Contributor Author

@WilcoFiers , thanks for the feedback.
Just so you know, I am currently maintaining pt_BR localization for EPUBCheck, Ace by Daisy and calibre.

@WilcoFiers
Copy link
Contributor

@thiagoeec Much appreciated! I did find someone to help with the review. This may take a week or two.

@WilcoFiers WilcoFiers added this to the Axe-core 3.4 milestone Aug 22, 2019
Copy link
Contributor

@carlosapaduarte carlosapaduarte left a comment

Choose a reason for hiding this comment

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

Good job. Nice to have a Portuguese localisation.

My major concern with the proposal is the use of single quotes both for attributes and for words that have not been translated. This might be confusing for the reader.

locales/pt_BR.json Outdated Show resolved Hide resolved
locales/pt_BR.json Outdated Show resolved Hide resolved
locales/pt_BR.json Outdated Show resolved Hide resolved
locales/pt_BR.json Outdated Show resolved Hide resolved
locales/pt_BR.json Outdated Show resolved Hide resolved
"fail": "A página não tem um cabeçalho (header)"
},
"heading-order": {
"pass": "Hierarquia de títulos válida",
Copy link
Contributor

Choose a reason for hiding this comment

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

The WCAG PT_BR translation for header is "cabeçalho", not "título"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed before.

"fail": "A tag <meta> força atualizações temporizadas da página"
},
"p-as-heading": {
"pass": "Elementos <p> não são estilizados como títulos",
Copy link
Contributor

Choose a reason for hiding this comment

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

The WCAG PT_BR translation for header is "cabeçalho", not "título"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And the word in English used here is 'heading' and not 'header'.

locales/pt_BR.json Outdated Show resolved Hide resolved
locales/pt_BR.json Outdated Show resolved Hide resolved
},
"incompleteFallbackMessage": {
"undefined": {
"failureMessage": "Corrija todos os itens a seguir:{{~it:value}}\n {{=value.split('\\n').join('\\n ')}}{{~}}"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not a translation of
"incompleteFallbackMessage": "axe couldn't tell the reason. Time to break out the element inspector!"

But I might be looking at the wrong place

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Axe message is not actually that. The right one is this:
"failureMessage": "Fix all of the following:{{~it:value}}\n {{=value.split('\\n').join('\\n ')}}{{~}}"
So, I think the translation is correct.

@WilcoFiers
Copy link
Contributor

@thiagoeec could you have a look at Carlos' feedback?

@thiagoeec
Copy link
Contributor Author

Good job. Nice to have a Portuguese localisation.

My major concern with the proposal is the use of single quotes both for attributes and for words that have not been translated. This might be confusing for the reader.

Hi, @carlosapaduarte.

I'll check on your comments as soon as possible. Thanks for reviewing it.

@straker
Copy link
Contributor

straker commented Sep 11, 2019

@thiagoeec have you had a chance to look over the comments? If not, we could try to take this over and update the strings per carlos's suggestions.

@thiagoeec
Copy link
Contributor Author

@thiagoeec have you had a chance to look over the comments? If not, we could try to take this over and update the strings per carlos's suggestions.

Sorry for the long delay. It has been some busy weeks.

Altered according to @carlosapaduarte suggestions.
Copy link
Contributor

@straker straker left a comment

Choose a reason for hiding this comment

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

Fantastic! Thanks for all your work in adding the translations

@straker straker merged commit 330e2ec into dequelabs:develop Sep 23, 2019
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