-
Notifications
You must be signed in to change notification settings - Fork 42
Refactor ViewAnimation to accept only subconfiguration #446
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
Refactor ViewAnimation to accept only subconfiguration #446
Conversation
…construction argument. For backward compatibility allow the full appConfig as parameter but prompt a warning.
…imation argument.
6299c07
to
4fff2a2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one is a nice refactoring, this makes more sense than passing the whole configuration object for sure !
I just suggested to change a parameter definition in a comment but I must admit I never verified in the rest of the codebase whether this change should be applied elsewhere or not so it's really not a problem if kept as is.
Feel free to merge this one and thanks again for the contribution !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice, thanks for this 👍
…nUtil object with the full application context.
Thanks for the reviews! As suggested by @chrismayer I removed the backward compatibility option. @sronveaux: A quick search for the usage of optional parameters yielded several modules, e.g. Let me know if this is fine for you, then I will merge. |
Hi @fschmenger and thanks again for this ! You are totally right concerning optional parameters in JSDoc comments and did well... I just opened issue #448 to keep this in mind in case someone has some time to spend on this one day... |
This PR updates the
ViewAnimation
utility to improve encapsulation by changing its constructor signature. Instead of receiving the entire application context object, it now accepts only theviewAnimation
sub-property. This aligns with a pattern already used elsewhere in the codebase.To avoid introducing a breaking change, the constructor still accepts the full application context for now. However, passing the full context will trigger a deprecation warning.