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: Improved Brazilian Portuguese translations #381

Merged
merged 2 commits into from
Dec 15, 2022
Merged

feat: Improved Brazilian Portuguese translations #381

merged 2 commits into from
Dec 15, 2022

Conversation

purp0s3
Copy link
Contributor

@purp0s3 purp0s3 commented Dec 15, 2022

Description

  • Better choice of words.
  • Added date format.
  • Added Week Streak translation.
  • Added Longest Week Streak translation.

Ref: #236

Type of change

  • Updated documentation (updated the readme, templates, or other repo files)

How Has This Been Tested?

  • Tested locally with a valid username
  • Tested locally with an invalid username
  • Ran tests with composer test
  • Added or updated test cases to test new features

Checklist:

  • I have checked to make sure no other pull requests are open for this issue
  • The code is properly formatted and is consistent with the existing code style
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

Screenshots

@DenverCoder1
Copy link
Owner

DenverCoder1 commented Dec 15, 2022

Looks great, thanks for contributing! 🎉

Regarding the date format field, it's only needed if the dates do not appear well without it.

i.e. as of right now, it uses the php intl library to format the dates
image

Overriding the date format will change it to this format and not use the locale default which can't be used if overridden
image

So if the dates look correct with the default date format, it should not have a date format specified, since it gives the user a choice of using the locale default or customizing it to a specific format themselves.

If you think the default date format displays badly or is incorrect, we can go ahead with it overriding it, though.

Let me know what you think.

@DenverCoder1 DenverCoder1 added the enhancement New feature or request label Dec 15, 2022
@purp0s3
Copy link
Contributor Author

purp0s3 commented Dec 15, 2022

The default format is correct actually, I didn't check it properly. Should I do another pull request or can you merge it without that line?

src/translations.php Outdated Show resolved Hide resolved
Co-authored-by: Jonah Lawrence <jonah@freshidea.com>
@DenverCoder1
Copy link
Owner

DenverCoder1 commented Dec 15, 2022

You don't have to create a new pull request. You can always update a PR by making changes on your source branch, which in this case is purp0s3:improved-brazilian-portuguese.

Or by clicking the commit suggestion button like I see you probably just did. 👍

Copy link
Owner

@DenverCoder1 DenverCoder1 left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for the contribution 🎉

@DenverCoder1 DenverCoder1 changed the title Improved Brazilian Portuguese translations feat: Improved Brazilian Portuguese translations Dec 15, 2022
@DenverCoder1 DenverCoder1 merged commit 740c4fe into DenverCoder1:main Dec 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants