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

Bugfix missing l10n load #919

Merged
merged 5 commits into from
Oct 17, 2023
Merged

Conversation

tvanekeris
Copy link
Contributor

Added missing load tag "l10n" in all templates where "localize" filter is used.

@jieter
Copy link
Owner

jieter commented Jun 23, 2023

@tvanekeris thanks for your contribution. Apparently, this is not commonly used, as I've never seen any reports of this bug.

Should we keep this functionality? Are you using it (with custom templates)?

@tvanekeris
Copy link
Contributor Author

tvanekeris commented Jun 26, 2023

Hi @jieter, first of all thanks for the great project! I have built a custom template based on the templates from the project (because I am mostly not using bootstrap, but bulma). I have then seen that loading the l10n module is missing in the templates and wanted to play it back to the main project. So far I have no values in my tables that need the localization (most data is currently strings), but I will probably have in the future (e.g. for comma or point separator for decimals), so I think to keep the l10n in the templates is a good thing.

@marksweb
Copy link

marksweb commented Jul 13, 2023

You only need to load l10n if you're going to use the template tags/filters it provides which are localize and unlocalize (docs).

The templates are making use of these filters, so this seems a valid change 👍

If it's just localization of data like dates and numbers that you need, the USE_L10N setting should be all you need (docs).

@tvanekeris
Copy link
Contributor Author

tvanekeris commented Jul 14, 2023

Hi, yes exactly. In every file where I added the {% load l10n %} the filter is used, e.g., in

<td {{ column.attrs.td.as_html }}>{% if column.localize == None %}{{ cell }}{% else %}{% if column.localize %}{{ cell|localize }}{% else %}{{ cell|unlocalize }}{% endif %}{% endif %}</td>

It might work without the include (have not tested it) but according to the docs (and also hints from PyCharm IDE) the load should be added.

@jieter
Copy link
Owner

jieter commented Oct 17, 2023

Thanks! What puzzles me is that this never failed for me in tests, or that we received any complaints about it. But I agree it looks to be necessary. I'll merge this.

@jieter jieter merged commit 0a5411c into jieter:master Oct 17, 2023
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.

3 participants