TokenizingTextBox exposes InterspersedObservableCollection instead of ObservableCollection

[michael-hawker]

Describe the bug

The original intent of the TokenizingTextBox was to not expose the internal collection type of InterspersedObservableCollection, and by providing an ObservableCollection to the ItemsSource property, a developer could retrieve only the tokenized elements from the control for their datamodel (or provide them as a pre-population/rehydration on a form).

A developer could then access the Text or raw Items properties to enumerate all partial strings with the Tokens if needed.

However, in the current implementation, we overwrite the ItemsSource value with our InterspersedObservableCollection reference here:

WindowsCommunityToolkit/Microsoft.Toolkit.Uwp.UI.Controls.Input/TokenizingTextBox/TokenizingTextBox.cs

Line 62 in 03c3f3c

ItemsSource = _innerItemsSource;

And more importantly here:

WindowsCommunityToolkit/Microsoft.Toolkit.Uwp.UI.Controls.Input/TokenizingTextBox/TokenizingTextBox.cs

Line 94 in 03c3f3c

ItemsSource = _innerItemsSource;

This means if you try to setup the scenario as we were in the current Sample app or Graph Sample App (see CommunityToolkit/Graph-Controls#149) binding to the ItemsSource property of the TTB directly, you'd get the internal copy of the InterspersedObservableCollection instead of the inner ObservableCollection of only tokened items.

Steps to Reproduce

• Can this be reproduced in the Sample App? (Either in a sample as-is or with new XAML pasted in the editor.) If so, please provide custom XAML or steps to reproduce. If not, let us know why it can't be reproduced (e.g. more complex setup, environment, dependencies, etc...)

We can actually observe this in our own sample app (but it was invisible) so we missed it during initial development.

Steps to reproduce the behavior:

1. Open the TTB page
2. Go to the Live Visual Tree tools and select the "Current Edit" text
3. In the Live Visual Tree find the ItemsControl next that's supposed to show the tokenized items.
4. Note that there'll be a ContentPresenter showing a empty TextBlock, this is a PretokenStingContainer element which shouldn't be part of our collection (only ever exposed on Items and for ToString to get value).

We should expect the ItemsControl to be empty at this time.

We can also further show this corruption by performing a few steps.

5. Add any two elements and some text:

6. Click on the first token
7. Type some visible characters, note now that we see the first letter that was set to a PretokenStringContainer within our collection 😲

This should not be the case!

Expected behavior

Binding to the ItemsSource even if we don't provide an external collection should only provide the tokenized items. We should only be able to observe the PretokenStringContainer within the Items collection which a developer would check for their own type or call ToString to get the raw values of typed text.

Screenshots

Environment

NuGet Package(s): Input

Package Version(s): 7.1.0-rc1

Windows 10 Build Number:

• Fall Creators Update (16299)
• April 2018 Update (17134)
• October 2018 Update (17763)
• May 2019 Update (18362)
• May 2020 Update (19041)
• Insider Build ({build_number})

App min and target version:

• Fall Creators Update (16299)
• April 2018 Update (17134)
• October 2018 Update (17763)
• May 2019 Update (18362)
• May 2020 Update (19041)
• Insider Build ({build_number})

Device form factor:

• Desktop
• Xbox
• Surface Hub
• IoT

Visual Studio version:

• 2017 (15.{minor_version})
• 2019 (16.{minor_version})
• 2022 (17.{minor_version})

Additional context

Updated the Graph Sample for now to avoid this issue and show the more practical pattern here: CommunityToolkit/Graph-Controls#160

Should still resolve this issue here.

I believe we did this the way we did initially as since we're inheriting from ListViewBase, we use ItemsPresenter to present the items here:

WindowsCommunityToolkit/Microsoft.Toolkit.Uwp.UI.Controls.Input/TokenizingTextBox/TokenizingTextBox.xaml

Lines 111 to 112 in 03c3f3c

	<ItemsPresenter Padding="{TemplateBinding Padding}"
	Margin="{StaticResource TokenizingTextBoxPresenterMargin}"/>

ItemsPresenter grabs the ItemsSource automatically... it's not something we can set. I'm not sure how we override it either, so this is going to need some thought... 🤔

[michael-hawker]

I've at least started on a fix for the sample itself to work-around the issue here: d528479

I think it's going to be tricky to resolve out-right though as ItemsSource is automatically picked up by the ItemsPresenter, so we need it to be the full collection vs. what we want exposed through the property externally...

I'm not sure what alternatives we may have here to work around this. I did just try to swap to an ItemsControl, but that crashed in the XAML layer somewhere for unknown reasons...

❓

Thinking due to the complexity here (and that it took us a year+ to hit ourselves) that we should move out of 7.1.