Remove @angular/animations dependency#432
Open
jtotht wants to merge 5 commits into
Open
Conversation
Many dependency URLs used to point to https://jfrog.devstack.vwgroup.com/, which is an internal, password-protected service for Volkswagen Group employees. Replace the URLs with https://registry.npmjs.org/ ones, which do not require login. I used sed -i 's~https://jfrog.devstack.vwgroup.com/artifactory/api/npm/npm/~https://registry.npmjs.org/~' package-lock.json for the replacement. I didn’t manually check the result, but `npm ci` did succeed afterwards.
To match the version of other Angular packages. Otherwise the demo app won’t build.
So that `import ... from 'angular2-notifications'` statements work. Also add Angular `CommonModule` to the module of the demo app, so that `*ngFor`s in the example component template work. (Alternatively, we could use the Angular 17+ control flow syntax, but since the library itself is currently Angular 16+, this may be too early.) Amend `.gitignore` to ignore the `node_modules/` directory of the library as well, which is now populated by an `npm i` in the root directory.
`fade`, `fromTop` and `fromBottom` used to be missing from the dropdown. Fortunately, TypeScript enums exist at run time, so we can just extract the values from that, ensuring that any eventual future animation types automatically appear.
The animations module is deprecated since Angular 20, for removal in Angular 23 (November 2026). The requirement to include the responsible module in consumer applications was confusing anyway. Use native CSS animations instead. Backward compatibility considerations: - Don’t use `animate.enter` and `animate.leave`. While they could avoid having to add another `setTimeout()`, they have only been available since Angular 20.2. That tiny extra code keeps the library compatible with Angular 16, bringing the performance benefits of native CSS animations to also those who are stuck on older Angular versions. - Don’t touch the value of `Notification.state`. While it would be more convenient to make it three-state, excluding the animation type from its value, it’s part of a public interface, and I don’t want to make breaking changes to it.
Merged
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
The animations module is deprecated since Angular 20, for removal in Angular 23 (November 2026). The requirement to include the responsible module in consumer applications was confusing anyway. Use native CSS animations instead.
Backward compatibility considerations:
animate.enterandanimate.leave. While they could avoid having to add anothersetTimeout(), they have only been available since Angular 20.2. That tiny extra code keeps the library compatible with Angular 16, bringing the performance benefits of native CSS animations to also those who are stuck on older Angular versions.Notification.state. While it would be more convenient to make it three-state, excluding the animation type from its value, it’s part of a public interface, and I don’t want to make breaking changes to it.Also fix a number of build errors in separate commits (please don’t squash commits upon merge).