WindowsCommunityToolkit icon indicating copy to clipboard operation
WindowsCommunityToolkit copied to clipboard

TokenizingTextBox Text property cannot be set manually from code behind in Release mode

Open Dieveral opened this issue 1 year ago • 6 comments

Describe the bug

In Release mode TokenizingTextBox Text property cannot be set from code behind or with x:Bind. In Debug mode everything works as expected.

Regression

No response

Reproducible in sample app?

  • [X] This bug can be reproduced in the sample app.

Steps to reproduce

Link to sample app:
https://github.com/Dieveral/TokenizingTextBox_TextBug

Steps:
1. Start application in Debug mood
2. Write text in field
3. Leave field without transforming text to TokenItem
4. Press 'Clear' button.
Text disappears.
5. Exit application
6. Start application in Release mood
7. Repeat steps 2-4
Text remains.

Expected behavior

You can change Text property of TokenizingTextBox from code behind and with binding system in Release mood, as in Debug mood. In sample app: Text must disappear after pressing 'Clear' button in Release mood.

Screenshots

TokenizingTextBox TokenizingTextBox_BackEnd

Windows Build Number

  • [ ] Windows 10 1809 (Build 17763)
  • [ ] Windows 10 1903 (Build 18362)
  • [ ] Windows 10 1909 (Build 18363)
  • [ ] Windows 10 2004 (Build 19041)
  • [ ] Windows 10 20H2 (Build 19042)
  • [ ] Windows 10 21H1 (Build 19043)
  • [ ] Windows 11 21H2 (Build 22000)
  • [X] Other (specify)

Other Windows Build number

Windows 10 21H2 (Build 19044.1889)

App minimum and target SDK version

  • [X] Windows 10, version 1809 (Build 17763)
  • [ ] Windows 10, version 1903 (Build 18362)
  • [ ] Windows 10, version 1909 (Build 18363)
  • [X] Windows 10, version 2004 (Build 19041)
  • [ ] Other (specify)

Other SDK version

No response

Visual Studio Version

2019

Visual Studio Build Number

16.11.17

Device form factor

Desktop

Nuget packages

Microsoft.NETCore.UniversalWindowsPlatform v6.2.12 Microsoft.Toolkit.Uwp.UI.Controls v7.1.12

Additional context

No response

Help us help you

No.

Dieveral avatar Sep 01 '22 13:09 Dieveral

Hello Dieveral, 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 🙌

msftbot[bot] avatar Sep 01 '22 13:09 msftbot[bot]

@LalithaNadimpalli this could be a good one for you to pick up next. Maybe see if you can reproduce in a new UWP project first? At least we can see if it's a general issue or specific to WinUI 3?

@Dieveral what Windows App SDK version are you using in your app? (@Arlodotexe we probably want to add that to our issue form template too. (For both WinUI and WindowsAppSDK - do they support conditional questions based on other answers yet?))

michael-hawker avatar Sep 07 '22 20:09 michael-hawker

I am reproducing the app in UWP using TokenizingTextBox code(XAML and .cs) but I see the issue user pointing to when I run the simple sample app provided

LalithaNadimpalli avatar Sep 07 '22 20:09 LalithaNadimpalli

Thanks @LalithaNadimpalli, want to attach your reproduction project here as well?

I built this version of the control, so let me know if you have any questions in digging into what might be happening here when the text isn't being set.

michael-hawker avatar Sep 07 '22 22:09 michael-hawker

@michael-hawker I am using TokenizingTextBox control in UWP project. Target: Universal Windows Target version: Windows 10, version 2004 (Build 19041) Minimum version: Windows 10, version 1809 (Build 17763)

Dieveral avatar Sep 08 '22 13:09 Dieveral

Synced with Laltiha and we think we found the root cause.

We discovered that it looks like the Text property appears to update and is set correctly (you can see this same behavior directly in the sample app as well as the clear tokens button should clear the text as well):

image

Above the Current Edit is bound to the Text property. Then when you click clear:

image

You can see the Current Edit properly reflects that the Text is now empty.

Still not sure why this is actually working in Debug vs. Release. But basically we think it's the AutoSuggestBox component that's part of the template that's not properly getting updated to reflect the proper state of the Text value.

You can actually see this if you try and set the default text value to the control as well, like:

<controls:TokenizingTextBox Text="Some Text Here"/>

image

The Text property reflects that, but not the initial state of the AutoSuggestBox.


We'll want to do three things:

  1. Add 2 new test cases that show these two scenarios for setting the initial text and clearing and seeing the text of the inner AutoSuggestBox cleared here: https://github.com/CommunityToolkit/WindowsCommunityToolkit/blob/main/UnitTests/UnitTests.UWP/UI/Controls/Test_TokenizingTextBox_General.cs

  2. Update the initialization to get set the AutoSuggestBox text to the value of the token text here:

https://github.com/CommunityToolkit/WindowsCommunityToolkit/blob/fdaef4750236713bd788f4c1d6162a4ea5959242/Microsoft.Toolkit.Uwp.UI.Controls.Input/TokenizingTextBox/TokenizingTextBoxItem.AutoSuggestBox.cs#L103-L126

  1. Update the TextPropertyChanged event to call a method on the last token item to tell it to reflect the changed text:

https://github.com/CommunityToolkit/WindowsCommunityToolkit/blob/fdaef4750236713bd788f4c1d6162a4ea5959242/Microsoft.Toolkit.Uwp.UI.Controls.Input/TokenizingTextBox/TokenizingTextBox.Properties.cs#L107-L113

Example of getting the last token item from focus event here:

https://github.com/CommunityToolkit/WindowsCommunityToolkit/blob/fdaef4750236713bd788f4c1d6162a4ea5959242/Microsoft.Toolkit.Uwp.UI.Controls.Input/TokenizingTextBox/TokenizingTextBox.cs#L136

That should hopefully resolve all these scenarios for this component.

Potentially in the future we could see if we could do this better with some sort of binding like we setup for the QueryIcon, but not sure what ramifications that has on the other tokenizing logic, so for now this seems like the most straight-forward fix without impacting a lot of the other parts of the control.

michael-hawker avatar Sep 13 '22 21:09 michael-hawker

Split up a test case and added two new ones:

image

Above is debug and we can see clearly the scenario not working for clearing the text.

And when run in Release, we can see...~the issue with setting the text initially as well (this works when in Debug for whatever reason)~:

image

Huh, the initial text scenario is working in the test... hmm. Will dig into it more as I work on fixing the other side.

(Could have something to do with initialization timing when loading directly vs. how the sample app processes?)

michael-hawker avatar Oct 11 '22 23:10 michael-hawker

Sorted out the test:

image

There was a rogue binding which should have been meaningless, and wasn't working as expected, but also somehow causing the test to pass even though the correct behavior wasn't working when tested in the sample app.

Removed the binding and now the test fails as expected. Going to work to adding the proposed fixes in now and then can submit a PR.

Did notice an issue when hitting the clear button in the textbox itself after typing on an existing token, but hope that'll be resolved as well, that'd be harder to test in the current setup.

michael-hawker avatar Oct 17 '22 21:10 michael-hawker

Close to fixing behavior, trying to test some other scenarios, and may have introduced a new bug. May have to try writing some UI tests here, looks like the first character received is being eaten now maybe.

  1. Add tokens to box
  2. Select first one with keyboard
  3. Type key, should go in textbox and start new dropdown for autosuggest, but currently not (though setting it in the Text field...)

michael-hawker avatar Oct 17 '22 23:10 michael-hawker

Figured out the issue, was only setting the initial text for the last box. Should be resolved now. Not sure how to go about testing that yet, may need to beef up tests there in 8.0 world.

michael-hawker avatar Oct 17 '22 23:10 michael-hawker