Skip to content

TextBoxMask Extension Binding Text Issue #3335

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.

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

Closed
4 of 5 tasks
rizamarhaban opened this issue Jun 8, 2020 · 12 comments · Fixed by #3338
Closed
4 of 5 tasks

TextBoxMask Extension Binding Text Issue #3335

rizamarhaban opened this issue Jun 8, 2020 · 12 comments · Fixed by #3338
Labels
bug 🐛 An unexpected issue that highlights incorrect behavior Completed 🔥
Milestone

Comments

@rizamarhaban
Copy link
Contributor

rizamarhaban commented Jun 8, 2020

Describe the bug

Applying TextBoxMask on TextBox control is working. However, when the binding value changes, the TextBox UI did not reflect the changes.

  • Is this bug a regression in the toolkit? If so, what toolkit version did you last see it work:

Steps to Reproduce

Steps to reproduce the behavior (I'm using PRISM 6.x):

  1. Add these XAML,
<TextBox
	Width="120"
	Padding="0,4,0,0"
	VerticalAlignment="Center"
	extensions:TextBoxMask.CustomMask="5:[0-5]"
	extensions:TextBoxMask.Mask="99:59:59"
	extensions:TextBoxMask.PlaceHolder="--:--:--"
	Header="Opening Time"
	Text="{x:Bind ViewModel.OpeningTime, Mode=TwoWay}"
	TextAlignment="Center" />
<TextBox
	Width="120"
	Padding="0,4,0,0"
	VerticalAlignment="Center"
	extensions:TextBoxMask.CustomMask="5:[0-5]"
	extensions:TextBoxMask.Mask="99:59:59"
	extensions:TextBoxMask.PlaceHolder="--:--:--"
	Header="Closing Time"
	Text="{x:Bind ViewModel.ClosingTime, Mode=TwoWay}"
	TextAlignment="Center" />
  1. Add the property binding in ViewModel,
private string _openingTime =  "00:00:00";
private string _closingTime = "23:59:59"; 
private ICommand _resetToDefaultCommand;

public string OpeningTime
{
    get { return _openingTime; }
    set { SetProperty(ref _openingTime, value); }
}

public string ClosingTime
{
    get { return _closingTime; }
    set { SetProperty(ref _closingTime, value); }
}

public ICommand ResetToDefaultCommand => _resetToDefaultCommand ??
    (_resetToDefaultCommand = new DelegateCommand(() =>
{
        OpeningTime = "00:00:00";
        ClosingTime = "23:59:59";
    }));

When Reset Button clicked, the TextBox did not update the UI. However, if we delete one digit at the back or delete any digit, the value updated.

Expected behavior

When ResetToDefault Button clicked:
OpeningTime should be showing: 00:00:00
ClosingTime should be showing: 23:59:59

Screenshots

scenario
As we can see here, if the value entered following the mask correctly, and then we update the value via binding, the TextBox did not properly reflect the changes. However, if one of the digit is missing, the value is updated perfectly.

Environment

NuGet Package(s):

Package Version(s):

Windows 10 Build Number:

  • Version 1909 (18363.778)

App min and target version:

  • Version 1909 (18363.778)

Device form factor:

  • Desktop

Visual Studio

  • 2019 Preview (version: 16.7.0 Preview 2.0)

Additional context

Add any other context about the problem here.

@rizamarhaban rizamarhaban added the bug 🐛 An unexpected issue that highlights incorrect behavior label Jun 8, 2020
@ghost ghost added the needs triage 🔍 label Jun 8, 2020
@ghost
Copy link

ghost commented Jun 8, 2020

Hello rizamarhaban, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌

@Kyaa-dost
Copy link
Contributor

@rizamarhaban Thanks for highlighting the bug. Is this something you can work on, create a PR ?

@Kyaa-dost Kyaa-dost added help wanted Issues identified as good community contribution opportunities and removed needs triage 🔍 labels Jun 8, 2020
@rizamarhaban
Copy link
Contributor Author

Okay, will check the code first.

@rizamarhaban
Copy link
Contributor Author

@Kyaa-dost I found the code. It seems the correct TextBox.Text was overridden by the oldtext. The binding works normally but was overridden. Will try to fix this and create PR later.

image

@rizamarhaban
Copy link
Contributor Author

rizamarhaban commented Jun 9, 2020

@Kyaa-dost I have fixed the code however I cannot push my branch or create the PR. Need help on this.

@Kyaa-dost Kyaa-dost removed the help wanted Issues identified as good community contribution opportunities label Jun 9, 2020
@michael-hawker
Copy link
Member

@rizamarhaban sounds like you just cloned the main repo. If you create a fork in GitHub:

image

You should be able to change your remote to your new fork, then you can push your feature branch to your fork, and then submit a PR to our main repo here with the fix!

Let us know if you need more help submitting the PR! Thanks!

@rizamarhaban
Copy link
Contributor Author

rizamarhaban commented Jun 10, 2020

@michael-hawker I'm getting this failed test when running the unit tests. Is this something that I should leave it? it seems has no relation with my simple changes. Other tests passed.

image

@michael-hawker
Copy link
Member

@rizamarhaban looks like the tests passed in the CI, but adding @Sergio0694 here for help with understanding why they might fail locally.

@michael-hawker michael-hawker added this to the 7.0 milestone Jun 10, 2020
@Sergio0694
Copy link
Member

Mmmh those crashes are very weird, also I'm pretty sure those FileNotFound are due to some referenced package not being loaded correctly. @michael-hawker have some NuGet packages been updated in the 7.0 branch? I see the CI is running all tests just fine in master 🤔

@rizamarhaban
Copy link
Contributor Author

@Sergio0694 @michael-hawker maybe this detail will help to resolve this,

image

My laptop has 32GB memory,

image

@michael-hawker
Copy link
Member

@rizamarhaban thanks, we've found the issue and it will be resolved by #3365. So, don't worry about those tests failing locally, you should be all good, thanks!

I also was able to repro similar issues with just XAML directly here in the sample app:

                <TextBox Grid.Row="3"
                x:Name="TestText"
                         Text="10:20:30"
                         Header="Source" 
                         Style="{StaticResource MaskedTextBoxStyle}"/>

                <TextBox Grid.Row="4"
                         extensions:TextBoxMask.CustomMask="5:[0-5]"
                         extensions:TextBoxMask.Mask="99:59:59"
                         Header="Target"
                         Text="{Binding Text, ElementName=TestText, Mode=TwoWay}"
                         Style="{StaticResource MaskedTextBoxStyle}" />

Taking a look at your PR suggested fix now, thanks!

@Sergio0694
Copy link
Member

@michael-hawker Actually the second issue, the OutOfMemoryException from that HighPerformance test, has already been solved (that was in #3350), so if that has been merged into this PR I think it should already be fixed in here as well 👍

@ghost ghost closed this as completed in #3338 Oct 6, 2020
ghost pushed a commit that referenced this issue Oct 6, 2020
## Fixes #3335

When using the TextBoxMask extension, the TextBox text not updated if the text with the same string length changes, unless if the string length is less than the current Text length.

## PR Type
What kind of change does this PR introduce?
<!-- Please uncomment one or more that apply to this PR. -->

- Bugfix
<!-- - Feature -->
<!-- - Code style update (formatting) -->
<!-- - Refactoring (no functional changes, no api changes) -->
<!-- - Build or CI related changes -->
<!-- - Documentation content changes -->
<!-- - Sample app changes -->
<!-- - Other... Please describe: -->


## What is the current behavior?
Example, a TextBox with Mask `99:59:59` where cutom mask 5:[0-5], if the current Text is 10:20:30 and we update with 20:30:40, the Text did not changed/updated. The UI will show the same '10:20:30' because of this code: `textbox.Text = oldText;`

## What is the new behavior?
Example, if the current Text is 10:20:30 and we update with 20:30:40, the Text changed/updated. The fix is to let the text updated on this regards and no other behavior changes.


## PR Checklist

Please check if your PR fulfills the following requirements:

- [ ] Tested code with current [supported SDKs](../readme.md#supported)
- [ ] Pull Request has been submitted to the documentation repository [instructions](..\contributing.md#docs). Link: <!-- docs PR link -->
- [ ] Sample in sample app has been added / updated (for bug fixes / features)
    - [ ] Icon has been created (if new sample) following the [Thumbnail Style Guide and templates](https://github.com/windows-toolkit/WindowsCommunityToolkit-design-assets)
- [ ] Tests for the changes have been added (for bug fixes / features) (if applicable)
- [ ] Header has been added to all new source files (run *build/UpdateHeaders.bat*)
- [X] Contains **NO** breaking changes

<!-- If this PR contains a breaking change, please describe the impact and migration path for existing applications below. 
     Please note that breaking changes are likely to be rejected. -->


## Other information
@ghost ghost added Completed 🔥 and removed In-PR 🚀 labels Oct 6, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 5, 2020
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug 🐛 An unexpected issue that highlights incorrect behavior Completed 🔥
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants