Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SF-3101 Highlight range of source segments #2957

Merged
merged 1 commit into from
Jan 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1530,6 +1530,30 @@ describe('TextComponent', () => {
tick();
}).toThrowError();
}));

it('highlights individual segments when a verse range is requested that does not directly correspond to a segment', fakeAsync(() => {
const env = new TestEnvironment();
env.fixture.detectChanges();
// Suppose we have a text with separate segments for verse 2 and verse 3.
env.id = new TextDocId('project01', 40, 1);
env.component.highlightSegment = true;
env.waitForEditor();

// Suppose in the target text, there is a single segment for the verse range of verse 2-3, and that the user clicks
// in that segment. This segment does not directly correspond to a segment in our source text.
env.component.setSegment('verse_1_2-3');
tick();
env.fixture.detectChanges();
// Because there is not a 'verse_1_2-3 segment, only verse 2 is set as the current segment.
expect(env.component.segment!.ref).toEqual('verse_1_2');

// Both the segment for verse 2 and the segment for verse 3 should be highlighted.
expect(env.isSegmentHighlighted(1, '2')).toBe(true);
expect(env.isSegmentHighlighted(1, '3')).toBe(true);
expect(env.isSegmentHighlighted(1, '1')).toBe(false);

TestEnvironment.waitForPresenceTimer();
}));
});

class MockDragEvent extends DragEvent {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1407,6 +1407,12 @@ export class TextComponent extends SubscriptionDisposable implements AfterViewIn
return false;
}

const originalSegmentRefRequest: string = segmentRef;

// One project's chapter may have verses combined (eg "1-2"), or a verse split in paragraphs, differently than the
// same chapter in another project. When the user clicks in a verse range, or in a paragraph of a verse, the segment
// reference they clicked in in one TextComponent may or may not directly correspond to a segment in another
// TextComponent's viewModel.
if (!this.viewModel.hasSegmentRange(segmentRef)) {
let resetSegment = true;

Expand Down Expand Up @@ -1454,8 +1460,23 @@ export class TextComponent extends SubscriptionDisposable implements AfterViewIn
}
this.updateSegment();
this.segmentRefChange.emit(this.segmentRef);

if (this.highlightSegment) {
this.highlight();
if (this.viewModel.hasSegmentRange(originalSegmentRefRequest)) {
this.highlight();
} else {
// If the requested segment was a verse range, highlight all segments in the range.
const verseStr: string | undefined = getVerseStrFromSegmentRef(originalSegmentRefRequest);
if (verseStr != null && this.id != null) {
const verseRef: VerseRef = new VerseRef(
Canon.bookNumberToId(this.id.bookNum),
this.id.chapterNum.toString(),
verseStr
);
const segmentsInVerseRange: string[] = this.getVerseSegments(verseRef);
this.highlight(segmentsInVerseRange);
}
}
}
return true;
}
Expand Down
Loading