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

Added pt-PT localization. #2015

Closed
wants to merge 4 commits into from
Closed

Added pt-PT localization. #2015

wants to merge 4 commits into from

Conversation

tiagodenoronha
Copy link
Contributor

Description

Adding localized strings for pt-PT

Copy link
Contributor

@corinagum corinagum left a comment

Choose a reason for hiding this comment

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

  • Please add your name to the LOCALIZATION.md :)
  • Earlier today, @bodyzatva merged in his own version of this file on this PR: Update pt-PT.js #2005. Could both of you collaborate to compromise on the differences?

packages/component/src/Localization/pt-PT.js Outdated Show resolved Hide resolved
packages/component/src/Localization/pt-PT.js Outdated Show resolved Hide resolved
packages/component/src/Localization/pt-PT.js Outdated Show resolved Hide resolved
@tiagodenoronha
Copy link
Contributor Author

Yep, let me check out what he did and get him on board with the changes aswell!

@tiagodenoronha
Copy link
Contributor Author

Merged the changes onto the PR.
Left the "Retry" as it was because it stands for "Try again" and not "Repeat"

Yet, I found this behaviour when trying to figure out when the message was sent, shouldn't that be localized? (Image for reference)

imagem

@bodyzatva can you give it a look and check if it's ok? :)

'Speak': 'Falar',
'Starting…': 'A iniciar',
'Starting…': 'A iniciar...',
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you switch this back to '…' (single character)

Copy link
Contributor

Choose a reason for hiding this comment

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

marked as unresolved; the ellipses is still three characters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wait, I definitely need help with Github.. It shows me like I did: 1b62894

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay that's odd.

So I have some bad news, haha. There's going to be a PR this week that will cause merge conflicts with this PR. Do you mind if we put this on hold for a couple days, and I can alert you when the new PR is in? That way you can branch off of the new master and incorporate your changes, and hopefully this odd situation will resolve itself in the new PR. Does that sound ok? Sorry for the trouble!

@corinagum
Copy link
Contributor

That should definitely be localized. Let me look into this while we're waiting for confirmation from @bodyzatva.
@tiagodenoronha if you have the bandwidth to help with the investigation, your help would be very appreciated! Thank you

Changed ... to … (single char)
@tiagodenoronha
Copy link
Contributor Author

Sure, I can help no problem! Question is, how can I help? JS isn't exactly my top language to code in :)

@corinagum
Copy link
Contributor

Oh no worries - we need to investigate why {retry} is showing instead of the localization, but if you aren't able to then I will get to it eventually -- hopefully by the end of next week :)

@tiagodenoronha
Copy link
Contributor Author

Sorry! When it comes to js, I'm more of a "bother" than a "helper" :)

@corinagum
Copy link
Contributor

No problem. Could you let me know what reproduction steps you're using to get the Send failed message? That will help a lot!

@tiagodenoronha
Copy link
Contributor Author

That one is easy! Spin up bot framework emulator, and send a message with pt-BR locale. If I'm not mistaken, the Bot Framework Emulator uses Webchat behind the scenes, so if it happens there, it happens here :)

@corinagum
Copy link
Contributor

Ah in that case, I think we expect it to fail since pt-BR is not available on Emulator's version of Web Chat. I'll investigate a little more soon and get back to this PR. Thanks for the help :)

Did you intend on updating both pt-PT and pt-BR?

@tiagodenoronha
Copy link
Contributor Author

I didn't, only pt-PT, did I update both by mistake?
I admit i'm still a little green on these "Request Changes" parts...

@corinagum
Copy link
Contributor

Hey @tiagodenoronha, sorry for the confusion. Let's try to clear things up before proceeding. :)

  • You made changes to pr-PR (thank you!)
  • You reported a bug that instead of using the string for 'retry', Web Chat displays {retry}, but based on your comments this is happening in pt-BR, not pt-PT. To clarify, is this problem occurring on pt-PT as well? Or just BR?
  • if just pt-BR, I believe we could do a quick fix by changing 'Retry': '{retry}' to 'Retry': 'Repetir'. What do you think?

@tiagodenoronha
Copy link
Contributor Author

  • Made changes to pt-PT, yes :b
  • Well, since the emulator is only available in pt-BR, I don't actually know if it happens in pt-PT, but I believe I saw this happening in en-US as well... Might need to check and confirm.
  • Sound OK! And for pt-PT also!

@bodyzatva
Copy link
Contributor

@tiagodenoronha this is your final changes? i need this changes for my project :)

@corinagum
Copy link
Contributor

@bodyzatva and @tiagodenoronha, sorry for the delay! I think I figured out the issue (it was a separate bug) and will have the fix up hopefully this week. I will keep you guys updated if anything changes. Thanks for your patience!

@tiagodenoronha
Copy link
Contributor Author

Hey @corinagum , did you check my comments? :)

@corinagum
Copy link
Contributor

@tiagodenoronha, sorry for the trouble but the new merge is in. Would you mind opening a new PR with your changes? Hopefully that will resolve the non-updating commits as well.
Closing this PR, but waiting for your next one! Thanks for the help :)

@corinagum corinagum closed this Jun 3, 2019
@tiagodenoronha
Copy link
Contributor Author

Hey no problem! Give me a couple of days and I'll send you a new PR! :)

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.

None yet

3 participants