Skip to content

Commit 9fa2ebb

Browse files
committed
SF-3322 Allow selecting book and chapter in community checking
1 parent 6e96876 commit 9fa2ebb

File tree

4 files changed

+51
-22
lines changed

4 files changed

+51
-22
lines changed

src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking-questions/checking-questions.component.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,8 @@ export class CheckingQuestionsComponent implements OnInit, OnChanges {
166166
// Handle changes to questionDocs in ngOnChanges instead of setter to ensure other @Inputs are set
167167
// when 'activateStoredQuestion' is called, such as 'routeBookChapter'.
168168
const questionDocs: Readonly<QuestionDoc[] | undefined> = changes.questionDocs?.currentValue;
169-
if (questionDocs != null) {
169+
if (questionDocs != null && this.activeQuestionDoc == null) {
170+
// Only activate the stored question if no previous question was selected
170171
if (questionDocs.length > 0) {
171172
this.activateStoredQuestion({ isQuestionListChange: true });
172173
} else {

src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking.component.html

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,7 @@ <h2 matSubheader>{{ t("filter_questions") }}</h2>
7171
[book]="book"
7272
[chapters]="chapters"
7373
[chapter]="chapter"
74-
[bookSelectDisabled]="activeQuestionScope === 'all'"
75-
[chapterSelectDisabled]="activeQuestionScope !== 'chapter'"
76-
[prevNextHidden]="activeQuestionScope !== 'chapter' || isScreenSmall"
74+
[prevNextHidden]="isScreenSmall"
7775
(bookChange)="onBookSelect($event)"
7876
(chapterChange)="onChapterSelect($event)"
7977
>

src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking.component.spec.ts

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,7 @@ describe('CheckingComponent', () => {
374374
env.setBookChapter('JHN', 2);
375375
expect(env.getQuestionText(question)).toBe('John 2');
376376
expect(await env.getCurrentBookAndChapter()).toBe('John 2');
377+
env.waitForQuestionTimersToComplete();
377378
}));
378379

379380
it('start question respects route book/chapter', fakeAsync(async () => {
@@ -569,8 +570,7 @@ describe('CheckingComponent', () => {
569570
expect(env.isSegmentHighlighted(1, 5)).toBe(true);
570571
expect(env.segmentHasQuestion(1, 5)).toBe(true);
571572
expect(env.component.questionVerseRefs.some(verseRef => verseRef.equals(new VerseRef('JHN 1:5')))).toBe(true);
572-
tick();
573-
flush();
573+
env.waitForQuestionTimersToComplete();
574574
}));
575575

576576
it('records audio for question when button clicked', fakeAsync(() => {
@@ -851,6 +851,38 @@ describe('CheckingComponent', () => {
851851
env.waitForQuestionTimersToComplete();
852852
}));
853853

854+
it('resets filter after a user changes chapter', fakeAsync(() => {
855+
const env = new TestEnvironment({
856+
user: ADMIN_USER,
857+
projectBookRoute: 'JHN',
858+
projectChapterRoute: 1,
859+
questionScope: 'chapter'
860+
});
861+
expect(env.questions.length).toEqual(14);
862+
env.setQuestionScope('all');
863+
env.setBookChapter('JHN', 2);
864+
tick();
865+
env.fixture.detectChanges();
866+
expect(env.component.activeQuestionScope).toEqual('all');
867+
env.waitForQuestionTimersToComplete();
868+
}));
869+
870+
it('resets filter after a user changes book', fakeAsync(() => {
871+
const env = new TestEnvironment({
872+
user: ADMIN_USER,
873+
projectBookRoute: 'JHN',
874+
projectChapterRoute: 1,
875+
questionScope: 'chapter'
876+
});
877+
expect(env.questions.length).toEqual(14);
878+
env.setQuestionScope('all');
879+
env.setBookChapter('MAT', 1);
880+
tick();
881+
env.fixture.detectChanges();
882+
expect(env.component.activeQuestionScope).toEqual('all');
883+
env.waitForQuestionTimersToComplete();
884+
}));
885+
854886
it('can narrow questions scope', fakeAsync(() => {
855887
const env = new TestEnvironment({
856888
user: ADMIN_USER,
@@ -921,7 +953,8 @@ describe('CheckingComponent', () => {
921953
env.waitForQuestionTimersToComplete();
922954

923955
expect(spyUpdateQuestionRefs).toHaveBeenCalledTimes(1);
924-
expect(spyRefreshSummary).toHaveBeenCalledTimes(1);
956+
// Called at least once or more depending on if another question replaces the archived question
957+
expect(spyRefreshSummary).toHaveBeenCalled();
925958
}));
926959
});
927960

@@ -1913,8 +1946,9 @@ describe('CheckingComponent', () => {
19131946
const env = new TestEnvironment({ user: CHECKER_USER });
19141947
const questionDoc = spy(env.getQuestionDoc('q6Id'));
19151948
env.selectQuestion(6);
1949+
verify(questionDoc!.updateAnswerFileCache()).times(1);
19161950
env.simulateRemoteEditAnswer(0, 'Question 6 edited answer');
1917-
verify(questionDoc!.updateAnswerFileCache()).times(3);
1951+
verify(questionDoc!.updateAnswerFileCache()).times(2);
19181952
expect().nothing();
19191953
tick();
19201954
flush();
@@ -1924,8 +1958,9 @@ describe('CheckingComponent', () => {
19241958
const env = new TestEnvironment({ user: ADMIN_USER });
19251959
const questionDoc = spy(env.getQuestionDoc('q6Id'));
19261960
env.selectQuestion(6);
1961+
verify(questionDoc!.updateAnswerFileCache()).times(1);
19271962
env.simulateRemoteDeleteAnswer('q6Id', 0);
1928-
verify(questionDoc!.updateAnswerFileCache()).times(3);
1963+
verify(questionDoc!.updateAnswerFileCache()).times(2);
19291964
expect().nothing();
19301965
tick();
19311966
flush();

src/SIL.XForge.Scripture/ClientApp/src/app/checking/checking/checking.component.ts

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,6 @@ export class CheckingComponent extends DataLoadingComponent implements OnInit, A
580580
};
581581
} else {
582582
const suggestedBookChapter: BookChapter = await this.getSuggestedNavBookChapter(routeBookNum);
583-
584583
this.navigateBookChapter(
585584
routeProjectId,
586585
routeScope!,
@@ -603,11 +602,10 @@ export class CheckingComponent extends DataLoadingComponent implements OnInit, A
603602
if (
604603
routeProjectId !== prevProjectId ||
605604
routeScope !== prevScope ||
606-
(routeScope !== 'all' && routeBookNum !== prevBookNum) ||
607-
(routeScope === 'chapter' && (routeChapter == null ? undefined : parseInt(routeChapter)) !== prevChapterNum)
605+
routeBookNum !== prevBookNum ||
606+
(routeChapter == null ? undefined : parseInt(routeChapter)) !== prevChapterNum
608607
) {
609608
this.cleanup();
610-
611609
this.questionsQuery = await this.checkingQuestionsService.queryQuestions(
612610
routeProjectId,
613611
{
@@ -733,7 +731,6 @@ export class CheckingComponent extends DataLoadingComponent implements OnInit, A
733731
.pipe(quietTakeUntilDestroyed(this.destroyRef))
734732
.subscribe((state: BreakpointState) => {
735733
this.calculateScriptureSliderPosition();
736-
737734
// `questionsPanel` is undefined until ngAfterViewInit, but setting `isQuestionListPermanent`
738735
// here causes `ExpressionChangedAfterItHasBeenCheckedError`, so wrap in setTimeout
739736
setTimeout(() => {
@@ -746,7 +743,8 @@ export class CheckingComponent extends DataLoadingComponent implements OnInit, A
746743
.observe(this.mediaBreakpointService.width('<', Breakpoint.MD))
747744
.pipe(quietTakeUntilDestroyed(this.destroyRef))
748745
.subscribe((state: BreakpointState) => {
749-
this.isScreenSmall = state.matches;
746+
// setting isScreenSmall causes `ExpressionChangedAfterItHasBeenCheckedError`, so wrap in setTimeout
747+
setTimeout(() => (this.isScreenSmall = state.matches));
750748
});
751749
}
752750

@@ -900,11 +898,11 @@ export class CheckingComponent extends DataLoadingComponent implements OnInit, A
900898

901899
onBookSelect(book: number): void {
902900
const navChapterNum: number = this.projectDoc!.data!.texts.find(t => t.bookNum === book)?.chapters[0].number ?? 1;
903-
this.navigateBookChapter(this.projectDoc!.id, this.activeQuestionScope!, book, navChapterNum);
901+
this.navigateBookChapter(this.projectDoc!.id, 'all', book, navChapterNum);
904902
}
905903

906904
onChapterSelect(chapter: number): void {
907-
this.navigateBookChapter(this.projectDoc!.id, this.activeQuestionScope!, this.book!, chapter);
905+
this.navigateBookChapter(this.projectDoc!.id, 'all', this.book!, chapter);
908906
}
909907

910908
questionUpdated(_questionDoc: QuestionDoc): void {
@@ -940,9 +938,7 @@ export class CheckingComponent extends DataLoadingComponent implements OnInit, A
940938
}
941939

942940
// Ensure navigation is set to book/chapter of selected question
943-
if (this.navigateQuestionChapter(questionDoc)) {
944-
return;
945-
}
941+
this.navigateQuestionChapter(questionDoc);
946942
}
947943
}
948944

@@ -1098,12 +1094,11 @@ export class CheckingComponent extends DataLoadingComponent implements OnInit, A
10981094
if (withFilterReset) {
10991095
this.resetFilter();
11001096
}
1101-
1102-
this.questionsList?.activateQuestion(questionDoc);
11031097
} else if (withFilterReset) {
11041098
// Reset filter, but don't update visible questions yet if navigating
11051099
this.activeQuestionFilter = QuestionFilter.None;
11061100
}
1101+
this.questionsList?.activateQuestion(questionDoc);
11071102
}
11081103

11091104
/**

0 commit comments

Comments
 (0)