This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I want to merge this change because... it adds react-intl integration. Basing on the great work of @Wael-Mohamed PR (#731) I made another clean attempt to localize storefront. I fully agree with @krzysztofwolski (#760) to split this task into smaller ones and resolve issues in smaller modules. I propose to go with three steps:
Make strings locale-aware (this PR is an attempt for that)
Add functionality to change storefront language and store configuration
Load localized model data from Django backend
For now, you can only manually change storefront language (still quite useful) by setting it in LocaleProvider. Also for now I only added one additional language (without translation) just for test.
I managed to localize as many strings as I found but some might still be missing. Known things TODO:
Localizing AccountMenu and AccountMenuMobile links
Please let me known what do you think about this work and the proposed steps. With your acceptance and guidance, I could try to tackle them.
Screenshots
Pull Request Checklist
All visible strings are translated with proper context.
All data-formatting is locale-aware (dates, numbers, and so on).
The changes are tested.
The code is documented (docstrings, project documentation).
Is everything fine with CircleCI config? This PR fails at both (build, cypress/run) steps with Error: Environment variable API_URI not set error. Locally it builds successfully
I added transifex resource for this project, so it's now really translatable :) #785
Oh and I also mentioned you in changelog because I somehow missed that you didn't add yourself there.
Hey @przlada, thanks a lot for your contribution! If you're willing to help even more, I think next week we should be able to provide more detailed issues with descriptions of what needs to be done. We already have designs of the language switcher which we'd share with you as well. We need to wait for @krzysztofwolski to return from holiday as he's in charge of the project :)
I added transifex resource for this project, so it's now really translatable :) #785
Oh and I also mentioned you in changelog because I somehow missed that you didn't add yourself there.
That's great, thanks @dominik-zeglen👍 I added my test Polish translation to Transifex and made PR with it #786
Hey @przlada, thanks a lot for your contribution! If you're willing to help even more, I think next week we should be able to provide more detailed issues with descriptions of what needs to be done. We already have designs of the language switcher which we'd share with you as well. We need to wait for @krzysztofwolski to return from holiday as he's in charge of the project :)
@maarcingebala Yes I'm willing to help 😊 I will patiently wait for @krzysztofwolski. What's the best way to discuss the details? Here on Github or using some different channels?
@przlada I'm back! On Thursday I'll have github issues for you with designs. Personally I'm fan of async communication on GH, but if there will be any questions I'm open for Hangouts call
@przlada thanks for patience :) Here is an issue with resources: #798. I suggest to move any questions to that issue. Let me know if you need anything 💪
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
None yet
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.
I want to merge this change because... it adds
react-intl
integration. Basing on the great work of @Wael-Mohamed PR (#731) I made another clean attempt to localize storefront. I fully agree with @krzysztofwolski (#760) to split this task into smaller ones and resolve issues in smaller modules. I propose to go with three steps:For now, you can only manually change storefront language (still quite useful) by setting it in
LocaleProvider
. Also for now I only added one additional language (without translation) just for test.I managed to localize as many strings as I found but some might still be missing. Known things TODO:
AccountMenu
andAccountMenuMobile
linksPlease let me known what do you think about this work and the proposed steps. With your acceptance and guidance, I could try to tackle them.
Screenshots
Pull Request Checklist