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
sronveaux
left a comment
There was a problem hiding this comment.
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 !
chrismayer
left a comment
There was a problem hiding this comment.
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
ViewAnimationutility to improve encapsulation by changing its constructor signature. Instead of receiving the entire application context object, it now accepts only theviewAnimationsub-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.