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

Improve Component Communication Logic #9730

Merged
merged 1 commit into from
Jan 29, 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 @@ -38,7 +38,7 @@ export class CodePanelComponent implements OnChanges{
@Input() showLineNumbers: boolean = true;
@Input() loadFailed : boolean = false;
@Input() codeLineSearchText: string | undefined;
@Input() codeLineNavigationDirection: number | undefined;
@Input() codeLineSearchInfo: CodeLineSearchInfo | undefined = undefined;

@Output() hasActiveConversationEmitter : EventEmitter<boolean> = new EventEmitter<boolean>();
@Output() codeLineSearchInfoEmitter : EventEmitter<CodeLineSearchInfo> = new EventEmitter<CodeLineSearchInfo>();
Expand All @@ -56,8 +56,6 @@ export class CodePanelComponent implements OnChanges{

searchMatchedRowInfo: Map<string, RegExpMatchArray[]> = new Map<string, RegExpMatchArray[]>();
codeLineSearchMatchInfo : DoublyLinkedList<CodeLineSearchMatch> | undefined = undefined;
currentCodeLineSearchMatch: DoublyLinkedListNode<CodeLineSearchMatch> | undefined = undefined;
codeLineSearchInfo: CodeLineSearchInfo | undefined = undefined;

destroy$ = new Subject<void>();

Expand Down Expand Up @@ -103,11 +101,8 @@ export class CodePanelComponent implements OnChanges{
await this.searchCodePanelRowData(this.codeLineSearchText!);
}

if (changes['codeLineNavigationDirection']) {
this.navigateToCodeLineWithSearchMatch(
changes['codeLineNavigationDirection'].previousValue,
changes['codeLineNavigationDirection'].currentValue
);
if (changes['codeLineSearchInfo'] && changes['codeLineSearchInfo'].currentValue != changes['codeLineSearchInfo'].previousValue) {
this.navigateToCodeLineWithSearchMatch();
}
}

Expand Down Expand Up @@ -702,7 +697,6 @@ export class CodePanelComponent implements OnChanges{
if (!searchText || searchText.length === 0) {
this.clearSearchMatchHighlights();
this.codeLineSearchMatchInfo = undefined;
this.currentCodeLineSearchMatch = undefined;
this.codeLineSearchInfo = undefined;
this.codeLineSearchInfoEmitter.emit(this.codeLineSearchInfo);
return;
Expand All @@ -726,34 +720,24 @@ export class CodePanelComponent implements OnChanges{
}
}
});

let currentMatch = 0;
let totalMatchCount = 0;

if (hasMatch) {
this.currentCodeLineSearchMatch = this.codeLineSearchMatchInfo.head;
this.codeLineSearchInfo = new CodeLineSearchInfo(this.codeLineSearchMatchInfo.head, this.codeLineSearchMatchInfo.length);

if (this.currentCodeLineSearchMatch?.value.rowIndex! < this.codePanelRowSource?.adapter?.firstVisible.$index! ||
this.currentCodeLineSearchMatch?.value.rowIndex! > this.codePanelRowSource?.adapter?.lastVisible.$index!) {
if (this.codeLineSearchInfo.currentMatch?.value.rowIndex! < this.codePanelRowSource?.adapter?.firstVisible.$index! ||
this.codeLineSearchInfo.currentMatch?.value.rowIndex! > this.codePanelRowSource?.adapter?.lastVisible.$index!) {
// Scroll first match into view
await this.scrollToNode(this.currentCodeLineSearchMatch!.value.nodeIdHashed, undefined, false, false);
await this.scrollToNode(this.codeLineSearchInfo.currentMatch!.value.nodeIdHashed, undefined, false, false);
await this.codePanelRowSource?.adapter?.relax();
}

currentMatch = this.currentCodeLineSearchMatch!.index + 1;
totalMatchCount = this.codeLineSearchMatchInfo.length;
this.highlightSearchMatches();
this.highlightActiveSearchMatch();
} else {
this.clearSearchMatchHighlights();
this.codeLineSearchMatchInfo = undefined;
this.currentCodeLineSearchMatch = undefined;
this.codeLineSearchInfo = undefined;
}

this.codeLineSearchInfo = {
currentMatch: currentMatch,
totalMatchCount: totalMatchCount
};
this.codeLineSearchInfoEmitter.emit(this.codeLineSearchInfo);
}

Expand Down Expand Up @@ -833,9 +817,9 @@ export class CodePanelComponent implements OnChanges{
}

private highlightActiveSearchMatch(scrollIntoView: boolean = true) {
if (this.currentCodeLineSearchMatch) {
const nodeIdHashed = this.currentCodeLineSearchMatch.value.nodeIdHashed;
const matchId = this.currentCodeLineSearchMatch.value.matchId;
if (this.codeLineSearchInfo?.currentMatch) {
const nodeIdHashed = this.codeLineSearchInfo?.currentMatch.value.nodeIdHashed;
const matchId = this.codeLineSearchInfo?.currentMatch.value.matchId;

const activeMatch = this.elementRef.nativeElement.querySelector('.codeline-search-match-highlight.active');
if (activeMatch) {
Expand All @@ -860,31 +844,16 @@ export class CodePanelComponent implements OnChanges{
/**
* Navigates to the next or previous code line that contains a search match but is outside the viewport
*/
private navigateToCodeLineWithSearchMatch(previousPosition: number, newPosition: number) {
if (this.currentCodeLineSearchMatch) {
private navigateToCodeLineWithSearchMatch() {
if (this.codeLineSearchInfo?.currentMatch) {
const firstVisibleIndex = this.codePanelRowSource?.adapter?.firstVisible.$index!;
const lastVisibleIndex = this.codePanelRowSource?.adapter?.lastVisible.$index!;
let currentMatch = this.codeLineSearchInfo?.currentMatch!;

if (!previousPosition || newPosition > previousPosition) {
this.currentCodeLineSearchMatch = this.currentCodeLineSearchMatch?.next!;
currentMatch++;
} else if (newPosition < previousPosition) {
this.currentCodeLineSearchMatch = this.currentCodeLineSearchMatch?.prev!;
currentMatch--;
}

if (this.currentCodeLineSearchMatch && (this.currentCodeLineSearchMatch.value.rowIndex < firstVisibleIndex || this.currentCodeLineSearchMatch.value.rowIndex > lastVisibleIndex)) {
this.scrollToNode(this.currentCodeLineSearchMatch.value.nodeIdHashed, undefined, false, false);
if (this.codeLineSearchInfo?.currentMatch && (this.codeLineSearchInfo?.currentMatch.value.rowIndex < firstVisibleIndex || this.codeLineSearchInfo?.currentMatch.value.rowIndex > lastVisibleIndex)) {
this.scrollToNode(this.codeLineSearchInfo?.currentMatch.value.nodeIdHashed, undefined, false, false);
this.codePanelRowSource?.adapter?.relax();
}

this.highlightActiveSearchMatch();
this.codeLineSearchInfo = {
currentMatch: currentMatch,
totalMatchCount: this.codeLineSearchInfo?.totalMatchCount
};
this.codeLineSearchInfoEmitter.emit(this.codeLineSearchInfo);
}
}

Expand Down Expand Up @@ -963,7 +932,7 @@ export class CodePanelComponent implements OnChanges{
const viewport = this.elementRef.nativeElement.ownerDocument.getElementById('viewport');
if (viewport) {
viewport.addEventListener('scroll', (event) => {
if (this.currentCodeLineSearchMatch) {
if (this.codeLineSearchInfo?.currentMatch) {
this.highlightSearchMatches();
this.highlightActiveSearchMatch(false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
<input type="text" [formControl]="codeLineSearchText" pInputText placeholder="Search Review" style="width: 100%; box-sizing: border-box;"/>
</p-iconField>
<div *ngIf="codeLineSearchInfo" class="d-flex justify-content-between">
<small class="review-search-info">{{ codeLineSearchInfo.currentMatch }} of {{ codeLineSearchInfo.totalMatchCount }}</small>
<small class="review-search-info">{{ (codeLineSearchInfo.currentMatch?.index ?? 0) + 1 }} of {{ codeLineSearchInfo.totalMatchCount }}</small>
<span class="mt-2">
<i class="bi bi-arrow-up-short border rounded-start review-search-icon" (click)="navigateSearch(-1)"></i>
<i class="bi bi-arrow-down-short border review-search-icon" (click)="navigateSearch(1)"></i>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export class ReviewPageOptionsComponent implements OnInit, OnChanges {
@Output() diffNavaigationEmitter : EventEmitter<CodeLineRowNavigationDirection> = new EventEmitter<CodeLineRowNavigationDirection>();
@Output() copyReviewTextEmitter : EventEmitter<boolean> = new EventEmitter<boolean>();
@Output() codeLineSearchTextEmitter : EventEmitter<string> = new EventEmitter<string>();
@Output() codeLineSearchNaviationEmmiter : EventEmitter<number> = new EventEmitter<number>();
@Output() codeLineSearchInfoEmitter : EventEmitter<CodeLineSearchInfo> = new EventEmitter<CodeLineSearchInfo>();

private destroy$ = new Subject<void>();

Expand Down Expand Up @@ -374,9 +374,17 @@ export class ReviewPageOptionsComponent implements OnInit, OnChanges {
* @param number
*/
navigateSearch(number: 1 | -1) {
const navigationPosition = this.codeLineSearchInfo?.currentMatch! + number;
if (navigationPosition >= 1 && navigationPosition <= this.codeLineSearchInfo?.totalMatchCount!) {
this.codeLineSearchNaviationEmmiter.emit(navigationPosition!);
if (number == 1) {
if (!this.codeLineSearchInfo?.currentMatch?.isTail()) {
this.codeLineSearchInfo!.currentMatch = this.codeLineSearchInfo?.currentMatch?.next;
this.codeLineSearchInfoEmitter.emit(this.codeLineSearchInfo!);
}
}
else {
if (!this.codeLineSearchInfo?.currentMatch?.isHead()) {
this.codeLineSearchInfo!.currentMatch = this.codeLineSearchInfo?.currentMatch?.prev;
this.codeLineSearchInfoEmitter.emit(this.codeLineSearchInfo!);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
[showLineNumbers]="showLineNumbers" [scrollToNodeIdHashed]="scrollToNodeIdHashed"
[scrollToNodeId]="scrollToNodeId"
[codeLineSearchText]="codeLineSearchText"
[codeLineNavigationDirection]="codeLineNavigationDirection"
[codeLineSearchInfo]="codeLineSearchInfo"
(hasActiveConversationEmitter)="handleHasActiveConversationEmitter($event)"
(codeLineSearchInfoEmitter)="handleCodeLineSearchInfoEmitter($event)"></app-code-panel>
</div>
Expand Down Expand Up @@ -66,7 +66,7 @@
(diffNavaigationEmitter)="handleDiffNavaigationEmitter($event)"
(copyReviewTextEmitter)="handleCopyReviewTextEmitter($event)"
(codeLineSearchTextEmitter)="handleCodeLineSearchTextEmitter($event)"
(codeLineSearchNaviationEmmiter)="handleCodeLineSearchNavigationEmitter($event)"></app-review-page-options>
(codeLineSearchInfoEmitter)="handleCodeLineSearchInfoEmitter($event)"></app-review-page-options>
</div>
</ng-template>
</p-splitter>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export class ReviewPageComponent implements OnInit {
preferredApprovers : string[] = [];
hasFatalDiagnostics : boolean = false;
hasActiveConversation : boolean = false;
codeLineSearchInfo : CodeLineSearchInfo | undefined = new CodeLineSearchInfo();
codeLineSearchInfo : CodeLineSearchInfo | undefined;
numberOfActiveConversation : number = 0;
hasHiddenAPIs : boolean = false;
hasHiddenAPIThatIsDiff : boolean = false;
Expand All @@ -72,7 +72,6 @@ export class ReviewPageComponent implements OnInit {
lastNodeIdUnhashedDiscarded = '';

codeLineSearchText: string | undefined = undefined;
codeLineNavigationDirection: number | undefined = undefined;

private destroy$ = new Subject<void>();
private destroyLoadAPIRevision$ : Subject<void> | null = null;
Expand Down Expand Up @@ -491,16 +490,14 @@ export class ReviewPageComponent implements OnInit {
this.codeLineSearchText = searchText;
}

handleCodeLineSearchNavigationEmitter(direction: number) {
this.codeLineNavigationDirection = direction;
}

handleHasActiveConversationEmitter(value: boolean) {
this.hasActiveConversation = value;
}

handleCodeLineSearchInfoEmitter(value: CodeLineSearchInfo) {
this.codeLineSearchInfo = value;
setTimeout(() => {
this.codeLineSearchInfo = (value) ? new CodeLineSearchInfo(value.currentMatch, value.totalMatchCount) : undefined;
}, 0);
}

handleNumberOfActiveThreadsEmitter(value: number) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,12 @@ export class DoublyLinkedListNode<T> {
this.value = value;
this.index = index;
}

isHead(): boolean {
return this.prev === undefined;
}

isTail(): boolean {
return this.next === undefined;
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
import { DoublyLinkedListNode } from "../_helpers/doubly-linkedlist";

export class CodeLineSearchInfo {
currentMatch?: number;
totalMatchCount?: number
currentMatch?: DoublyLinkedListNode<CodeLineSearchMatch> | undefined;
totalMatchCount?: number;

constructor(currentMatch: DoublyLinkedListNode<CodeLineSearchMatch> | undefined, totalMatchCount: number | undefined) {
this.currentMatch = currentMatch;
this.totalMatchCount = totalMatchCount;
}
}

export class CodeLineSearchMatch {
Expand Down