7556 localized numbers #9439
Merged
7556 localized numbers #9439
Conversation
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
There are no references to this template anywhere in the repo. I found this because this would have been a location to use `unlocalize` and I wanted to open a test page while making the change. Since this template is unused, I can not test it. And since it is unused, it also seems unnecessary to keep around. If we miss it in the future, we can restore it from git.
This reverts commit 47867e9. Guess I missed that this is implicitly used by the BanneredCampaignPage model. Ups.
tbrlpld
force-pushed
the
7556-localized-numbers
branch
from
2ad10a0 to
ce8f1d6
Compare
I could not find a more appropriate place in the docs, so I put it in the README.
tbrlpld
requested review from
fessehaye,
danielfmiranda,
kevinhowbrook and
TheoChevalier
|
@fessehaye @danielfmiranda @TheoChevalier @kevinhowbrook Tagging all of you for visibility. This is probably something we all need to keep an eye out for going forward. |
TheoChevalier
approved these changes
Oct 3, 2022
There was a problem hiding this comment.
Thanks for going through all this @tbrlpld!
I also suspect disabling USE_L10N would have broken the date localization (blog post publication dates, for instance), so this seem to be the right (albeit painful) way
danielfmiranda
approved these changes
Oct 4, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Suggestion cannot be applied right now. Please check back later.
Description
It was previously observed that numbers entered into the template have been localized broadly. As a response we had turned off the
USE_THOUSAND_SEPARATORsetting. But, this still left the decimal separators to be localized (e.g. the German locale would use a comma instead of a dot). This can lead to issues when the values are inserted into the template for computational use, rather than enduser consumption (think values of inline scripts and style, but also HTML attributes likewidthandheighton theimgtag). It would be possible to deactivate that feature withL10N = False, but that is not a future proof change since the setting will be deprecated in Django 4.0 and ignored starting with Django 5.0.Therefore, we will need to make sure that when ever we insert a value (especially float values) into the template for programmatic use, that we use the
unlocalizefilter to make sure the format of the numbers follows English standards and is interpretable in a programmatic way.See also: https://docs.djangoproject.com/en/4.1/topics/i18n/formatting/#controlling-localization-in-templates
Link to sample test page:
Related PRs/issues: #7556
Checklist
Tests
Is the code I'm adding covered by tests?Changes in Models:
Did I update or add new fake data?Did I squash my migration?Are my changes backward-compatible. If not, did I schedule a deploy with the rest of the team?Documentation: