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

Using a TabbedPage for you root view will add tabs multiple times #432

Closed
johot opened this Issue May 7, 2017 · 3 comments

Comments

Projects
None yet
2 participants
@johot

johot commented May 7, 2017

Update: Might have found the bug

It seems InitializeComponent(); gets called twice for ViewLocator.LocateForModel because of the following code:

public static void InitializeComponent(object element) {
#if !WinRT && !XFORMS
            var method = element.GetType()
                .GetMethod("InitializeComponent", BindingFlags.Public | BindingFlags.Instance);
#else
            var method = element.GetType().GetTypeInfo()
                .GetDeclaredMethods("InitializeComponent")
                .SingleOrDefault(m => m.GetParameters().Length == 0);
#endif

            if (method == null)
                return;

            method.Invoke(element, null);
        }

Since a default implementation using xaml + code behind has a call to InitializeComponent() in it's constructor I believe maybe we should simply let the code behind file call it?

Original description

I used the default Caliburn.Micro sample with Xamarin.Forms and modified my HomeView.xaml file to use a tabbed page:

<?xml version="1.0" encoding="utf-8" ?>
<TabbedPage xmlns="http://xamarin.com/schemas/2014/forms"
             xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
             x:Class="Setup.Forms.Views.HomeView">
    <ContentPage Title="First tab">
        <Label Text="Testing first tab">
            
        </Label></ContentPage>
    <ContentPage Title="Second tab">
        <Label Text="Testing second tab"></Label>
    </ContentPage>
</TabbedPage>

The view will show up but now the tabs have been added multiple times (4 times instead of 2). I've located the error to something happening inside the ViewLocator.LocateForModel(viewModel, null, null); code. If I do a new HomeView() the problem will not appear.

Note:

I've also seen this when using the NavigationService. In this case a toolbar button was added multiple times. I am guessing it also uses ViewLocator.LocateForModel internally? It's like all Xaml gets evaluated twice or something.

Here's a picture of the result:

caliburnbug

@nigel-sampson

This comment has been minimized.

Show comment
Hide comment
@nigel-sampson

nigel-sampson May 8, 2017

Contributor

Thanks for this, I'll have to look into the generated code by Xamarin.Forms. The other xaml platforms have guards in to detect duplicate initialization, it appears XF doesn't.

This is here to support xaml files without a code behind which admittedly isn't a highly used feature.

Contributor

nigel-sampson commented May 8, 2017

Thanks for this, I'll have to look into the generated code by Xamarin.Forms. The other xaml platforms have guards in to detect duplicate initialization, it appears XF doesn't.

This is here to support xaml files without a code behind which admittedly isn't a highly used feature.

@nigel-sampson nigel-sampson added the bug label May 8, 2017

@nigel-sampson nigel-sampson added this to the v3.1.0 milestone May 8, 2017

nigel-sampson added a commit that referenced this issue May 8, 2017

#432 Don't call InitializeComponent in Forms
This was a feature to support xaml files with no code behind that
doesn't get much use. It worked well because the WIndows generated code
guards against double initialisation, Xamarin.Forms doesn't.
@nigel-sampson

This comment has been minimized.

Show comment
Hide comment
@nigel-sampson

nigel-sampson May 8, 2017

Contributor

I've pushed a change that doesn't call InitializeComponent on Xamarin.Forms.

Contributor

nigel-sampson commented May 8, 2017

I've pushed a change that doesn't call InitializeComponent on Xamarin.Forms.

@johot

This comment has been minimized.

Show comment
Hide comment
@johot

johot May 8, 2017

Hi @nigel-sampson!

Thank you for fixing this so fast!
I hope we can see this in a relase real soon, good work!

johot commented May 8, 2017

Hi @nigel-sampson!

Thank you for fixing this so fast!
I hope we can see this in a relase real soon, good work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment