-
Notifications
You must be signed in to change notification settings - Fork 544
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
Enable USE_THOUSANDS_SEPARATOR
by default & have some larger test ids
#458
Comments
My one reservation is that I'd rather not encourage greater use of USE_THOUSAND_SEPARATOR in the wild - IMO it really is a terrible design choice to apply formatting by default and require a template filter to turn it off, rather than the other way round - and if we put it in bakerydemo, people will copy it however much we advise against it. Maybe if we put a big capitalised PLEASE DON'T DO THIS comment next to it... :-) |
Hmm. That's a good point. Do we know if this kind of issue will appear without that setting on? Maybe a different approach is to run our Wagtail tests differently, instead of Bakerydemo data we update the data used in unit tests to be numbers above 1000 and run one version of the CI with this setting on? |
@lb- |
This shouldn't be necessary any more, now that wagtail/wagtail#12251 is merged. |
Awesome. Great work on that PR Matt. |
I think it would be good for us to have a way to easily test for common issues that relate to large ID values mixed with the usage of thousands separator.
Problem
This code will mostly work but if the id is over 1000, it will be rendered as
1,000
.This kind of bug has happened a few times and it would be good for us to more easily see this problem in Bakerydemo.
It's more pronounced depending on language and the setting
USE_THOUSAND_SEPARATOR
is True.https://docs.djangoproject.com/en/4.2/ref/settings/#std-setting-USE_THOUSAND_SEPARATOR
Proposal
Related
MultipleChooserPanel
modal whenUSE_THOUSAND_SEPARATOR
isTrue
wagtail#11137 / Broken checkboxes in MultipleChooserPanel modal when USE_THOUSAND_SEPARATOR is True wagtail#11134The text was updated successfully, but these errors were encountered: