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

Don't accept result values with . #2303

Merged
merged 1 commit into from
May 15, 2024

Conversation

VirginiaDooley
Copy link
Contributor

@VirginiaDooley VirginiaDooley commented May 7, 2024

Closes #2300


@VirginiaDooley VirginiaDooley requested a review from GeoWill May 7, 2024 08:40
Copy link
Contributor

@GeoWill GeoWill left a comment

Choose a reason for hiding this comment

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

Small comment inline.
Also do we never want decimals when excepting number input using DCNumberInput? It sounds quite general purpose to me. Would we ever be expecting percentages/proportions to be entered with it?

@@ -43,7 +43,7 @@ def build_attrs(self, base_attrs, extra_attrs=None):
attrs.update(
{
"inputmode": "numeric",
"pattern": r"[0-9\s\.,]*",
"pattern": r"[0-9\s\,]*",
Copy link
Contributor

Choose a reason for hiding this comment

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

backslash (\) is the escape character for regex, except you don't need to escape , so I'm not sure if it's doing anything - this makes it confusing. It was in front of . because . is a wildcard in regex, but I'm not sure even that was necessary when enclosed in square brackets...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've fixed this up as suggested.

@VirginiaDooley
Copy link
Contributor Author

Small comment inline. Also do we never want decimals when excepting number input using DCNumberInput? It sounds quite general purpose to me. Would we ever be expecting percentages/proportions to be entered with it?

@pmk01 ^^ could you comment? I personally think accepting it and trying to figure out if it's the same context and mentioned in the issue vs a percentage and cleaning it could get tricky.

@pmk01
Copy link
Contributor

pmk01 commented May 7, 2024

We never want decimals (well, we might if we add other electoral systems, but right now, no). We don't want people to add percentages either.

@VirginiaDooley VirginiaDooley force-pushed the hotfix/results-with-decimals branch 2 times, most recently from ee09bf2 to 6dee736 Compare May 7, 2024 11:58
@VirginiaDooley VirginiaDooley requested review from GeoWill and symroe May 7, 2024 13:27
@symroe
Copy link
Member

symroe commented May 14, 2024

@VirginiaDooley This is good, but based on the decimal point, can you rename this field to DCIntegerInput to make it clear that it's for integers only?

@VirginiaDooley VirginiaDooley force-pushed the hotfix/results-with-decimals branch from 6dee736 to 7b6c43d Compare May 14, 2024 11:22
@VirginiaDooley VirginiaDooley merged commit 5b9d73f into master May 15, 2024
5 checks passed
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.

Error submitting results with a decimal value
4 participants