Skip to content
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

Unhandled exception in PeoplePicker sample #149

Closed
1 task
shweaver-MSFT opened this issue Aug 31, 2021 · 6 comments · Fixed by #160
Closed
1 task

Unhandled exception in PeoplePicker sample #149

shweaver-MSFT opened this issue Aug 31, 2021 · 6 comments · Fixed by #160
Assignees
Labels
bug 🐛 Something isn't working Completed 🔥
Milestone

Comments

@shweaver-MSFT
Copy link
Member

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

image

@shweaver-MSFT shweaver-MSFT added the bug 🐛 Something isn't working label Aug 31, 2021
@ghost ghost added the needs triage 🔍 label Aug 31, 2021
@ghost
Copy link

ghost commented Aug 31, 2021

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
Copy link
Member Author

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.

@shweaver-MSFT shweaver-MSFT added bug 🐛 Something isn't working and removed bug 🐛 Something isn't working needs triage 🔍 labels Sep 14, 2021
@michael-hawker
Copy link
Member

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 michael-hawker self-assigned this Sep 14, 2021
@michael-hawker
Copy link
Member

michael-hawker commented Sep 14, 2021

@shweaver-MSFT have you seen this issue before?

image

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
Copy link
Member

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:

image

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 added a commit that referenced this issue Sep 14, 2021
We re-write the sample to use an external collection as a developer would in their app. This then properly lets the ItemsControl display just the tokenized items. (Though there's still an issue with how TokenizingTextBox behaves we should resolve.)
@ghost ghost added the In-PR 🚀 label Sep 14, 2021
@michael-hawker
Copy link
Member

Was able to reproduce this in the main Sample App as well (though not to a crashing extent, but a visible one): CommunityToolkit/WindowsCommunityToolkit#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.

shweaver-MSFT pushed a commit that referenced this issue Sep 15, 2021
We re-write the sample to use an external collection as a developer would in their app. This then properly lets the ItemsControl display just the tokenized items. (Though there's still an issue with how TokenizingTextBox behaves we should resolve.)
@ghost ghost added Completed 🔥 and removed In-PR 🚀 labels Sep 15, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Nov 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug 🐛 Something isn't working Completed 🔥
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants