Skip to content

Commit 15c6a72

Browse files
pmachapmanpmachapman
committed
Code review updates
1 parent 753bd02 commit 15c6a72

21 files changed

+324
-232
lines changed

‎src/SIL.XForge.Scripture/ClientApp/src/app/machine-api/build-dto.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ export interface ServalBuildAdditionalInfo {
1616
buildId: string;
1717
corporaIds?: string[];
1818
dateFinished?: string;
19-
dateRequested?: string;
2019
dateGenerated?: string;
20+
dateRequested?: string;
2121
parallelCorporaIds?: string[];
2222
step: number;
2323
trainingScriptureRanges: ProjectScriptureRange[];

‎src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-download-button/draft-download-button.component.html

+47-24
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,49 @@
11
<ng-container *transloco="let t; read: 'draft_generation'">
2-
<button
3-
mat-button
4-
type="button"
5-
(click)="downloadDraft()"
6-
[disabled]="downloadProgress > 0"
7-
data-test-id="download-button"
8-
color="primary"
9-
[ngClass]="flat ? 'mat-mdc-unelevated-button' : ''"
10-
>
11-
@if (downloadProgress === 0) {
12-
<mat-icon [ngClass]="flat ? '' : 'material-icons-outlined'">cloud_download</mat-icon>
13-
} @else if (downloadProgress > 0) {
14-
<mat-icon>
15-
<mat-spinner
16-
diameter="18"
17-
[value]="downloadProgress"
18-
mode="determinate"
19-
color="accent"
20-
data-test-id="download-spinner"
21-
></mat-spinner>
22-
</mat-icon>
23-
}
24-
<span>{{ t("download_draft") }}</span>
25-
</button>
2+
@if (flat) {
3+
<button
4+
mat-flat-button
5+
(click)="downloadDraft()"
6+
[disabled]="downloadProgress > 0"
7+
data-test-id="download-button"
8+
color="primary"
9+
>
10+
@if (downloadProgress === 0) {
11+
<mat-icon class="material-icons-outlined">cloud_download</mat-icon>
12+
} @else if (downloadProgress > 0) {
13+
<mat-icon>
14+
<mat-spinner
15+
diameter="18"
16+
[value]="downloadProgress"
17+
mode="determinate"
18+
color="accent"
19+
data-test-id="download-spinner"
20+
></mat-spinner>
21+
</mat-icon>
22+
}
23+
{{ t("download_draft") }}
24+
</button>
25+
} @else {
26+
<button
27+
mat-button
28+
(click)="downloadDraft()"
29+
[disabled]="downloadProgress > 0"
30+
data-test-id="download-button"
31+
color="primary"
32+
>
33+
@if (downloadProgress === 0) {
34+
<mat-icon>cloud_download</mat-icon>
35+
} @else if (downloadProgress > 0) {
36+
<mat-icon>
37+
<mat-spinner
38+
diameter="18"
39+
[value]="downloadProgress"
40+
mode="determinate"
41+
color="accent"
42+
data-test-id="download-spinner"
43+
></mat-spinner>
44+
</mat-icon>
45+
}
46+
{{ t("download_draft") }}
47+
</button>
48+
}
2649
</ng-container>

‎src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.html

+5-2
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ <h3>{{ t("draft_finishing_header") }}</h3>
278278
</mat-card-content>
279279
<mat-card-actions>
280280
@if (hasDraftBooksAvailable) {
281-
<app-draft-download-button [build]="lastCompletedBuild"></app-draft-download-button>
281+
<app-draft-download-button [build]="lastCompletedBuild" />
282282
}
283283
</mat-card-actions>
284284
</mat-card>
@@ -334,7 +334,10 @@ <h3>{{ t("draft_finishing_header") }}</h3>
334334
</section>
335335

336336
@if (featureFlags.newDraftHistory.enabled) {
337-
<app-draft-history-list></app-draft-history-list>
337+
<app-draft-history-list />
338+
@if (isServalAdmin()) {
339+
<h2>Further information</h2>
340+
}
338341
}
339342

340343
@if (canShowAdditionalInfo(draftJob)) {

‎src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.service.spec.ts

+5-5
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ describe('DraftGenerationService', () => {
168168
it('should return undefined for a 401 error', fakeAsync(() => {
169169
// SUT
170170
service.getBuildHistory(projectId).subscribe(result => {
171-
expect(result).toEqual(undefined);
171+
expect(result).toBeUndefined();
172172
verify(mockNoticeService.showError(anything())).once();
173173
});
174174
tick();
@@ -185,7 +185,7 @@ describe('DraftGenerationService', () => {
185185
it('should return undefined for a 404 error', fakeAsync(() => {
186186
// SUT
187187
service.getBuildHistory(projectId).subscribe(result => {
188-
expect(result).toEqual(undefined);
188+
expect(result).toBeUndefined();
189189
verify(mockNoticeService.showError(anything())).never();
190190
});
191191
tick();
@@ -617,7 +617,7 @@ describe('DraftGenerationService', () => {
617617

618618
// SUT
619619
service.getGeneratedDraftHistory(projectId, book, chapter).subscribe(result => {
620-
expect(result).toEqual(undefined);
620+
expect(result).toBeUndefined();
621621
verify(mockNoticeService.showError(anything())).once();
622622
});
623623
tick();
@@ -637,7 +637,7 @@ describe('DraftGenerationService', () => {
637637

638638
// SUT
639639
service.getGeneratedDraftHistory(projectId, book, chapter).subscribe(result => {
640-
expect(result).toEqual(undefined);
640+
expect(result).toBeUndefined();
641641
verify(mockNoticeService.showError(anything())).never();
642642
});
643643
tick();
@@ -658,7 +658,7 @@ describe('DraftGenerationService', () => {
658658

659659
// SUT
660660
service.getGeneratedDraftHistory(projectId, book, chapter).subscribe(result => {
661-
expect(result).toEqual(undefined);
661+
expect(result).toBeUndefined();
662662
});
663663
tick();
664664
}));

‎src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.service.ts

+11-3
Original file line numberDiff line numberDiff line change
@@ -314,12 +314,20 @@ export class DraftGenerationService {
314314
const usfmFiles: Promise<void>[] = [];
315315

316316
// Build the list of book numbers, first checking the build, then the project document if that is null
317-
const books: number[] =
317+
let books = new Set<number>(
318318
lastCompletedBuild?.additionalInfo?.translationScriptureRanges?.flatMap(range =>
319319
booksFromScriptureRange(range.scriptureRange)
320-
) ?? projectDoc.data.texts.filter(text => text.chapters.some(c => c.hasDraft)).map(text => text.bookNum);
320+
)
321+
);
322+
323+
// If no books were found in the build, use the project document
324+
if (books.size === 0) {
325+
books = new Set<number>(
326+
projectDoc.data.texts.filter(text => text.chapters.some(c => c.hasDraft)).map(text => text.bookNum)
327+
);
328+
}
321329

322-
const zipProgress: DraftZipProgress = { current: 0, total: books.length };
330+
const zipProgress: DraftZipProgress = { current: 0, total: books.size };
323331
observer.next(zipProgress);
324332

325333
// Get the date the draft was generated and written to Scripture Forge

‎src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-handling.service.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ export class DraftHandlingService {
162162
// If the corpus does not support USFM, use the legacy format.
163163
// The legacy format does not support a timestamp
164164
if (err.status === 405 && timestamp == null) {
165-
return this.getDraft(textDocId, { isDraftLegacy: true, timestamp });
165+
return this.getDraft(textDocId, { isDraftLegacy: true, timestamp: undefined });
166166
}
167167

168168
return throwError(() => err);

‎src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/_draft-history-entry-theme.scss

+3-3
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@
33
@mixin color($theme) {
44
$is-dark: mat.get-theme-type($theme) == dark;
55

6-
--draft-history-entry-heading-background-color: #{mat.get-theme-color($theme, neutral, if($is-dark, 20, 95))};
7-
--draft-history-entry-heading-background-hover-color: #{mat.get-theme-color($theme, neutral, if($is-dark, 30, 90))};
8-
--draft-history-entry-details-background-color: #{mat.get-theme-color($theme, neutral, if($is-dark, 10, 98))};
6+
--draft-history-entry-red-color: #{if($is-dark, red, darkRed)};
7+
--draft-history-entry-green-color: #{if($is-dark, lightGreen, darkGreen)};
8+
--draft-history-entry-grey-color: lightGrey;
99
}
1010

1111
@mixin theme($theme) {

‎src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.html

+46-36
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,54 @@
11
<ng-container *transloco="let t; read: 'draft_history_entry'">
22
@if (entry != null) {
3-
<div class="draft-entry-heading" (click)="headerClicked()" [class.expandable]="!forceDetailsOpen">
4-
<div class="entry-description">
5-
<span class="drafted-books">{{ i18n.enumerateList(bookNames) }}</span>
6-
<span class="date">{{ formatDate(entry.additionalInfo?.dateFinished) }}</span>
7-
</div>
8-
<span class="spacer"></span>
9-
<span class="status" [style.color]="getStatus(entry.state).color">
10-
<mat-icon class="status-icon">{{ getStatus(entry.state).icons }}</mat-icon>
11-
{{ t(getStatus(entry.state).text) }}
12-
</span>
13-
@if (!forceDetailsOpen && hasDetails) {
14-
<mat-icon class="expand-icon">expand_{{ detailsOpen ? "less" : "more" }}</mat-icon>
15-
}
16-
</div>
17-
@if (detailsOpen && hasDetails) {
3+
<mat-expansion-panel
4+
[disabled]="forceDetailsOpen || !hasDetails"
5+
[expanded]="forceDetailsOpen"
6+
[hideToggle]="forceDetailsOpen || !hasDetails"
7+
>
8+
<mat-expansion-panel-header [collapsedHeight]="'auto'" [expandedHeight]="'auto'">
9+
<mat-panel-title>
10+
<span class="drafted-books">{{ i18n.enumerateList(bookNames) }}</span>
11+
<span class="date">{{ formatDate(entry.additionalInfo?.dateFinished) }}</span>
12+
</mat-panel-title>
13+
<mat-panel-description>
14+
<span class="status" [ngClass]="getStatus(entry.state).color">
15+
<mat-icon class="status-icon">{{ getStatus(entry.state).icons }}</mat-icon>
16+
{{ t(getStatus(entry.state).text) }}
17+
</span>
18+
</mat-panel-description>
19+
</mat-expansion-panel-header>
1820
<div class="draft-entry-body">
19-
<ng-container *ngIf="canDownloadBuild">
21+
@if (canDownloadBuild) {
2022
<p>{{ t("click_book_to_preview") }}</p>
2123
<p class="book-buttons">
22-
<app-draft-preview-books [build]="entry"></app-draft-preview-books>
23-
</p>
24-
<app-draft-download-button [build]="entry" [flat]="true"></app-draft-download-button>
25-
</ng-container>
26-
<ng-container *ngIf="hasTrainingData">
27-
<p *ngIf="canDownloadBuild">
28-
<a
29-
href="javascript:;"
30-
(click)="$event.preventDefault(); trainingDataOpen = !trainingDataOpen"
31-
class="training-data-link"
32-
>
33-
<mat-icon>expand_{{ trainingDataOpen ? "less" : "more" }}</mat-icon>
34-
{{ t(trainingDataOpen ? "hide_model_training_data" : "show_model_training_data") }}
35-
</a>
24+
<app-draft-preview-books [build]="entry" />
3625
</p>
37-
@if (trainingDataOpen) {
38-
<p class="training-data">
39-
<strong>{{ t("training_data_configuration") }}</strong>
26+
<app-draft-download-button [build]="entry" [flat]="true" />
27+
}
28+
@if (hasTrainingConfiguration) {
29+
@if (canDownloadBuild) {
30+
<p>
31+
<a
32+
href="javascript:;"
33+
(click)="trainingConfigurationOpen = !trainingConfigurationOpen"
34+
class="training-configuration-link"
35+
>
36+
<mat-icon>expand_{{ trainingConfigurationOpen ? "less" : "more" }}</mat-icon>
37+
{{
38+
t(
39+
trainingConfigurationOpen
40+
? "hide_model_training_configuration"
41+
: "show_model_training_configuration"
42+
)
43+
}}
44+
</a>
45+
</p>
46+
}
47+
@if (trainingConfigurationOpen) {
48+
<p>
49+
<strong>{{ t("training_model_description") }}</strong>
4050
</p>
41-
<table mat-table [dataSource]="trainingData">
51+
<table mat-table [dataSource]="trainingConfiguration">
4252
<ng-container matColumnDef="bookNames">
4353
<th mat-header-cell *matHeaderCellDef></th>
4454
<td mat-cell *matCellDef="let element">{{ i18n.enumerateList(element.bookNames) }}</td>
@@ -59,8 +69,8 @@
5969
<tr mat-row *matRowDef="let row; columns: columnsToDisplay"></tr>
6070
</table>
6171
}
62-
</ng-container>
72+
}
6373
</div>
64-
}
74+
</mat-expansion-panel>
6575
}
6676
</ng-container>

‎src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-history-list/draft-history-entry/draft-history-entry.component.scss

+28-28
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,24 @@
1-
:host {
2-
background-color: var(--draft-history-entry-details-background-color);
3-
border-radius: 8px;
4-
> * {
5-
padding: 8px 16px;
6-
border-radius: 8px;
7-
}
1+
mat-expansion-panel-header {
2+
padding: 8px 16px;
83
}
94

10-
.draft-entry-heading {
11-
background-color: var(--draft-history-entry-heading-background-color);
5+
mat-panel-title {
126
display: flex;
13-
align-items: center;
14-
gap: 1em;
15-
&.expandable {
16-
cursor: pointer;
17-
}
18-
&.expandable:hover {
19-
background-color: var(--draft-history-entry-heading-background-hover-color);
20-
}
21-
}
22-
23-
.drafted-books {
24-
font-size: 1.1em;
7+
align-items: flex-start;
8+
flex-direction: column;
9+
flex-grow: 1;
2510
}
2611

27-
.spacer {
28-
flex-grow: 1;
12+
mat-panel-description {
13+
flex-grow: 0;
2914
}
3015

31-
.entry-description {
32-
display: flex;
33-
flex-direction: column;
34-
gap: 4px;
16+
.drafted-books {
17+
color: var(
18+
--mat-expansion-header-text-color,
19+
var(--mat-app-on-surface)
20+
); /* Keep the text color, even when disabled */
21+
font-size: 1.1em;
3522
}
3623

3724
.date {
@@ -49,7 +36,7 @@
4936
flex-shrink: 0;
5037
}
5138

52-
.training-data-link {
39+
.training-configuration-link {
5340
display: flex;
5441
align-items: center;
5542
}
@@ -62,3 +49,16 @@ strong {
6249
display: flex;
6350
gap: 4px;
6451
}
52+
53+
/* Color classes for the status icon */
54+
.grey {
55+
color: var(--draft-history-entry-grey-color);
56+
}
57+
58+
.green {
59+
color: var(--draft-history-entry-green-color);
60+
}
61+
62+
.red {
63+
color: var(--draft-history-entry-red-color);
64+
}

0 commit comments

Comments
 (0)