magesuite / theme-creativeshop

Open Software License 3.0
38 stars 24 forks source link

Price slider incorrect format in german locale store #59

Open tim-breitenstein-it opened 3 years ago

tim-breitenstein-it commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

tim-breitenstein-it commented 3 years ago

Seems to work!