XamarinCommunityToolkit

XamarinCommunityToolkit copied to clipboard

Description of Change

Fixes RemoveMask method. Originally it wasn't taking into account the position of the user's input and removing it if it was part of the mask.

Bugs Fixed

• Fixes #1276

API Changes

It now compares the char position to the mask position.

Behavioral Changes

None that I can think of, all other samples work as before.

PR Checklist

• [x] Has a linked Issue, and the Issue has been approved
• [x] Has tests (if omitted, state reason in description)
• [x] Has samples (if omitted, state reason in description)
• [ ] Rebased on top of main at time of PR
• [ ] Changes adhere to coding standard
• [ ] Updated documentation

@IeuanWalker thanks for the quick fix! Your code doesn't allow me to type 1 as the first number. Can you review it?

Also, will be great if you add unit tests for it

@pictos you can now enter 1, but it does count as the masked character, i.e. text would become +1 where as if you typed 8 for example it wouldn't, i.e. text value would become +1 (8

Atm not sure how to get around that. Let me know what you think.

Will look at adding some tests.

Ping us when you think it's done for another review @IeuanWalker

@jfversluis @pictos @AndreiMisiukevich

Think this is ready for review.

One thing to note, the unit test behaves differently than the sample app (it's what I mentioned in my previous comment)

This is the test -

[Test]
public void NumbersInMaskShouldBeAllowed()
{
	// arrange
	var entry = CreateEntry("+1 (AAA) AAA-AAAA", 'A');

	// act
	entry.Text = "12345";

	// assert
	Assert.IsTrue(entry.Text.Equals("+1 (123) 45"));
}

But when running the sample app this is the outcome - +1 (234) 5. I can't see why this is happening, but this PR does fix the issue raised + doesn't change any existing behaviour, so I think it's ok.

@IeuanWalker I added a unitTest that reproduces the error, maybe that helps. I'll try to find some solution for this, but if we can't the best is to say that we don't have support for such a feature.

Seems like the build is failing atm

Could you rebase to fix the merge conflicts?

@AndreiMisiukevich sorry not quite sure how to rebase

Now that we're so close to the sunsetting of Xamarin (and therefore also Xamarin Community Toolkit) unfortunately we won't be able to take this in anymore, we're really sorry about that. Nevertheless, thank you so much for your time and effort that you have put into this PR, we really appreciate it.

Please have a look at the evolution of the Xamarin Community Toolkit, The .NET MAUI Community Toolkit. A lot of development has been going on there. Hopefully this issue was already fixed in that codebase. If not, feel free to port this over to there.

Again, thank you so much for being a contributor and Xamarin user!