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.

Sign up for GitHub

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

Jump to bottom

Price slider incorrect format in german locale store #59

Open
tim-breitenstein-it opened this issue Feb 19, 2021 · 5 comments
Open

Price slider incorrect format in german locale store #59

tim-breitenstein-it opened this issue Feb 19, 2021 · 5 comments

Comments

@tim-breitenstein-it
Copy link

The javascrip parseFloat() function is not working for price slider if the store locale is german.

In a german store the , is the decimal separator und the . is the thousand separator.

URL: http://m24.test.com/test.html?price=0-4870

Initial situation:

image

Click into from price (1) => to price change to incorret format

image

Click into to price (2) => to price change to incorret format

image

Same behavior for to price if to price is filled.

In english locale store it works fine benause the . is the decimal separator und the , the thousand separator.

image

Solution for German store:

_onInputsChange: function(event) {
                var key = event.key || event.keyCode;

                if (event.type === 'blur' || key === 'Enter' || key === 13) {
                    var from = parseFloat(
                        this.element
                            .find(this.options.fromInput)
                            .val()
                            .replace(/[^\d\.\,]/, '')
                            .replace(/\./g, '')
                            .replace(/\,/g, '.') || 0
                    );
                    var to = parseFloat(
                        this.element
                            .find(this.options.toInput)
                            .val()
                            .replace(/[^\d\.\,]/, '')
                            .replace(/\./g, '')
                            .replace(/\,/g, '.') || 0
                    );

                    if (!to) {
                        to = this.maxValue;
                    }

                    if (from > to) {
                        from = to - this.rate;
                    }

                    if (from < this.minValue) {
                        from = this.minValue;
                    }

                    if (to > this.maxValue) {
                        to = this.maxValue;
                    }

                    this._updateRange(from, to);
                }
            }

If switch back to english store the solution for german store will not work.
@mborkowski
Copy link

Hi @tim-breitenstein-it
Nice catch. Sorry for late reply but I had to wait for QA result. So I can confirm the issue, however reproduction path seems to be a bit different so I'm not sure if we're talking about the same thing. Could you please check if the same thing happens gor you after you simply focus/blur an input?
Second: we'd be grateful for a PR covering all scenarios (not only german case). If, for any reason you can't or simply don't want to contribute, please let me know so I could create internal ticket.

@tim-breitenstein-it
Copy link
Author

I implemented a solution based on:

Get the actual language based on <html lang="de">.

But I think it`s a bit overloaded for this small feature? What you think?

    let lang = document.querySelector('html').getAttribute('lang') || 'en';

    Globalize.locale(lang);
    let numberParser = Globalize.numberParser();

...
            _onInputsChange: function(event) {
...
                    var from = numberParser(this.element.find(this.options.fromInput).val());
                    var to = numberParser(this.element.find(this.options.toInput).val());

@mborkowski
Copy link

mborkowski commented Mar 18, 2021 •
edited

Yeah, I'd rather play around with toLocaleString but then we have to parseInt those values etc, so it has to work 2-way (read and write). We created internal ticket for this but I wouldn't expect this fix soonish 😞
If your fix works and you're ok with it - please leave it as customization on your side while waiting for the proper fix

@ptylek
Copy link
Contributor

ptylek commented Apr 19, 2021

@tim-breitenstein-it we prepared solution and it should be available with next release

@tim-breitenstein-it
Copy link
Author

Seems to work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mborkowski @ptylek @tim-breitenstein-it