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

ItemsRepeater crashes when using custom ItemTemplateSelector that does not implement SelectTemplateCore(item) override #2170

Closed
jtorjo opened this issue Mar 26, 2020 · 7 comments · Fixed by #2185
Labels
area-ItemsRepeater team-Controls Issue for the Controls team

Comments

@jtorjo
Copy link

jtorjo commented Mar 26, 2020

Describe the bug
I want to create a combination of treeview/listview that would behave like this: have a few roots (such as: Effects/Transitions/Text Effects). Each will expand to a few effect packs. An effect pack will show a (wrap panel) list of effects.

I wanted to implemement this as an ItemsRepeater, but no matter what I do, it crashes at startup (the app can't even start).

Steps to reproduce the bug
It could be that I'm doing something wrong, but I don't know what that would be. I've trimmed off pretty much everything, just to make it easy to debug.

<UserControl
	x:Class="gridsplittertest.browse_effects_ctrl"
	xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
	xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
	xmlns:local="using:gridsplittertest"
	xmlns:muxc="using:Microsoft.UI.Xaml.Controls"
	xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
	xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
	mc:Ignorable="d"
	d:DesignHeight="300"
	d:DesignWidth="400">
	<UserControl.DataContext>
		<local:browse_effects_view_model/>
	</UserControl.DataContext>
	<UserControl.Resources>

		<DataTemplate x:Key="et" x:DataType="local:effect_type">
			<Grid Height="10" Width="200" MaxHeight="10" MaxWidth="10">
			</Grid>
		</DataTemplate>

		<DataTemplate x:Key="ep" x:DataType="local:effect_pack">
			<Grid Height="10" Width="200" MaxHeight="10" MaxWidth="10">
			</Grid>

		</DataTemplate>

		<DataTemplate x:Key="ei" x:DataType="local:effect_info">
			<Grid></Grid>
		</DataTemplate>

		<local:effect_data_template_selector x:Key="edts" effect_type_template="{StaticResource et}" effect_pack_template="{StaticResource ep}" effect_item_template="{StaticResource ei}"
		/>

	</UserControl.Resources>
	<Grid>
		<muxc:ItemsRepeater ItemsSource="{Binding ui_effect_types}" ItemTemplate="{StaticResource edts}">
			<muxc:ItemsRepeater.Layout>
				<muxc:StackLayout Orientation="Vertical"/>
			</muxc:ItemsRepeater.Layout>
		</muxc:ItemsRepeater>
	</Grid>
</UserControl>

To my knowledge, the above should work. Am I doing something wrong?

I've attached the whole sample project I'm using.
gridsplittertest.zip

Windows 10 version Saw the problem?
November 2019 Update (18363) Yes
Device form factor Saw the problem?
Desktop Yes
@msft-github-bot msft-github-bot added the needs-triage Issue needs to be triaged by the area owners label Mar 26, 2020
@ranjeshj
Copy link
Contributor

ranjeshj commented Mar 26, 2020

@jtorjo I was able to make it work by using the other override of DataTemplateSelector. ItemsRepeater does not have a container, so it uses this override. We should be providing a better error in this case though or handle this scenario (we can use this bug to track that)

After I made the update below and changed the data templates to show something, i was able to see the items.

 protected override DataTemplate SelectTemplateCore(object item)
        {
            if (item is effect_type)
                return effect_type_template;
            if (item is effect_pack)
                return effect_type_template;
            if (item is effect_info)
                return effect_item_template;

            Debug.Assert(false);
            return null;
        }

@ranjeshj ranjeshj changed the title ItemsRepeater crashes on heterogeneous list ItemsRepeater crashes when using custom ItemTemplateSelector that does not implement SelectTemplateCore(item) override Mar 26, 2020
@ranjeshj ranjeshj added area-ItemsRepeater team-Controls Issue for the Controls team labels Mar 26, 2020
@marcelwgn
Copy link
Collaborator

Couldn't we just support the SelectTemplateCore(object item, DependencyObject container) method by passing nullptr as the container? Not supporting this method and crashing/returning an error seems a bit overkill here.

@ranjeshj
Copy link
Contributor

Couldn't we just support the SelectTemplateCore(object item, DependencyObject container) method by passing nullptr as the container? Not supporting this method and crashing/returning an error seems a bit overkill here.

We could look at one and fallback to the other. Yes.

@jtorjo
Copy link
Author

jtorjo commented Mar 26, 2020

@ranjeshj

I was able to make it work by using the other override of DataTemplateSelector. ItemsRepeater does not have a container, so it uses this override. We should be providing a better error in this case though or handle this scenario (we can use this bug to track that)

After I made the update below and changed the data templates to show something, i was able to see the items.

Awesome, thanks! Yeah, the error should be reported better - in my case, it basically stops the app from starting. There's no way to understand anything.

@StephenLPeters StephenLPeters removed the needs-triage Issue needs to be triaged by the area owners label Mar 26, 2020
@marcelwgn
Copy link
Collaborator

marcelwgn commented Mar 28, 2020

Would the following be a suitable fix:

  1. Call SelectTemplate(IInspectable item)
  2. If we received a non nullptr, use that, otherwise try SelectTemplate(IInspectable item, DependencyObject container)
  3. Check if that got us a valid pointer, if so use that
  4. If we still got a nullptr, throw some exception like "No valid template returned"

@jtorjo @ranjeshj

If that would be a fix you are fine with, I would be more than happy to implement that.

@ranjeshj
Copy link
Contributor

That sounds reasonable. We will have to pass null as the container and there could be app code that fails because of that, but that should be easier to debug from the app side.

@jtorjo
Copy link
Author

jtorjo commented Mar 28, 2020

@chingucoding Sounds perfect!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-ItemsRepeater team-Controls Issue for the Controls team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants