akhilpb001 commented on code in PR #94:
URL: https://github.com/apache/yunikorn-web/pull/94#discussion_r1028903092
##########
src/app/components/apps-view/apps-view.component.html:
##########
@@ -20,10 +20,7 @@
<div class="dropdown-wrapper">
<label class="dropdown-label">Partition: </label>
<mat-form-field>
- <mat-select
- [(value)]="partitionSelected"
- (selectionChange)="onPartitionSelectionChanged($event)"
- >
+ <mat-select [(value)]="partitionSelected"
(selectionChange)="onPartitionSelectionChanged($event)">
Review Comment:
Pls use prettier in your IDE so that it honors prettierrc config file and
does not do unnecessary file formatting as part of the PR changes. I see lot of
file formatting changes in the files.
##########
src/app/components/apps-view/apps-view.component.html:
##########
@@ -41,20 +38,9 @@
</mat-form-field>
</div>
<mat-form-field class="search-wrapper white-mat-form-field">
- <input
Review Comment:
Pls use prettier in your IDE so that it honors prettierrc config file and
does not do unnecessary file formatting as part of the PR changes.
##########
src/app/components/apps-view/apps-view.component.ts:
##########
@@ -79,6 +79,7 @@ export class AppsViewComponent implements OnInit {
{ colId: 'applicationId', colName: 'Application ID' },
{ colId: 'queueName', colName: 'Queue Name' },
{ colId: 'applicationState', colName: 'Application State' },
+ { colId: 'lastStateChangeTime', colName: 'Last State Change Time' },
Review Comment:
This could be like:
```
{
colId: 'lastStateChangeTime',
colName: 'Last State Change Time',
colFormatter: CommonUtil.timeColumnFormatter,
},
```
##########
src/app/components/apps-view/apps-view.component.html:
##########
@@ -85,10 +71,16 @@
<ng-container [matColumnDef]="columnDef.colId" *ngFor="let columnDef of
appColumnDef">
<mat-header-cell *matHeaderCellDef mat-sort-header>{{
columnDef.colName }}</mat-header-cell>
- <ng-container *ngIf="columnDef.colId === 'submissionTime'; else
renderNext_1">
+ <ng-container *ngIf="columnDef.colId === 'submissionTime'; else
renderNext_4">
Review Comment:
I think we can do better rendering for submissionTime and
lastStateChangeTime.
1. Move formatTimeIfNotNull function to common-util file as
timeColumnFormatter. Use this as colFormatter for submissionTime as well as
lastStateChangeTime.
2. Here check for colFormatter and use innerText to render the text. Like:
```
<ng-container *ngIf="columnDef.colId === 'usedResource'; else
renderNext_1">
<mat-cell *matCellDef="let element">
<ng-container *ngIf="columnDef.colFormatter; else
showAppRawData">
<span
[innerHTML]="columnDef.colFormatter(element[columnDef.colId])"></span>
</ng-container>
<ng-template #showAppRawData>
<span>{{ element[columnDef.colId] }}</span>
</ng-template>
</mat-cell>
</ng-container>
<ng-template #renderNext_1>
<ng-container *ngIf="columnDef.colFormatter; else renderNext_2">
<mat-cell *matCellDef="let element">
<span
[innerText]="columnDef.colFormatter(element[columnDef.colId])"></span>
</mat-cell>
</ng-container>
</ng-template>
<ng-template #renderNext_2>
<mat-cell *matCellDef="let element">{{ element[columnDef.colId] ||
'n/a' }}</mat-cell>
</ng-template>
</ng-container>
```
##########
src/app/components/apps-view/apps-view.component.html:
##########
@@ -41,20 +38,9 @@
</mat-form-field>
</div>
<mat-form-field class="search-wrapper white-mat-form-field">
- <input
- matInput
- type="text"
- [(ngModel)]="searchText"
- placeholder="Search By Application ID"
- #searchInput
- />
- <button
- class="clear-btn"
- *ngIf="searchText"
- (click)="onClearSearch()"
- matTooltip="Clear Search"
- matTooltipShowDelay="500"
- >
+ <input matInput type="text" [(ngModel)]="searchText" placeholder="Search
By Application ID" #searchInput />
+ <button class="clear-btn" *ngIf="searchText" (click)="onClearSearch()"
matTooltip="Clear Search"
+ matTooltipShowDelay="500">
<i class="far fa-times-circle"></i>
</button>
<i *ngIf="!searchText" class="fas fa-search search-icon"></i>
Review Comment:
Remove below commented section of code.
##########
src/app/components/apps-view/apps-view.component.ts:
##########
@@ -79,6 +79,7 @@ export class AppsViewComponent implements OnInit {
{ colId: 'applicationId', colName: 'Application ID' },
{ colId: 'queueName', colName: 'Queue Name' },
{ colId: 'applicationState', colName: 'Application State' },
+ { colId: 'lastStateChangeTime', colName: 'Last State Change Time' },
Review Comment:
Same for submissionTime too.
```
{
colId: 'submissionTime',
colName: 'Submission Time',
colFormatter: CommonUtil.timeColumnFormatter,
},
```
##########
src/app/models/app-info.model.ts:
##########
@@ -31,24 +31,35 @@ export class AppInfo {
public queueName: string,
public submissionTime: number,
public finishedTime: null | number,
+ public lastStateChangeTime: null | number,
public applicationState: string,
public allocations: AllocationInfo[] | null
- ) {}
+ ) { }
get formattedSubmissionTime() {
- const millisecs = Math.round(this.submissionTime / (1000 * 1000));
- return moment(millisecs).format('YYYY/MM/DD HH:mm:ss');
+ return this.formatTime(this.submissionTime)
}
get formattedFinishedTime() {
- if (this.finishedTime) {
- const millisecs = Math.round(this.finishedTime / (1000 * 1000));
- return moment(millisecs).format('YYYY/MM/DD HH:mm:ss');
- }
+ return this.formatTimeIfNotNull(this.finishedTime)
+ }
+
+ get formattedLastStateChangeTime() {
+ return this.formatTimeIfNotNull(this.lastStateChangeTime)
+ }
+ formatTimeIfNotNull(unformattedtime: null | number) {
Review Comment:
Move this to common-util as `timeColumnFormatter`.
##########
src/app/models/app-info.model.ts:
##########
@@ -31,24 +31,35 @@ export class AppInfo {
public queueName: string,
public submissionTime: number,
public finishedTime: null | number,
+ public lastStateChangeTime: null | number,
public applicationState: string,
public allocations: AllocationInfo[] | null
- ) {}
+ ) { }
get formattedSubmissionTime() {
- const millisecs = Math.round(this.submissionTime / (1000 * 1000));
- return moment(millisecs).format('YYYY/MM/DD HH:mm:ss');
+ return this.formatTime(this.submissionTime)
}
get formattedFinishedTime() {
- if (this.finishedTime) {
- const millisecs = Math.round(this.finishedTime / (1000 * 1000));
- return moment(millisecs).format('YYYY/MM/DD HH:mm:ss');
- }
+ return this.formatTimeIfNotNull(this.finishedTime)
+ }
+
+ get formattedLastStateChangeTime() {
+ return this.formatTimeIfNotNull(this.lastStateChangeTime)
+ }
+ formatTimeIfNotNull(unformattedtime: null | number) {
+ if (unformattedtime) {
+ return this.formatTime(unformattedtime)
Review Comment:
Use code from formatTime here. No need of formatTime function.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
