feat(create-session): add test cases and new angular update for sessi… - #806
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| [(ngModel)]="searchTerm" | ||
| (ionInput)="filterItems($event)" | ||
| placeholder="Search..." | ||
| <ion-searchbar [(ngModel)]="searchTerm" (ionInput)="filterItems($event)" placeholder="Search..." |
There was a problem hiding this comment.
use translate for the placeholder
| <div class="height"> | ||
| <div class="height" [ngSwitch]="type"> | ||
| <ion-list class="height" *ngSwitchCase="'default'"> | ||
| <ion-content class="ion-padding"> |
There was a problem hiding this comment.
Remove the inner ion-content
| <ion-button expand="full" (click)="onSubmit()">{{'SAVE' | translate}}</ion-button> | ||
| } | ||
| </div> | ||
| @if (isNotCompleted) { |
There was a problem hiding this comment.
reduce the nested conditions. Do it like this : get showMeetingSubmit(): boolean {
return this.isNotCompleted && this.type === 'segment';
} and in html @if (showMeetingSubmit) {
| </ion-card-header> | ||
| <ion-card-content> | ||
| <ion-list> | ||
| <ion-select #link class="select" [(ngModel)]="selectedLink" interface="popover" (ionChange)="clickOptions($event)" [compareWith]="compareWithFn" [value]="selectedLink"> |
There was a problem hiding this comment.
use ngModel and remove the value
| <div *ngFor="let option of meetingPlatforms"> | ||
| <app-dynamic-form *ngIf="selectedLink==option" [jsonFormData]=option.form #platformForm></app-dynamic-form> | ||
| </div> | ||
| @for (option of meetingPlatforms; track option.name) { |
There was a problem hiding this comment.
Keep this logic in ts file
| <div class="height"> | ||
| <div class="height" [ngSwitch]="type"> | ||
| <ion-list class="height" *ngSwitchCase="'default'"> | ||
| <ion-content class="ion-padding"> |
There was a problem hiding this comment.
Ducplicate ion-content
| @@ -51,13 +61,21 @@ | |||
|
|
|||
| <section [ngClass]="{'btn-disabled': form1?.myForm.invalid , 'btn-valid':form1?.myForm.valid}"> | |||
There was a problem hiding this comment.
Wrap it in the footer
| <ion-list> | ||
| <ion-select #link class="select" [(ngModel)]="selectedLink" interface="popover" (ionChange)="clickOptions($event)" [compareWith]="compareWithFn" [value]="selectedLink"> | ||
| <ion-select-option [value]="option" *ngFor="let option of meetingPlatforms"> | ||
| @for (option of meetingPlatforms; track $index) { |
There was a problem hiding this comment.
Instead of index use option.name
| <div class="ion-no-border segment" submitButton *ngIf="type == 'segment'"> | ||
| <ion-button *ngIf="sessionDetails?.meeting_info?.platform === 'OFF'" class="btns" size="default" fill="solid" (click)="setItLater()">{{'SET_IT_LATER' | translate}}</ion-button> | ||
| <ion-button [disabled]="platformForm && !platformForm.myForm.valid" class="btns" size="default" (click)="onSubmitLink()">{{'SUBMIT' | translate}}</ion-button> | ||
| @if (type == 'default' && !id) { |
| <ion-button expand="full" (click)="onSubmit()">{{'SAVE' | translate}}</ion-button> | ||
| } | ||
| </div> | ||
| @if (isNotCompleted) { |
There was a problem hiding this comment.
move this logic to ts, and do something like this
get showMeetingSubmit(): boolean {
return this.isNotCompleted && this.type === 'segment';
} and in html : @if (showMeetingSubmit) {
vishwanath1004
left a comment
There was a problem hiding this comment.
Keep Component size control , Strong typing,
Service extraction , Subscription cleanup
| ngOnInit() { | ||
|
|
||
| } |
There was a problem hiding this comment.
can we slipt this component to file upload service, form service to prefill the data so we can re use these services in other pages.
| this.formData = await this.form.populateEntity(this.formData, this.entityList); | ||
| this.cdr.detectChanges(); | ||
| this.permissionService.getPlatformConfig(); | ||
| this.activatedRoute.queryParamMap.subscribe(async (params) => { |
There was a problem hiding this comment.
Clear the subscriptions on ngDestroy
|
|
||
| } | ||
| async ionViewWillEnter() { |
There was a problem hiding this comment.
reduce the workload of ionViewWILLEnter add new method inside this called initialise or something and split the each funcationality as methods something like this : async initializePage(){
await this.loadConfig();
await this.loadForm();
awai t this.loadSession();
}
| styleUrls: ['./create-session.page.scss'], | ||
| standalone: false | ||
| }) | ||
| export class CreateSessionPage implements OnInit { |
There was a problem hiding this comment.
Split different reusable services like :
Form manage
Files upload and other if needed
|
|
||
| } | ||
| async ionViewWillEnter() { |
There was a problem hiding this comment.
Reduce the load for this method, create a seperate method for each funcationality to handle
| this.formData = await this.form.populateEntity(this.formData, this.entityList); | ||
| this.cdr.detectChanges(); | ||
| this.permissionService.getPlatformConfig(); | ||
| this.activatedRoute.queryParamMap.subscribe(async (params) => { |
There was a problem hiding this comment.
Clear this on destroy
|
|
||
| @if (control?.showField != false) { | ||
| <small class="error-msg d-flex error-msg flex-align-items-center"> | ||
| @if (myForm.get(control.name)?.touched && myForm.get(control.name)?.invalid) { |
There was a problem hiding this comment.
Each get() is expensive so we can do like this : @let ctrl = myForm.get(control.name); hen :- ctrl?.invalid && ctrl?.touched
| @if (control?.showField != false) { | ||
| <ion-item | ||
| *ngIf="control?.showField != false" | ||
| class="ion-no-padding" |
There was a problem hiding this comment.
repeated classes : [ngClass]="['ion-no-padding', control.class]"
There was a problem hiding this comment.
i tried to combine but it is not working
| </ng-template> | ||
| </ion-popover> | ||
| </div> | ||
| @if (control.label !== '' && !['rating'].includes(control.type) && !['chip'].includes(control.type) && !['search'].includes(control.type)) { |
There was a problem hiding this comment.
multiple includes, do like this : hiddenLabelTypes = ['rating','chip','search']; then in html !hiddenLabelTypes.includes(control.type)
| @for (filter of filterData(); track $index) { | ||
| <div class="ion-padding-start ion-padding-top"> | ||
| <ion-label class="sub-header">{{filter?.title}}</ion-label> | ||
| @for (data of filter?.options; track $index) { |
| @@ -1,51 +1,51 @@ | |||
| import { Component, EventEmitter, Input, OnInit, Output } from '@angular/core'; | |||
| import { Component, EventEmitter, input, OnInit, Output, signal } from '@angular/core'; | |||
| import * as _ from 'lodash'; | |||
There was a problem hiding this comment.
if not using remove ths
| (click)="saveLink()" | ||
| > | ||
| <ion-button color="primary" fill="solid" | ||
| [disabled]="(type === 'link' && (!link || showLinkError())) || (type === 'file' && !uploadedFile())" |
There was a problem hiding this comment.
use computed signlas for showLinkError() and uploadedFile()
| </div> | ||
| </ion-content> | ||
|
|
||
| <div class="modal-footer"> |
| isNew: true, | ||
| }; | ||
| const obj = { | ||
| name: this.name ? this.name : this.uploadedFile().name, |
There was a problem hiding this comment.
this.uploadedFile().name this may crash if this.uploadedFile() is null
| }); | ||
| await actionSheet.present(); | ||
| const actionSheet = await this.actionSheetController.create({ | ||
| header: 'Select Resource', |
| }); | ||
| } else if (this.type === 'link') { | ||
| const obj = { | ||
| name: this.name ? this.name : this.link, |
There was a problem hiding this comment.
name: this.name || this.link
| <div class="d-flex flex-justify-center"> | ||
| <div class="profile-container " | ||
| *ngIf="!profileImageData?.image || profileImageData?.image==''; else Img; let image"> | ||
| @if (!profileImageData?.image || profileImageData?.image=='') { |
There was a problem hiding this comment.
In multiple places we are using the same condition so move this logic to ts. get hasImage(): boolean {
return !!this.profileImageData?.image;
} and in html use like this : @if (!hasImage)
| <div *ngIf="profileImageData?.location"> | ||
| } | ||
| @if (profileImageData?.location) { | ||
| <div> |
There was a problem hiding this comment.
Div is not required for this
| <ng-container *ngFor="let data of selectedData; let i = index"> | ||
| <ion-chip *ngIf="i < 5 && (data.name || data.label) && control.meta.addPopupType != 'file'" class="color-red" [disabled]="control?.disabled || data?.type === 'ENROLLED'"> | ||
| } | ||
| @for (data of selectedData; track $index) { |
There was a problem hiding this comment.
dont use track by index anywhere
| <ion-chip (click)="viewSelectedList()" class="color-red" | ||
| [disabled]="control?.disabled"> | ||
| <ion-icon class="icon" name="add-outline" color="light"></ion-icon> | ||
| <ion-label>{{selectedData.length-5}} more</ion-label> |
There was a problem hiding this comment.
recalculates each change detection. beter move this logic into ts
9e3f1d7
into
ELEVATE-Project:release-3.4.0_ng20
…on creation flow