Improving animation states

[stsrki]

This is the first try at making the animation state properly styled. I have focused on the Modal component. The biggest problem was to make it work with BS4 and BS5 because natively it works by controlling display. And display is hard to animate when it is none. So I introduced new styles for animations based on opacity and visibility.

Please test and see if you have anything to change. @David-Moreira

PS. Tailwind still needs fixing.

[David-Moreira]

This is the first try at making the animation state properly styled.

Was it not properly styled before?

[David-Moreira]

Seems to be working? Animation is now different? Is it supposed to "grow" inputs? Is not just fade in and fade out whats expected?

And then if I open the same Modal, the same no longer happens. It only happens as you open different sized modals.

[David-Moreira]

The Lazy Reload no longer works the same as the others it seems.

[David-Moreira]

I did not bother checking other providers yet, just BS.

[stsrki]

This is taking me longer than I anticipated 🧐

@David-Moreira can you try again BS4 and BS5?

[stsrki]

This is the first try at making the animation state properly styled.

Was it not properly styled before?

It worked. But the problem was that our animation system was rigged to work only with Modal. Remember that hardcoded 10ms? That made it to switch CSS class names and styles really fast, and then bootstrap CSS would do its thing. But it worked only in that scenario. If we remove 10ms and make it longer as it should be, then the CSS animations would break.

This PR introduces new CSS that works around those limitations. Hopefully by not breaking something else instead.

[David-Moreira]

BS / BS5 Lazy reload does not seem to fade out. Other than that, it's ok it seems.
I started testing BUlma but it doesn't seem to be working correctly. You've only done for BS for now right?

[stsrki]

BS / BS5 Lazy reload does not seem to fade out. Other than that, it's ok it seems.

LazyReload is different because it is removed from the DOM when hidden and there is nothing to animate. One thing I can think of fixing that is to wait for the hiding animation before we remove it. But, we can do that in the future. If you agree I will open new ticket?

I started testing BUlma but it doesn't seem to be working correctly. You've only done for BS for now right?

Bulma never worked 100%, and since there is soon going to be released in v1, the plan is to focus on it in the Blazorise v1.6. We should be part of the closed beta group for Bulma release so hopefully plenty of time to work.

[David-Moreira]

BS / BS5 Lazy reload does not seem to fade out. Other than that, it's ok it seems.

LazyReload is different because it is removed from the DOM when hidden and there is nothing to animate. One thing I can think of fixing that is to wait for the hiding animation before we remove it. But, we can do that in the future. If you agree I will open new ticket?

I started testing BUlma but it doesn't seem to be working correctly. You've only done for BS for now right?

Bulma never worked 100%, and since there is soon going to be released in v1, the plan is to focus on it in the Blazorise v1.6. We should be part of the closed beta group for Bulma release so hopefully plenty of time to work.

Well, it seems like to me like it's working currently? So we are consciously breaking it now? I'm okay if you prefer fixing it later if we are time constrained, but I'd prefer to not break it consciously if we could avoid?

Ok, so Bulma never worked? From looking at the demo it does seem like it is only fading in currently and not quite consistent.

Should I test the other providers, or are they ok for now?

[stsrki]

Well, it seems like to me like it's working currently?

It works now, yes. But, as I already mentioned. The CloseableAdapter.Run is currently made in a way that it is suited best for Modal. The problem is because of the forced await Task.Delay( 10 );. That is not working for Toast component. So either we go with changes in this PR. Or override the await Task.Delay( 10 ); conditionally for other component. eg

public async Task Run( bool visible, bool shortAnimation )
{
    if ( component is IAnimatedComponent animatedComponent && animatedComponent.Animated )
    {
        await animatedComponent.BeginAnimation( visible );

        if ( visible && shortAnimation )
            // The 10ms delay is selected based on testing.
            // Task.Yield works for Blazor WebAssembly, but not for Blazor Server apps.
            // A small delay works well for both.
            await Task.Delay( 10 );
        else
            await Task.Delay( animatedComponent.AnimationDuration );

        await animatedComponent.EndAnimation( visible );
    }
    else
    {
        if ( visible )
            await component.Show();
        else
            await component.Hide();
    }
}

[David-Moreira]

Well, it seems like to me like it's working currently?

It works now, yes. But, as I already mentioned. The CloseableAdapter.Run is currently made in a way that it is suited best for Modal. The problem is because of the forced await Task.Delay( 10 );. That is not working for Toast component. So either we go with changes in this PR. Or override the await Task.Delay( 10 ); conditionally for other component. eg

public async Task Run( bool visible, bool shortAnimation )
{
    if ( component is IAnimatedComponent animatedComponent && animatedComponent.Animated )
    {
        await animatedComponent.BeginAnimation( visible );

        if ( visible && shortAnimation )
            // The 10ms delay is selected based on testing.
            // Task.Yield works for Blazor WebAssembly, but not for Blazor Server apps.
            // A small delay works well for both.
            await Task.Delay( 10 );
        else
            await Task.Delay( animatedComponent.AnimationDuration );

        await animatedComponent.EndAnimation( visible );
    }
    else
    {
        if ( visible )
            await component.Show();
        else
            await component.Hide();
    }
}

Well... now you went through all this work refactoring. As long as your solution is flexible enough to still fix edge cases like this I'm fine keeping it.

I'd probably have kept the previous version with whatever edge case we needed to add in the interest of time, since animation takes a lot of time to get right through blazor or so I feel.

If you're sure this version will be for the best, again, I'm okay with it, the old version did feel convoluted/ maybe a bit hacky (although at the end of the day, what matters is that it works, hehe)

[stsrki]

I still believe this is a more flexible solution. And if anyone complains we can always fix it.