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

IsVisible? #57

Closed
tleylan opened this issue Jan 23, 2020 · 5 comments
Closed

IsVisible? #57

tleylan opened this issue Jan 23, 2020 · 5 comments
Assignees
Labels
area-mobileblazorbindings Experimental Mobile Bindings for Blazor
Milestone

Comments

@tleylan
Copy link

tleylan commented Jan 23, 2020

At least I'm trying things :-) Does the IsVisible attribute work? I set it to false on a StackLayout that is in a GridCell but the components in that StackLayout are displayed.

There are also warnings displayed on two properties
Component parameter 'IsVisible' should not be set outside of its component
Component parameter 'BackgroundColor' should not be set outside of its component

@0x414c49
Copy link
Contributor

0x414c49 commented Jan 23, 2020

It works and it doesn't work. @Eilon I think I found the bug:

Considering the example below, if the IsVisible property is true at the first, then an event changes it to false, the element will be hidden.

xamarin-01

<TabbedPage>
    <ContentPage Title="Visible Test">
        <StackLayout>
            <Button  OnClick="Toggle" Text="Click me" />
            <Label IsVisible="IsVisible" FontSize="20" TextColor="Color.Crimson" Text="Todo Items" HorizontalTextAlignment="TextAlignment.Center" />
        </StackLayout>
    </ContentPage>
</TabbedPage>


@code{
    public bool IsVisible { get; set; } = true;

    public void Toggle()
    {
        this.IsVisible = ! this.IsVisible;
    }
}

But if change the code to this:

<TabbedPage>
    <ContentPage Title="Visible Test">
        <StackLayout>
            <Label IsVisible="false" FontSize="20" TextColor="Color.Crimson" Text="Todo Items" HorizontalTextAlignment="TextAlignment.Center" />
        </StackLayout>
    </ContentPage>
</TabbedPage>


@code{

}

Annotation 2020-01-23 151301

The label is still there!

As we know, if the IsVisible prop is false, the element is removed from the visual tree:

Setting IsVisible to false will remove the element from the visual tree. The element will no longer take up space in layouts or be eligle to receive any kind of input event.
https://docs.microsoft.com/en-us/dotnet/api/Xamarin.Forms.VisualElement.IsVisible?view=xamarin-forms

Every element in MobileBlazorBindings has two classes. A wrapper and handler. In VisualElementclass the property of IsVisible is defined. Inside RenderAttributes of ElementClass the property is also added to the builder:

if (IsVisible != null)
{
	builder.AddAttribute(nameof(IsVisible), IsVisible.Value);
}

Later, in VisualElementHandler class, inside ApplyAttributes method, we can see it's mentioned:

case nameof(XF.VisualElement.IsVisible):
	VisualElementControl.IsVisible = AttributeHelper.GetBool(attributeValue);
	break;

The only problem is, this ApplyAttributes method (for IsVisible prop) only will be called if the value is true, just set it to false and this method will never be called.

I think the bug is, when the IsVisible is false at the beginning, the element is not listed in the frames in (NativeComponentAdapter class), so the ApplyAttribute method won't be called.

I debugged it several times, and it seems I'm correct.

@tleylan
Copy link
Author

tleylan commented Jan 23, 2020

@0x414c49 Thanks very much... your answer led me to the answer. I got caught up trying to figure out Xamarin and forgot I was using Blazor. It's working now.

@Eilon
Copy link
Member

Eilon commented Jan 23, 2020

Ah yes I figured there were some cases like this that weren't handled properly, but I hadn't yet seen any.

The fundamental issue is covered by this issue I logged a while ago: #15

And, the tool I'm working on in #25 will help produce the right code for these.

This is what I'm working on right now so I hope to have this fixed very soon.

@Eilon Eilon self-assigned this Jan 23, 2020
@Eilon Eilon added the area-mobileblazorbindings Experimental Mobile Bindings for Blazor label Jan 23, 2020
@Eilon Eilon added this to the 0.2.0-preview milestone Jan 23, 2020
@Eilon
Copy link
Member

Eilon commented Jan 25, 2020

It turns out there were numerous problems conspiring together to prevent this from working. I think I have them all fixed on my machine.

  1. When attributes were un-set, the wrong value could be set on the Xamarin.Forms element. This happens because Mobile Blazor Bindings sees a null value in this case, and makes arbitrary assumptions (sometimes right, sometimes wrong) about what to do in those cases. I'm fixing this by generating the right 'default' value when a property is un-set. The new component wrapper generator tool will make this super easy.
  2. Blazor/Razor have some rendering logic where false values for bool element properties seem to not be flowed through to the native renderer. I'm working around this by not rendering bool values as bools, but rather as strings, which means that the native renderer can always do the right thing with the value.

Eilon added a commit that referenced this issue Jan 25, 2020
@Eilon Eilon closed this as completed in 974f79c Jan 25, 2020
@Eilon
Copy link
Member

Eilon commented Jan 25, 2020

OK I think this is now fixed in the live code base. Definitely a very interesting issue to fix because it helped uncover some gnarly bugs!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-mobileblazorbindings Experimental Mobile Bindings for Blazor
Projects
None yet
Development

No branches or pull requests

3 participants