Improve handling of autocomplete values in X-Mask

[robertmarney]

Closes #4252

Description:

This change improves the handling of potentially valid autocomplete values, where we were seeing valid values being stripped.

Also adds more test cases for the jest tests for mask to aide future changes.

Implementation:

• Added additional logical checks for two cases:
  1.If we have a match of input length to the template length we take a different approach to applying the mask (simpler)

2. If the input length matches the wildcard template we also try to short cut application, after testing the inputs if the input matches the regex we will return early.

• To support this without breaking $money which is a dynamic template I was forced to update the money functions to utilize the unicode symbol for currency ¤ and skip the new autocomplete loop when we encounter this symbol.

In summary if we have a perfect template match or a perfect wildcard template match to the provided input we try to take a different execution path to not drop / mangle input, if we do not then we follow the existing logic.

Note: I am not overly satisfied with my mechanism for deciding whether we have a money replacement (and to skip the new functionality) but I could not find a different way without a much bigger refactor.

Edit: Following @ekwoka's refactor:

1. Refactor formatInput to combine / simplify the previously separate functionality of tearDown and buildUp this dramatically simplifies the code and appears to deliver the previous spec and resolves the bug in autocomplete.
2. Removes the old formatMoney assertions from the jest tests (did not work as formatMoney was not exported) and added a number of new jest test cases, adds a few more cypress assertions to target autocomplete functionality, specifically where there was overlap in characters between the template and the input (which is presently being mangled)

Potential Breaking Change

See @ekwoka's excellent writeup: there is a change in behavior to what the user will see when the user has only completed part of the input (specifically, if the next character is part of the mask, it will not pre-apply) 👀 #4260 (comment)

Testing:

I added a number of cases to the jest test to validate/demonstrate the functionality in different cases.

[ekwoka]

Looking at this, I'm really questioning why there even is strip down and build up as separate things... 🤔

I think we can solve these issues with less code instead of more...

This change improves the handling of potentially valid autocomplete values, where we were seeing valid values being stripped.

Examples of such values and masks and what the conflict is?

[SimoTod]

won't any dynamic function have the same issues as money? i'm not sure if the additional wildcard is the best way to address it.

[ekwoka]

I opened a PR: robertmarney#1 that removes a lot of the code and improved performance and fixes some other behaviors. I think it demonstrates a better approach to work from.

[robertmarney]

Thanks @ekwoka - Apologies for the delay, I have been bed ridden for a few days.

I took your refactor and updated the Jest tests and added a few more cypress assertions to validate the original complaint was resolved (autocomplete of the value side of the mask was being misapplied).

In the PR you mentioned there was a change in behavior in some cases? What were you seeing?

won't any dynamic function have the same issues as money? i'm not sure if the additional wildcard is the best way to address it.

@SimoTod - With @ekwoka's refactor, this is no longer an issue. For posterity, I agree, the wildcard route seemed hacky, I was afraid of proposing such a large refactor (removal of the stripdown / buildup lifecycle) due to my lack of context with the alpine code base.

[ekwoka]

In the PR you mentioned there was a change in behavior in some cases? What were you seeing?

right, so the change in behavior is the eager vs lazy injection of non-wildcard values.

like take a bad date mask 99/99/99 if the user types 12, the old behavior would eagerly add / making the input be 12/ while this change (as written) would not, leaving 12 until the next is written, so once 123 is typed, it would mask to 12/3

This solves an issue regarding formatting on backspace. Before, to allow the user to remove the eagerly injected / the mask just skipped masking when the new input was 1 less than the previous.

When the characters are lazily added, that ceases to be an issue, since the algo doesn't have an opinion on if the / is there or not.

this then allows formatting on backspace.

so for the $money mask, if the user types 123456 both would have the input 123,456. But if the user then hit backspace, the old behavior would be to show 123,45 while this change makes it 12,345.

I would say this is the better behavior.

I just call it out as it is DIFFERENT behavior. Some of the tests did test for this specifically (and needed to be changed) so I don't know how actually important that becomes, or how intentional it even was in the first place.

So that could be a breaking change. It's not difficult to just append any sequential non-wildcards to the end after the main masking, though.

I prefer this new behavior personally since it simplifies a lot of the interactions.

[calebporzio]

Ok, I've finally had time to review this more deeply.

Overview of my thoughts:
A) I love the code simplification, great work!
B) I don't think this PR actually solves the original problem
C) I'm on the fence about changing from eagerly adding template literals to lazily adding them

Let me start with B. The original problem raised in the discussion is that when pasting in values that start with a character that is a literal part of the template, it get's "consumed" and therefore a value like a phone number may not be fully pasted in.

I recorded a gif to demonstrate that with this branch, the problem still persists:

CleanShot.2024-07-03.at.13.07.30.mp4

I saw in the discussion that this may actually be considered intended behavior and I think I agree. So I'm assuming that's why it's not fixed in this PR, but I think it's important to at least note that this doesn't appear to fix any autocomplete behavior.

It also doesn't appear to solve the money formatting on backspace as mentioned earlier. Here's a gif of this branch and backspace NOT formatting money as you go:

Now on to C: The eagerness vs laziness of the algo.

Here is a gif demonstrating the original "eager" behavior:

Now here's one demonstrating the new "lazy" behavior contained in this PR:

Here's my summary of the points of the debate as I see them:

Eager

• Pro: You see a preview of the formatting as you type. If you type 716, you will see (716) and know that you are done with the aria code part of a telephone number.
• Pro: If the template ends in a literal, for example (999) ends in ), when you type 716, the input will show the final and full (716). Where as if it were lazy it would not and you would only see (716 with no trailing )

Lazy

• Pro: If formatting on backspace is unlocked by this, that's probably better?
• Pro: If you aren't required to fully complete the mask template, like 99/99/9999, you could type 0826 and it would display 08/26 instead of 08/26/ - which feels less intrusive and more flexible
• Pro: if it enables a much more maintainable algo/codebase then that's a big pro

---

Thoughts?

[ekwoka]

On C, and the backspace thing, I guess a change I proposed in a PR to the branch was not carried over (or somehow got lost along the way)

alpine/packages/mask/src/index.js

Lines 71 to 74 in c574bef

	// If they hit backspace, don't process input.
	if (lastInputValue.length - el.value.length === 1) {
	return lastInputValue = el.value
	}

Those lines would serve no purpose in the lazy version.

Overall I'm in favor of lazy, especially in how it does just stop some problems from happening (like the backspace issue), where-as eager makes that now be something you have to handle specifically.

And yes, it does address the initial issue. I think that issue might be too specific or unlikely, or even unmanageable. How could one know that content in the input is from the mask or manually added? The only way would be to actively maintain the "real" input behind the scenes, and modify then and then mask to the value. Otherwise it's just impossible to know.

The specific scenario of a US phone number sort of just doesn't matter, since no area code starts with a 1, but maybe there are other scenarios where it is more of a real conflict.

I think the one issue of template ends in literal to maybe be handleable on a different event, like blur/change instead of the input event. Since that indicates more of a user commit. If it's considered to be worth handling.

[ekwoka]

For some performance reference.

For pretty normal sized masked (dates/phone numbers, addresses) this new way processes about 1.75-2x faster.

For some so long nobody in their right mind would actually have such a mask its 8-13x faster.

But realistically, this doesn't matter since this should only ever be firing in isolation, since it doesn't run on initialization.