mikke89 / RmlUi

RmlUi - The HTML/CSS User Interface library evolved
https://mikke89.github.io/RmlUiDoc/
MIT License
2.57k stars 295 forks source link

Multiple animations with the same property on the same element not supported #608

Closed archydragon closed 2 months ago

archydragon commented 2 months ago

Hello! First of all, thanks a lot for such a great library! It's very pleasant to integrate and use so far.

However, I've encountered a bug: if I define multiple animations for an element comma-separated, the first one does not play.

Example: I'm trying to do a simple fade in-out loop.

RML:

<div id="text">Some test splash</div>

RCSS:

@keyframes fadein {
    from {
        opacity: 0;
        visibility: hidden;
    }
    to {
        opacity: 1;
        visibility: visible;
    }
}

@keyframes fadeout {
    from {
        opacity: 1;
        visibility: visible;
    }
    to {
        opacity: 0;
        visibility: hidden;
    }
}

body {
    background: #000;
    height: 100%;
}

div#text {
    font-size: 48pt;
    color: #c0c0c0;
    text-align: center;
    width: 100vw;
    height: 48pt;
    margin: auto;
    animation:
        1s fadein,
        1s 5s fadeout;
}

Expected result: blank screen, then the text fades in, then in a few seconds fades out. Works as intended in browsers: https://jsfiddle.net/29wab1s7/

Result with RmlUi: nothing happens within 5 seconds, then text fades out.

Used RmlUi commit: 43955b75557185728a44d35f535388fe851e9901 (the latest one in master at the moment of writing).

Thank you very much in advance!

mikke89 commented 2 months ago

Hey, and thanks for the kind words! And also for the detailed issue description.

Yeah, I see that we are not acting like web browsers here. In fact, we haven't really made any considerations for multiple animation values acting on the same properties. That's a use case I haven't come across yet, so we simply don't handle that in any particular way.

Just to be clear, we should support multiple animation values more generally, I added a test here just be sure that it works as expected in a more straightforward case: https://github.com/mikke89/RmlUi/commit/559d84ab274b8e480d9106f17602d439bf759b0e

I might look to see if we can do something about this without too large changes. For now though, I'd suggest that you use a single set of keyframes if you can, e.g.:

@keyframes fadeinout {
    20%, 80% {
        opacity: 1;
        visibility: visible;
    }
}

.fadeinout {
    opacity: 0;
    visibility: hidden;
    animation: 5s fadeinout;
}

I would also say that in the example you posted, the element is visible with full opacity before and after the animation, so it has a sharp transition which could be problematic. The above example should be more stable in this regard.

archydragon commented 2 months ago

Single set of keyframes works very well for me! Sorry, I'm not really proficient with CSS animations so just copy-pasted fadein and fadeout from samples code originally :D

The documentation at https://mikke89.github.io/RmlUiDoc/pages/rcss/animations_transitions_transforms.html states that "Multiple animations can be specified on the same element by using a comma-separated list", so yep, it's just a question of feature parity with browser CSS behavior people like me might expect. Probably something for 7.0...

mikke89 commented 2 months ago

Looked a bit more into this one again to see if there is a quick way to make it work with multiple animations with the same property on the same element. Unfortunately, it requires quite a bit more work due to some architectural assumptions, especially since different animations can have completely different animation styles (such as repetition count).

For now, I added a warning in this situation, so that users at least know something is going on.

I am closing this issue for now, and instead putting it on my long-term feature list. Certainly would be a nice-to-have feature for the future.

archydragon commented 2 months ago

Yup, that's fine, thank you very much for the support!