-
Notifications
You must be signed in to change notification settings - Fork 91
Enable declarative effect variations #35
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
Open
esDotDev
wants to merge
12
commits into
gskinner:main
Choose a base branch
from
esDotDev:feature-composite-effects
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…osition / architecture Add CompositeEffect helper class, and composeEffects helper method Tweak expectWidgetWithDouble method to be less strict
… a composite tween that changes the underlying type, which is not currently supported cleanly.
# Conflicts: # lib/effects/shake_effect.dart
089c49a
to
24a10c9
Compare
aa33ce0
to
eaf8ab8
Compare
56d4cfb
to
45ef6b2
Compare
Renamed `buildAnimation` to `buildBeginEndAnimation` to make things a little more clear.
…e they have no use for begin/end
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this does
Initial pass at a solution for #31 and beginnings of #34.
It sets a core architecture that supports both declarative syntax
[ FadeInEffect() ]
and imperitiveanimate().fadeIn()
when creating effect variations. The current solution for variations supports only the imperative syntax.It also adds some plumbing to more easily make variations of other effects. Whether this is composing two or more existing effect together (FadeEffect + SlideEffect), or simply creating a variation of an existing one (FadeIn or BlurX).
NOTE: While this touches many files, that is mostly due to a rename. The bulk of the interesting changes are in the effect.dart class and the new variations themselves.
Architecture
Effects do not always want to expose a public begin/end value.
FadeInAndUp
may haveOffset beginOffset
anddouble beginOpacity
Toggle
orSwap
.Being forced to inherit
begin
/end
fields in these cases is confusing/undesired, so (imo) we need to make this optional.To allow this, a new
BeginEndEffect<T>
class was created, which extendsEffect
, and thebegin/end
fields were moved there.Most of the existing 'core' effects were modified to extend
BeginEndEffect
, the exceptions wereThen
,Toggle
,Callback
andSwap
where we were able to extendEffect
directly and remove the weirdEffect<double>
andEffect<void>
stuff that was being forced. There are also no longer phantom begin/end properties on these effects which is nice.Finally a
CompositeEffectMixin
was created, to make it easier to create a new effect from existing ones. The mixin is just a small bit of syntactic sugar, that requires a list of effects, overrides build, and auto-builds all the effects. This allows variations to be created with virtually no boilerplate.Importantly, a new effect can both extend
BeginEndEffect<T>
and use theCompositeEffectMixin
if needed so it's quite flexible and easy to use in different configurations.Example 1,
FadeInEffect
, does not want to expose public begin/end values, so it onlyextends Effect
. It usesCompositeEffectMixin
to keep boilerplate to a minimum:Example 2,
BlurX
does want a begin and end, so itextends BeginEndEffect
and usesCompositeEffectMixin
. This shows how a variation can change the type of its core effect,BlurEffect
takes anOffset
, but this takes adouble
, straight inheritence would struggle here but the compositional approach has no problems:Example 3,
FadeInUp
composesFadeIn
andSlideInUp
effects, which themselves are composed ofFade
andSlide
, showing advanced multi-level composition. It does not useextends BeginEndEffect
, because it would be unclear what properties they refer to. Instead it declares it's ownbeginY
for clarity:NOTE: In this example its been decided that
beginY
would make sense but the other values do not (beginOpacity
,endOpacity
andendY
). This is debatable, but also not relevant to the example.What is left?
FadeInDown
,SlideRight
, etc)