Groupsetmanager/migration #244
Conversation
|
Hi Amos, you also need to import the component to AngularModule.ts and downgrade your service to AngularJS.ts |
|
Hi amos, I have reviewed your component seems like the component still hasn't been linked to the angular module and hasn't been downgraded in the angularjs. Please make these changes then you'll might have to test it again. Thanks. |
|
I have made the required changes that both of you have commented. Please do provide your feedback. |
|
Hi Amos, I have tested your component It seems like I cannot get to your component because of the nested component group-set-selector. And I can also see that you are having an error regarding it within your component. Apart from that the code looks good the might be some updates needed to be done in the TS file regarding the logic. |
|
You don't have the right screenshot attached to this PR. Current screenshot shows unit-group-set-editor. group-set-manager is used in the Groups tab not Admin tab. Please include before and after screenshots |
|
Multiple nested components are still in AngularJS thus cannot be used. This should have been identified in the component review and brought to attention at the stand up to be resolved. |
|
When testing with your changes, Groups page is blank unlike the before version. Code is not working |
|
I did mention that issue in the comments about the nested components in the Teams planner. By the time I got to know about the proper way how to migrate it was pretty late, and I could not mention the nested components on time. And could you give me a response with the things that I can do to make these issues right. |
| @@ -28,7 +28,7 @@ | |||
| "dependencies": { | |||
| "@angular/animations": "^17.3.6", | |||
| "@angular/cdk": "^17.3.6", | |||
| "@angular/cli": "^17.3.6", | |||
| "@angular/cli": "^17.3.9", | |||
There was a problem hiding this comment.
This should not have been updated
| > --> | ||
| <!-- </group-selector> --> | ||
|
|
||
| <mat-card *ngIf="selectedGroup"> |
There was a problem hiding this comment.
ngIf and the like should not be used
| </form> | ||
| </mat-card-content> | ||
|
|
||
| <group-member-list |
There was a problem hiding this comment.
This is still a AngularJS component so this would not work
| @@ -0,0 +1,59 @@ | |||
| <!-- <group-selector | |||
There was a problem hiding this comment.
Why is this commented out? You should still be using group-selector. But that component would need to be migrated to Angular 17 first
There was a problem hiding this comment.
Instead of using the scss file, you should use TailwindCSS in the html file
| if (this.selectedGroupSet) { | ||
| this.selectedGroup = this.selectedGroupSet.groups[0]; | ||
| } | ||
| console.log('New group selected:', this.selectedGroup); |
There was a problem hiding this comment.
No need for console logs. If the original coffee file showed alerts or success messages upon completing actions, follow that
Description
This pull request migrates the group-set-manager component from CoffeeScript and AngularJS to TypeScript and Angular. The migration includes replacing the original .coffee, .scss, and .tpl.html files with new .ts, .scss, and .html files. The component now follows Angular Material design principles, and Bootstrap elements have been replaced with Angular Material components.
Type of change
After
How Has This Been Tested?
The component has been tested in the local development environment. The following actions have been verified:
Replaced the old AngularJS component in the project.
Ensured that the new TypeScript component renders and functions correctly.
The component’s interaction with the project module, unit, and group elements has been verified
Testing Checklist:
Checklist: