fuse-open / fuselibs

Fuselibs is the Uno-libraries that provide the UI framework used in Fuse apps
https://npmjs.com/package/@fuse-open/fuselibs
MIT License
176 stars 72 forks source link

ActivatingAnimation overrides Set #357

Open kusma opened 7 years ago

kusma commented 7 years ago

If you try to run this example:

<App>
    <EdgeNavigator>
        <Panel ux:Name="sidebar" Edge="Left" Width="100%" Margin="0,0,56,0" Background="#37474F">
            <Shadow ux:Name="shadow" Angle="180" Distance="8" Size="16" Color="Red" />
            <ActivatingAnimation>
                <Change shadow.Color="Blue" />
            </ActivatingAnimation>
        </Panel>

        <DockPanel Color="#263238">
            <Rectangle Color="Green" Width="100" Height="100">
                <Clicked>
                    <Set shadow.Color="Green" />
                </Clicked>
            </Rectangle>
            <StackPanel Dock="Top" Color="Blue" Height="50">
                <StatusBarBackground />
                <Clicked>
                    <NavigateToggle Target="sidebar" />
                </Clicked>
            </StackPanel>
        </DockPanel>
    </EdgeNavigator>
</App>

1) Click the blue top-bar. This pulls out the side-menu. 2) Click outside the side-menu to dismiss it. 3) Click the green button. This changes the shadow-color from red to green. 4) Click the blue top-bar to pull out the side-menu again. 5) Click outside the side-menu to dismiss it.

The result is that the shadow of the side-menu is red, instead of the expected green.

If we instead click the green button before ever opening the side-bar, everything works fine, and we're left with a green shadow.

This all indicates that the animation-system captures some states that can change in the mean time. This is especially disturbing in preview, where the property is changed by color-scrubbing instead.

Originally reported here: https://github.com/fusetools/fuselibs/issues/4259

mortoray commented 7 years ago

Change does capture state, but it's supposed to reset it when it's deactivated.

mortoray commented 7 years ago

This happens because Shadow.Color is not a reactive variable (it doesn't use the property system or emit change events). I could fix this, but it's just one of many variables in fuselibs that have the same problem. The overhead in using the property system is large enough that we just don't do it for all variables. These variables can still be animated, but they can't be simultaneously changed anywhere else.

We should probably have a doc-tag indicating which properties are "reactive" (the error you encountered can also manifest when using bindings in various ways, not just animation).

I'll check if I can at least get the animator to respect the value change when at rest (which is just a subset of the problem, but at least somewhat useful).

mortoray commented 7 years ago

This example also uses ActivatingAnimation, ...Animation triggers don't have inactive states, they are basically always active. Using a WhileActive trigger would actually solve the problem when the value changes in the rest state. Hmmm...

mortoray commented 7 years ago

Oh boy, there's a one-line fix, but the possibility for unintended consequences, including performance on some navigation, probably means I can't use it and will need a more thorough change.

The issue is that ...Animation triggers, or anything using ...Seek never calls CleanupStableState in Trigger. Thus at 0-state they'll remain "active". I can't simply call the function when progress==0 either, since some animations will bounce, and cleanup/creation is expensive.

NOTE: This fix still doesn't touch the underyling problem of the .Color property.