Adding decision document around new animation api

runemadsen commented 2 years ago

You can read a nicer layout by visiting the markdown file directly.

netlify[bot] commented 2 years ago

Deploy Preview for dsi-logo-maker ready!

Name	Link
Latest commit	de8afea0d5a75120f9fd183e3c7e4e45c631ba59
Latest deploy log	https://app.netlify.com/sites/dsi-logo-maker/deploys/6343dda2ac917b0008edb90c
Deploy Preview	https://deploy-preview-156--dsi-logo-maker.netlify.app
Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

runemadsen commented 2 years ago

Great! After thinking about it, I'm not a big fan of the customAnimation mode name. I'm wondering if we can rename it to something that makes more sense, but I don't really have a good idea yet. static and animation still seem good to me.

fdoflorenzano commented 2 years ago

Great! After thinking about it, I'm not a big fan of the customAnimation mode name. I'm wondering if we can rename it to something that makes more sense, but I don't really have a good idea yet. static and animation still seem good to me.

Since they are strings anyways, I was thinking on going with "custom-animation" really, but yeah maybe it's not great. I'm honestly fine with it. Maybe a separate setting only applicable for "animated", that ahs values:"frame-based" and "event-based", so we start this animation basis language in the code too.

runemadsen commented 2 years ago

Thanks @fdoflorenzano! I think both of those could work. If we keep the single mode setting, then perhaps static, animation and animation-custom?

lucasdinonolte commented 2 years ago

I updated #152 with a draft implementation showing the new engine behavior for the two new animation modes.

A few observations:

Rewriting the frame and done methods as discussed above removes the need for treating them mutually exclusive in an if-else-block.
- you only need to wrap done in an if clause, but can call frame however you want, as done will make sure to exit the function after the next call of frame
- this logic is needed to correctly capture the last frame
Refactoring settings from animated?: booelan to mode?: 'static' | 'animation' | 'animation-custom' is a breaking change to existing design functions
- how do we want to communicate this? Is it docs only? Or should we add a checkDeprecations method to the mechanic core, that can run on initialization and check for any deprecated settings being present?
Additionally I just realized this should probably change the way static functions are handled too. I think static functions can always be thought of as pure, so I'd argue for them being treated similar to animation functions. But I think we can even go one step further here and say, that they don't even need to call done but can just return whenever they are done drawing.

runemadsen commented 1 year ago

@fdoflorenzano I have edited the doc and I think it's ready for you to take a look :)

fdoflorenzano commented 1 year ago

@runemadsen I like it! A couple of thoughts came to mind:

Is there an animated setting or similar in the settings of a design function? I didn't see any in the examples. Maybe having an example for the static case will help this feel complete.
For an animation DF, if I understood correctly, the api let's the user not use the drawLoophelper, right? Maybe adding an example of that too would help exemplify the flexibility aspect of the setup.

designsystemsinternational / mechanic