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

fix: remove error when rendering template with language not set to English #67

Merged
merged 2 commits into from
Dec 28, 2018
Merged

Conversation

dodumosu
Copy link
Collaborator

Setting a browser language to a non-English language will cause the application to error out.

@dodumosu dodumosu requested a review from takinbo December 21, 2018 15:50
@takinbo
Copy link
Collaborator

takinbo commented Dec 27, 2018

Can you explain what kind of error you receive? If your PR is merged, it would prevent the display of the menu items in any other language other than English. Ideally, the menu items should be marked as translatable and rendered in English if there's no available string in the chosen language, unless I'm missing something.

@dodumosu
Copy link
Collaborator Author

dodumosu commented Dec 28, 2018

The specific exception is a TypeError with the message: "unhashable type: _LazyString". To reproduce:

  1. Compile language catalogs
  2. Set browser language to any supported language, eg French
  3. Open app in browser (preferably with debug mode turned on)

The reason the error comes up is because the items I'm removing the translation helper from are all already strings marked for translation in the Python source code. Marking them again for translation in the template is what causes the error.

Please see this issue and the PR that fixes it.

@takinbo
Copy link
Collaborator

takinbo commented Dec 28, 2018

You are absolutely right. I've approved the PR. We would need to also confirm that all the menu item names are adequately marked for translation.

@takinbo takinbo merged commit ee8859e into nditech:develop Dec 28, 2018
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.

2 participants