taiga-family / maskito

Collection of libraries to create an input mask which ensures that user types value according to predefined format.
https://maskito.dev
Apache License 2.0
1.36k stars 30 forks source link

🐞 - New built-in plugin `maskitoChangeEventPlugin` (to dispatch native `change` event) #1329

Closed robkuz closed 2 months ago

robkuz commented 2 months ago

Which package(s) are the source of the bug?

@maskito/core, @maskito/kit

Playground Link

https://um2sal.stackblitz.io/

Description

I need to enter at least 4 digits + decimal seperator in order to have the onchange function triggered.

If you enter for example ",1" ",12" "0,12" "1,22"

The onchange function is not triggered

Only if I enter

"12,34" "12," => which gets transformed into "12,00" "01,00" "01," => which gets transformed into "1,00"

the on change function is triggered. Also deleting every digit will trigger the onchange.

Maskito version

latest

Which browsers have you used?

Which operating systems have you used?

nsbarsukov commented 2 months ago

Native onChange (not react's synthetic!) event is triggered only when

When the element loses focus after its value was changed (c) MDN

Use native onInput instead!

Learn more about all textfield's events in my article: https://medium.com/javascript-in-plain-english/uphill-battle-to-create-a-mask-for-text-field-f1e385455dd5

nsbarsukov commented 2 months ago

Also read section "Controlled masked input" from this documentation page: https://maskito.dev/frameworks/react

robkuz commented 2 months ago

Native onChange (not react's synthetic!) event is triggered only when

When the element loses focus after its value was changed (c) MDN

Use native onInput instead!

Sorry, I was unclear. onchange should indeed be triggered after input changes + the element looses focus. And that doesnt happen if not at least 4 digits

robkuz commented 2 months ago

to be more clear. Use the stackblitz above. enter "1,23" and tab out of the input. This should imo trigger onchange but it doesnt. If however I type "01,23" and then tab out of the input the event is fired

waterplea commented 2 months ago

Hi @robkuz. Seems like native change event is not dispatched if we prevent default behavior of beforeinput event, which we have to do to properly process the entered value in some cases. To be honest, I haven't seen change event used for years, ever since input event became available in all browsers over a decade ago. This could be augmented by a custom Maskito plugin that would dispatch synthetic change event upon blur if since focus there were only synthetic input events (isTrusted: false). I'm not really sure it is necessary for common use, though. Could you share with us why do you need change event as opposed to input?

robkuz commented 2 months ago

Hi @waterplea,

actualle the input event is much to finegranular for my needs. Which is check upon loosing focus (blur) if the value of the control has changed. I can not use just blur because blur doesnt tell me if the value has changed- In my particular case I will call a server method if and only if the value has changed and there is no chance of further editing because the control has lost focus. and that is exactly what the onchange event delivers. I have been using this since the dawn of time.

Apart from that - the behaviour I am describing above is a bug and would at least be a candidate for a fat disclaimer in the docs. Something like "Due to ... (technical description) ... the delivery of the onchange event can not bee guaranteed when using moskito"

waterplea commented 2 months ago

Ok, thanks for clarifying. change event is fired right before blur. We can easily add a post-blur synthetic change if the value has changed and change event did not fire. Still, I think it should be a plugin, rather than built-in behavior because I think most people would not need it and I would like to keep synthetic implicit behavior to a minimum. We definitely should add a disclaimer with the name of plugin, something like:

Native `beforeinput` event default behavior is cancelled to process user entered value.
This causes native `change` event to NOT be dispatched by browser. 
A `change` event, as opposed to `input`, is triggered only when user left the field and value was changed during interaction.
If you rely on this behavior, add `maskitoChangeEventPlugin` to your mask configuration.
It will dispatch synthetic `change` event using the same logic.
robkuz commented 2 months ago

Hi @waterplea,

first of all - thanks for your swift replies. Thats awesome.

However I'd like to point out the following - as per my description above the onchange event does get fired. Its only that it doesnt happen always.

Entering:

"12,34" fires the event "1,23" doesnt fire the event "01,23" does fire it

I am pointing that out as your description indicates that the event is NEVER dispathced by the browser. And there might be a chance that the plugin then leads to situations where the event is fired twice.

Sorry for being a pita on this

waterplea commented 2 months ago

No worries, not pita at all. The event is only cancelled if we cancel beforeinput which we do if it will lead to an invalid value, so that we can conform it to mask and never have wrong content inside the input. We will take care of that and only dispatch the event if native event did not happen. Just in case you might need to write your own plugins in future, you can check out the PR attached to this issue, it's pretty straightforward:

export function maskitoChangeEventPlugin(): MaskitoPlugin {
    return element => {
        if (element.isContentEditable) {
            return;
        }

        let value = element.value;

        const valueListener = (): void => {
            value = element.value;
        };
        const blurListener = (): void => {
            if (element.value !== value) {
                element.dispatchEvent(new Event('change', {bubbles: true}));
            }
        };

        element.addEventListener('focus', valueListener);
        element.addEventListener('change', valueListener);
        element.addEventListener('blur', blurListener);

        return () => {
            element.removeEventListener('focus', valueListener);
            element.removeEventListener('change', valueListener);
            element.removeEventListener('blur', blurListener);
        };
    };
}

Basically it's a function that gets the native element and returns cleanup logic or nothing. As you can see, native change event that can happen right before blur will updated stored value so the synthetic event will not be dispatched.