Unhandled exception in PeoplePicker sample

[shweaver-MSFT]

Describe the bug

A clear and concise description of what the bug is.

• [ ] 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:

1. Launch the SampleTest app
2. Navigate to the PeoplePicker sample
3. See error

Expected behavior

No exception should occur.

Screenshots

[ghost]

Hello shweaver-MSFT, 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 🙌

[shweaver-MSFT]

After some investigation, I've determined that this exception is being caused by the PeoplePickerSample:

<ItemsControl Margin="8,0,0,0" ItemsSource="{x:Bind PeopleChooser.ItemsSource}">
    <ItemsControl.ItemTemplate>
        <DataTemplate x:DataType="graph:Person">
            <TextBlock Text="{x:Bind DisplayName}" />
        </DataTemplate>
    </ItemsControl.ItemTemplate>
</ItemsControl>

The problem is that the underlying TokenizingTextBox (TTB) is initializing its ItemSource with a default input item (AKA PretokenStringContainer). So on load, the ItemsSource has one non-token item. This is causing the exception, because the DataTemplate is expecting a DataType of graph:Person.

That said, I believe that there is still a bug in the underlying TokenizingTextBox because I would expect the ItemsSource property to exclude any non-token items. Retrieving ALL items should be done through the Items property.

Fixing the bug in TokenizingTextBox should resolve this issue as well.

[michael-hawker]

Thanks for the chat @shweaver-MSFT, I'm taking a look on my dev stream and will see what's going on and write some tests. 🙂

[michael-hawker]

@shweaver-MSFT have you seen this issue before?

System.InvalidOperationException
  HResult=0x80131509
  Message=The requested operation requires an element of type 'Object', but the target element has type 'Array'.
  Source=System.Text.Json
  StackTrace:
   at System.Text.Json.JsonDocument.TryGetNamedPropertyValue(Int32 index, ReadOnlySpan`1 propertyName, JsonElement& value)
>   [Async] CommunityToolkit.Graph.dll!CommunityToolkit.Graph.Extensions.GraphExtensions.FindUserAsync(Microsoft.Graph.GraphServiceClient graph, string query) Line 59  C#

For context, I tweaked the sample to provide its own ObservableCollection as a developer would (instead of pointing to the PeoplePicker directly):

<controls:PeoplePicker x:Name="PeopleChooser"
                       ItemsSource="{x:Bind MyPeople}" />
<TextBlock Margin="0,8,0,0"
           FontWeight="Bold">
    Picked People:
</TextBlock>
<ItemsControl Margin="8,0,0,0"
              ItemsSource="{x:Bind MyPeople}">

To see if that resolves the issue in the sample, should still be able to do what we had, but at least we mitigate the graph sample for now showing a way it would more likely be used in an app.

[michael-hawker]

Ah, nevermind, I didn't have "Enable Just My Code on" it's some internal graph exception which maybe is expected? As it works now:

I'll submit a PR to this repo for now and then continue to investigate the issue in the TokenizingTextBox. It's definitely related to us overwriting the ItemsSource here. We should leave the original collection as the bound ItemsSource. We just didn't catch this before.

So, in this case since we bind to the control's ItemsSource (instead of an external one), we're getting the wrapped Interspersed collection variant which then doesn't behave as we expect.

[michael-hawker]

Was able to reproduce this in the main Sample App as well (though not to a crashing extent, but a visible one): https://github.com/CommunityToolkit/WindowsCommunityToolkit/issues/4248

It's always been there 🙁, but mostly invisible. Which is how we missed it.

I think I have a straight-forward solution??? Going to write some unit tests and go from there first though to ensure we have a proper fix.