Commit 6f590587 by Pankaj

CodeReview : VQODE - Usman : Sprint 2 : Overall code is good. Managed for…

CodeReview : VQODE - Usman : Sprint 2 : Overall code is good. Managed for component & UI properly. Have to update code to reuse & format properly
parent 908833ff
// @CodeReview : G008 : Similar style is been used as in "work-preference.component.scss", it must be reused (manage similar thing in whole applicaiton)
@import "../../../../styles/my-career-styles/variables"; @import "../../../../styles/my-career-styles/variables";
.assessment-wrapper { .assessment-wrapper {
......
// @CodeReview : A008 : imports to be in proper order (already informed to use typehero plugin, which do the same), do this on all pages in system
import { Component, OnInit } from '@angular/core'; import { Component, OnInit } from '@angular/core';
import { BaseComponent } from '../../base/base.component'; import { BaseComponent } from '../../base/base.component';
import { AssessmentService } from '../../services/assessment.service'; import { AssessmentService } from '../../services/assessment.service';
...@@ -19,6 +21,9 @@ export class DiversityProfileComponent extends BaseComponent implements OnInit { ...@@ -19,6 +21,9 @@ export class DiversityProfileComponent extends BaseComponent implements OnInit {
assocs = ['Answers']; assocs = ['Answers'];
autoSaveInterval; autoSaveInterval;
autoSavedOn; autoSavedOn;
// @CodeReview : G008 : Similar variable is been defined in other pages as well, please define in common place and reuse it
autoSaveLabel = 'a few seconds ago'; autoSaveLabel = 'a few seconds ago';
selectedAnswers = []; selectedAnswers = [];
...@@ -91,6 +96,7 @@ export class DiversityProfileComponent extends BaseComponent implements OnInit { ...@@ -91,6 +96,7 @@ export class DiversityProfileComponent extends BaseComponent implements OnInit {
}); });
} }
// @CodeReview : G008 : Similar method setAutoSaveInterval is been created in other pages as well, please define in common place and reuse it
setAutoSaveInterval(): void { setAutoSaveInterval(): void {
if (!this.autoSaveInterval) { if (!this.autoSaveInterval) {
this.autoSave(); this.autoSave();
...@@ -100,6 +106,7 @@ export class DiversityProfileComponent extends BaseComponent implements OnInit { ...@@ -100,6 +106,7 @@ export class DiversityProfileComponent extends BaseComponent implements OnInit {
} }
} }
// @CodeReview : G008 : Similar method setAutoSaveInterval is been created in other pages as well, please define in common place and reuse it
autoSave(): void { autoSave(): void {
this.autoSaveLabel = moment(this.autoSavedOn).fromNow(); this.autoSaveLabel = moment(this.autoSavedOn).fromNow();
} }
......
// @CodeReview : A008 : imports to be in proper order (already informed to use typehero plugin, which do the same), do this on all pages in system
import { Component, OnDestroy, OnInit } from '@angular/core'; import { Component, OnDestroy, OnInit } from '@angular/core';
import { BaseComponent } from '../../base/base.component'; import { BaseComponent } from '../../base/base.component';
import * as moment from 'moment'; import * as moment from 'moment';
......
// @CodeReview : : Similar style is been used as in "work-preference.component.scss", it must be reused
@import "../../../../styles/my-career-styles/variables"; @import "../../../../styles/my-career-styles/variables";
.assessment-wrapper { .assessment-wrapper {
......
// @CodeReview : A008 : imports to be in proper order (already informed to use typehero plugin, which do the same)
import { Component, OnDestroy, OnInit } from '@angular/core'; import { Component, OnDestroy, OnInit } from '@angular/core';
import { BaseComponent } from '../../base/base.component'; import { BaseComponent } from '../../base/base.component';
import { AssessmentService } from '../../services/assessment.service'; import { AssessmentService } from '../../services/assessment.service';
......
<div class="assessment-wrapper row"> <div class="assessment-wrapper row">
<div class="col-12" style="padding: 0;"> <div class="col-12" style="padding: 0;">
<!-- @CodeReview : G008 : Header is been added in all pages separately, better to move this portion to common area, so that will not be redundant -->
<div class="assessment-header"> <div class="assessment-header">
<div class="header-logo" *ngIf="!isMobileView"> <div class="header-logo" *ngIf="!isMobileView">
<img src="assets/my-career-web/images/MATCHD_LOGO.jpg" alt=""> <img src="assets/my-career-web/images/MATCHD_LOGO.jpg" alt="">
......
// @CodeReview : : Similar style is been used in assessments as well
@import "../../../../styles/my-career-styles/variables"; @import "../../../../styles/my-career-styles/variables";
.assessment-wrapper { .assessment-wrapper {
......
// @CodeReview : A008 : imports to be in proper order (already informed to use typehero plugin, which do the same), do this on all pages in system
import { Component, OnInit, ViewChild } from '@angular/core'; import { Component, OnInit, ViewChild } from '@angular/core';
import { BaseComponent } from '../../base/base.component'; import { BaseComponent } from '../../base/base.component';
import { ActivatedRoute, Router } from '@angular/router'; import { ActivatedRoute, Router } from '@angular/router';
......
...@@ -29,6 +29,8 @@ ...@@ -29,6 +29,8 @@
<button class="mc-btn-primary" pButton label="Preview Profile"></button> <button class="mc-btn-primary" pButton label="Preview Profile"></button>
</div> </div>
<!-- @CodeReview : : please remove such empty lines from whole application, if it's not placed intentionally -->
<h4 *ngIf="!asChild" style="font-weight: bold;">Update Career Profile</h4> <h4 *ngIf="!asChild" style="font-weight: bold;">Update Career Profile</h4>
...@@ -74,6 +76,8 @@ ...@@ -74,6 +76,8 @@
<div class="row mt-4" *ngIf="careerProfile?.PublishProfile"> <div class="row mt-4" *ngIf="careerProfile?.PublishProfile">
<div class="col-md-6 d-flex justify-content-between"> <div class="col-md-6 d-flex justify-content-between">
<span class="ml-4" style="white-space: nowrap;">Share URL</span> <span class="ml-4" style="white-space: nowrap;">Share URL</span>
<!-- @CodeReview : G000 : Why this hard coded share URL is been placed here ? -->
<a target="_blank" style="word-break: break-all; margin-left: 80px;" href="#">http://career.matchd.com/john+peters4</a> <a target="_blank" style="word-break: break-all; margin-left: 80px;" href="#">http://career.matchd.com/john+peters4</a>
</div> </div>
</div> </div>
......
// @CodeReview : A008 : imports to be in proper order (already informed to use typehero plugin, which do the same), do this on all pages in system
import { Component, EventEmitter, OnInit, Output, ViewChild } from '@angular/core'; import { Component, EventEmitter, OnInit, Output, ViewChild } from '@angular/core';
import { BaseComponent } from '../../../base/base.component'; import { BaseComponent } from '../../../base/base.component';
import { takeUntil } from 'rxjs/operators'; import { takeUntil } from 'rxjs/operators';
......
// @CodeReview : A008 : imports to be in proper order (already informed to use typehero plugin, which do the same), do this on all pages in system
import { Component, OnInit } from '@angular/core'; import { Component, OnInit } from '@angular/core';
import * as FileSaver from 'file-saver'; import * as FileSaver from 'file-saver';
import { takeUntil } from 'rxjs/operators'; import { takeUntil } from 'rxjs/operators';
...@@ -15,6 +17,9 @@ import { HelperService } from '../../services/helper.service'; ...@@ -15,6 +17,9 @@ import { HelperService } from '../../services/helper.service';
export class SummaryReportComponent extends BaseComponent implements OnInit { export class SummaryReportComponent extends BaseComponent implements OnInit {
showBody = false; showBody = false;
// @CodeReview : : Try to manage 'colorClasses' in html / scss (as style in "ts" is not good way),
// but considering compleity of this report, leave it if can't be.
colorClasses = { colorClasses = {
'percent-yellow-b': {incompleteColor: '#e5e8eb', completeColor: '#fedd6f'} 'percent-yellow-b': {incompleteColor: '#e5e8eb', completeColor: '#fedd6f'}
}; };
......
...@@ -51,6 +51,8 @@ ...@@ -51,6 +51,8 @@
<td> <td>
<div class="exp-row"> <div class="exp-row">
<i class="fa fa-bars" pReorderableRowHandle></i> <i class="fa fa-bars" pReorderableRowHandle></i>
<!-- @CodeReview : A004 : break line, if it's going out of view (whenever possible) -->
<p-checkbox [binary]="true" [(ngModel)]="ILWorkExperience.IsSelected" name="EXP{{index}}" [label]="ILWorkExperience?.WorkExperience?.ActualJobTitle + ' - ' + ILWorkExperience?.WorkExperience?.Employer"></p-checkbox> <p-checkbox [binary]="true" [(ngModel)]="ILWorkExperience.IsSelected" name="EXP{{index}}" [label]="ILWorkExperience?.WorkExperience?.ActualJobTitle + ' - ' + ILWorkExperience?.WorkExperience?.Employer"></p-checkbox>
</div> </div>
</td> </td>
......
...@@ -76,6 +76,7 @@ export class IntroLetterComponent extends BaseComponent implements OnInit { ...@@ -76,6 +76,7 @@ export class IntroLetterComponent extends BaseComponent implements OnInit {
getIntroLetter(download = false): void { getIntroLetter(download = false): void {
this.isLoading = true; this.isLoading = true;
// @CodeReview : A004 : break line, if it's going out of view (whenever possible)
const sub = download ? this.introLetterService.downloadIntroLetter(this.introLetterId) : this.introLetterService.getIntroLetter(this.introLetterId, ['WorkExperiences.WorkExperience', 'Skills.Skill']); const sub = download ? this.introLetterService.downloadIntroLetter(this.introLetterId) : this.introLetterService.getIntroLetter(this.introLetterId, ['WorkExperiences.WorkExperience', 'Skills.Skill']);
sub sub
.pipe(takeUntil(this.componentInView)) .pipe(takeUntil(this.componentInView))
......
...@@ -14,6 +14,9 @@ export class IntroLettersComponent extends BaseComponent implements OnInit { ...@@ -14,6 +14,9 @@ export class IntroLettersComponent extends BaseComponent implements OnInit {
introLetters: IntroductionLetterModel[] = []; introLetters: IntroductionLetterModel[] = [];
// @CodeReview : G000 : Please give proper name to package (this seems : "list into letters"), please update relevant variables as well (if any).
constructor( constructor(
private introLetterService: IntroLetterService, private introLetterService: IntroLetterService,
private utilService: UtilsService private utilService: UtilsService
......
...@@ -51,6 +51,10 @@ export class TemplatesComponent extends BaseComponent implements OnInit { ...@@ -51,6 +51,10 @@ export class TemplatesComponent extends BaseComponent implements OnInit {
|| t.Availability === this.tempalteStatus.INCLUDED_IN_PREMIUM; || t.Availability === this.tempalteStatus.INCLUDED_IN_PREMIUM;
t.ImgPath = this.apiBase + t.FullImageURI t.ImgPath = this.apiBase + t.FullImageURI
}); });
// @CodeReview : A101 : Sorting can be done by util's method, please try to use that (if possible)
// this.utilsService.sort(this.careerProfile.Candidate.AllTempletes, ['SortOrder'], [1]);
this.careerProfile.Candidate.AllTempletes.sort((t1, t2) => t1.SortOrder - t2.SortOrder); this.careerProfile.Candidate.AllTempletes.sort((t1, t2) => t1.SortOrder - t2.SortOrder);
} }
this.careerProfileLoaded.emit(this.careerProfile); this.careerProfileLoaded.emit(this.careerProfile);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment