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 range inputs #554

Merged
merged 2 commits into from
Mar 16, 2024
Merged

Conversation

MarvinKlein1508
Copy link

Range inputs doesn't work when you want to use them with float, double or decimal when CultureInfo uses a , as seperator for floating points.

Otherwise the component will behave buggy and renders invalid HTML.

This patch fixes this behaviour by always using CultureInfo.InvariantCulture to always work in all localizations.

@gvreddy04
Copy link
Contributor

@MarvinKlein1508 I'm reviewing this PR. Could you help me reproduce this issue? If you wouldn't mind, could you share a screen recording of the issue?

@MarvinKlein1508
Copy link
Author

@gvreddy04 the decimal demo I've added is showcasing this issue. All your need is a browser with a locale which uses , as decimal separator

@MarvinKlein1508
Copy link
Author

RANGE

See:
html-range

Fixing only the representation in the DOM doesn't fix the issue because the same problem occurs from the sent Javascript value. That's why the convert functions need to use CultureInfo.InvarientCulture as well.

@gvreddy04
Copy link
Contributor

Fixing only the representation in the DOM doesn't fix the issue because the same problem occurs from the sent Javascript value. That's why the convert functions need to use CultureInfo.InvarientCulture as well.

@MarvinKlein1508 Thank you for your quick reply on this. I'll test and complete this PR.

@gvreddy04 gvreddy04 added this to the 2.1.0 milestone Feb 26, 2024
@gvreddy04
Copy link
Contributor

@MarvinKlein1508 I changed the browser language to French to reproduce the issue, but I was not able to reproduce it. Please let me know if I'm missing anything. Additionally, please share the steps you followed to change the browser language.

image

image

@MarvinKlein1508
Copy link
Author

If I remember correctly then Google Chrome automatically fixes the behaviour. We had this issue for InputNumber compoennts in our app in the past as well.

Try Firefox with German locale.

@MarvinKlein1508
Copy link
Author

It's also broken on Safari on with locale which uses , as separator

@gvreddy04
Copy link
Contributor

@MarvinKlein1508 I was able to replicate this issue by using Opera with German.

Copy link
Contributor

@gvreddy04 gvreddy04 left a comment

Choose a reason for hiding this comment

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

LGTM

@gvreddy04 gvreddy04 merged commit d998196 into vikramlearning:master Mar 16, 2024
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants