Angular - Applying the latest migrations - Issue 24564#24579
Conversation
Replaced usage of NgClass with Angular's [class] and [class.*] bindings in multiple components for improved performance and clarity. Removed NgClass imports where no longer needed.
Replaced [ngStyle] with [style] bindings in multiple components for improved Angular compatibility and performance. Removed unnecessary NgStyle imports from affected components and updated their module imports accordingly.
Replaced CommonModule with more specific Angular imports (NgTemplateOutlet, NgClass) in several components to optimize module usage and reduce unnecessary dependencies. Also removed unused CommonModule import from email-setting-group and http-error-wrapper components.
Replaces all usages of [ngClass] with [class] bindings in component templates and removes NgClass from component imports. This simplifies the code, improves performance, and ensures compatibility with standalone Angular components.
| <div | ||
| class="alert alert-{{ alert.type }} fade show" | ||
| [ngClass]="{ 'alert-dismissible fade show': alert.dismissible }" | ||
| [class.alert-dismissible]="alert.dismissible" [class.fade]="alert.dismissible" [class.show]="alert.dismissible" |
There was a problem hiding this comment.
We should keep fade and show always present, only conditionally add alert-dismissible like this:
<div
class="alert alert-{{ alert.type }} fade show"
[class.alert-dismissible]="alert.dismissible"
role="alert"
>
sumeyyeKurtulus
left a comment
There was a problem hiding this comment.
Hello @fahrigedik thank you for this migration. Here are some points that we can consider:
-
We can also update this animation deprecation here
npm/ng-packs/packages/account/src/lib/components/manage-profile/manage-profile.component.tsWe can declare such styles.fade-in { animation: fadeIn 350ms ease both; } @keyframes fadeIn { from { opacity: 0; } to { opacity: 1; } }
and replace the
[@fadeIn]usage withanimate.enter="fade-in"usage instead. -
We should also consider a formatting rule such as prettier formatting to use in an IDE. Because, some changes get larger because of an auto-formatting mismatch. I guess we can discuss it together when you check this @fahrigedik @erdemcaygor
Simplified the HTML structure of manage-profile and page-alert-container components by reducing unnecessary markup and improving readability. Replaced Angular animation usage in manage-profile with a CSS-based fade-in effect for better maintainability.
There was a problem hiding this comment.
Pull request overview
This pull request applies the latest Angular migrations to modernize the codebase, focusing on migrating away from deprecated or legacy Angular patterns to modern alternatives.
Changes:
- Migrated from
NgClassdirective to native[class]bindings throughout all components and templates - Migrated from
NgStyledirective to native[style]bindings throughout all components and templates - Converted
CommonModuleimports to specific standalone imports (e.g.,AsyncPipe,NgTemplateOutlet) - Replaced Angular animation with CSS animation in manage-profile component
Reviewed changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| templates/app/angular/src/app/home/home.component.html | Migrated NgClass to [class] bindings and reformatted template for consistency |
| templates/app-nolayers/angular/src/app/home/home.component.html | Migrated NgClass to [class] bindings and reformatted template for consistency |
| npm/ng-packs/packages/theme-shared/src/lib/components/toast/.ts/.html | Removed NgClass imports and migrated to [class] bindings |
| npm/ng-packs/packages/theme-shared/src/lib/components/password/.ts/.html | Removed NgClass imports and migrated to [class] bindings |
| npm/ng-packs/packages/theme-shared/src/lib/components/loader-bar/*.ts | Removed NgClass and NgStyle imports, migrated to [class] and [style] bindings |
| npm/ng-packs/packages/theme-shared/src/lib/components/http-error-wrapper/*.ts | Removed CommonModule import, kept only necessary standalone imports |
| npm/ng-packs/packages/theme-shared/src/lib/components/form-input/*.ts | Removed NgClass and NgStyle imports, migrated to [class] and [style] bindings |
| npm/ng-packs/packages/theme-shared/src/lib/components/confirmation/.ts/.html | Removed NgClass imports and migrated to [class] bindings |
| npm/ng-packs/packages/theme-shared/src/lib/components/checkbox/*.ts | Removed NgClass and NgStyle imports, migrated to [class] and [style] bindings |
| npm/ng-packs/packages/theme-shared/src/lib/components/card/*.ts | Removed NgClass and NgStyle imports from all card-related components |
| npm/ng-packs/packages/theme-shared/src/lib/components/button/*.ts | Removed CommonModule import, migrated to [class] bindings |
| npm/ng-packs/packages/theme-basic/src/lib/components/routes/.ts/.html | Removed NgClass imports and migrated to [class] bindings |
| npm/ng-packs/packages/theme-basic/src/lib/components/page-alert-container/.ts/.html | Removed NgClass imports and migrated to [class] bindings with potential behavioral change |
| npm/ng-packs/packages/setting-management/config/src/lib/components/email-setting-group/*.ts | Removed CommonModule import |
| npm/ng-packs/packages/permission-management/src/lib/components/.ts/.html | Removed NgStyle imports and migrated to [style] bindings |
| npm/ng-packs/packages/feature-management/src/lib/components/feature-management/*.html | Migrated NgStyle to [style] bindings and reformatted template |
| npm/ng-packs/packages/components/lookup/src/lib/*.ts | Replaced CommonModule with NgTemplateOutlet import |
| npm/ng-packs/packages/components/extensible/src/lib/components/page-toolbar/.ts/.html | Removed NgClass imports and migrated to [class] bindings |
| npm/ng-packs/packages/components/extensible/src/lib/components/grid-actions/.ts/.html | Removed NgClass imports and migrated to [class] bindings |
| npm/ng-packs/packages/components/extensible/src/lib/components/extensible-table/*.html | Migrated NgClass to [class] bindings |
| npm/ng-packs/packages/components/extensible/src/lib/components/extensible-form/.ts/.html | Removed NgClass imports and migrated to [class] bindings |
| npm/ng-packs/packages/account/src/lib/components/manage-profile/.ts/.html | Replaced Angular animation with CSS animation, removed NgClass import |
| PermissionDirective | ||
| ], |
There was a problem hiding this comment.
The trailing comma is missing after the last element in the imports array. This is inconsistent with TypeScript style conventions used elsewhere in the codebase where trailing commas are present after the last array element.
| PermissionDirective | |
| ], | |
| PermissionDirective, | |
| ], |
| ModalCloseDirective | ||
| ], |
There was a problem hiding this comment.
The trailing comma is missing after the last element in the imports array. This is inconsistent with TypeScript style conventions used elsewhere in the codebase where trailing commas are present after the last array element.
| ModalCloseDirective | |
| ], | |
| ModalCloseDirective, | |
| ], |
| LocalizationPipe, | ||
| ReplaceableTemplateDirective, | ||
| LoadingDirective, | ||
| LoadingDirective |
There was a problem hiding this comment.
The trailing comma is missing after the last element in the imports array. This is inconsistent with TypeScript style conventions used elsewhere in the codebase where trailing commas are present after the last array element.
| LoadingDirective | |
| LoadingDirective, |
| } | ||
| </div> | ||
| } | ||
| <div class="alert alert-{{ alert.type }} fade show" [class.alert-dismissible]="alert.dismissible" role="alert"> |
There was a problem hiding this comment.
The migration from ngClass changes the behavior. The original code applied "fade show" classes conditionally only when alert.dismissible is true. The new code always applies "fade show" classes and only conditionally applies "alert-dismissible". This change in behavior may be intentional as part of the Angular migration, but it should be verified that alerts should always fade in/show regardless of dismissibility.
| <div class="alert alert-{{ alert.type }} fade show" [class.alert-dismissible]="alert.dismissible" role="alert"> | |
| <div class="alert alert-{{ alert.type }}" [class.fade]="alert.dismissible" [class.show]="alert.dismissible" [class.alert-dismissible]="alert.dismissible" role="alert"> |
Description
Resolves #24564 (write the related issue number if available)