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.

Sign up for GitHub

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

Jump to bottom

fix: translations #12942

Merged
merged 4 commits into from May 11, 2021
Merged

fix: translations #12942

merged 4 commits into from May 11, 2021

Conversation

barredterra
Copy link
Collaborator

@barredterra barredterra commented Apr 21, 2021 •
edited

Issue 1: Wrong path in get_messages_from_include_files

hooks.py

app_include_js = "/assets/js/erpnext.min.js"
web_include_js = "/assets/js/erpnext-web.min.js"

Old (not a valid path)

os.path.join(frappe.local.sites_path, js_path) # -> /assets/js/erpnext.min.js

New (valid relative path)

os.path.join(frappe.local.sites_path, js_path.strip('/')) # -> ./assets/js/erpnext.min.js

Issue 2: Navbar labels not included in translations

Include the labels of Navbar Items, as defined in Navbar Settings, in the translatable messages.

Close #12368
Close #12421

@barredterra barredterra requested a review from a team as a code owner April 21, 2021 15:41
@barredterra barredterra requested review from hasnain2808 and removed request for a team April 21, 2021 15:41
@stale

This comment has been minimized.

Sign in to view
@stale stale bot added the inactive label May 4, 2021
@barredterra
Copy link
Collaborator Author

Not stale. @hasnain2808 could you give this a review?

@stale stale bot removed the inactive label May 4, 2021
@barredterra barredterra changed the title fix: get_messages_from_include_files fix: translations May 5, 2021
@barredterra barredterra force-pushed the fix_get_messages_from_include_files branch from 77ed851 to e7ec57b Compare May 10, 2021 13:28
@barredterra barredterra mentioned this pull request May 10, 2021
Closed
gavindsouza
gavindsouza requested changes May 10, 2021
View reviewed changes
Copy link
Collaborator

@gavindsouza gavindsouza left a comment

Choose a reason for hiding this comment

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

Works as expected.

LGTM. Just a couple of nitpicks

frappe/translate.py Outdated Show resolved Hide resolved
frappe/translate.py Outdated Show resolved Hide resolved
@gavindsouza gavindsouza self-assigned this May 10, 2021
@barredterra @gavindsouza
Update frappe/translate.py
38b9eaa 
Co-authored-by: gavin <gavin18d@gmail.com>
@barredterra @gavindsouza
refactor: strip -> lstrip
50fcbb7 
Co-authored-by: gavin <gavin18d@gmail.com>
@gavindsouza gavindsouza merged commit 67ceab8 into frappe:develop May 11, 2021
@barredterra barredterra deleted the fix_get_messages_from_include_files branch May 11, 2021 11:20
@barredterra
Copy link
Collaborator Author

@Mergifyio backport version-13-hotfix

mergify bot pushed a commit that referenced this pull request May 11, 2021
@barredterra
fix: translations (#12942)
c5c7b3c 
* fix: get_messages_from_include_files
* feat: include labels of navbar items
* refactor: strip -> lstrip

Co-authored-by: gavin <gavin18d@gmail.com>
(cherry picked from commit 67ceab8)
@mergify mergify bot mentioned this pull request May 11, 2021
Merged
@mergify
Copy link
Contributor

mergify bot commented May 11, 2021

Command backport version-13-hotfix: success

Backports have been created

surajshetty3416 added a commit that referenced this pull request May 12, 2021
@surajshetty3416
Merge pull request #13194 from frappe/mergify/bp/version-13-hotfix/pr…
1275ae4 
…-12942

fix: translations (backport #12942)
@shariquerik shariquerik mentioned this pull request May 12, 2021
Closed
codescientist703 pushed a commit to codescientist703/frappe that referenced this pull request May 16, 2021
@barredterra @gavindsouza @codescientist703
fix: translations (frappe#12942)
0e15ede 
* fix: get_messages_from_include_files
* feat: include labels of navbar items
* refactor: strip -> lstrip

Co-authored-by: gavin <gavin18d@gmail.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@gavindsouza gavindsouza gavindsouza approved these changes

@hasnain2808 hasnain2808 Awaiting requested review from hasnain2808

Assignees

@gavindsouza gavindsouza

Labels
squash
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ideas to fix untranslatable labels/text Translations are not working
2 participants
@barredterra @gavindsouza