080624.01/comment and diff navigation (#8898)

* Add Previous and Next Comment and Diff Buttons * Comment and Diff Navigation * Add Navigation button to comment threads * Ensure navigation starts from current comment * Increase component budget
Azure · Sep 12, 2024 · f786382 · f786382
1 parent 3feb333
commit f786382
Show file tree

Hide file tree

Showing 21 changed files with 364 additions and 29 deletions.
diff --git a/src/dotnet/APIView/ClientSPA/angular.json b/src/dotnet/APIView/ClientSPA/angular.json
@@ -59,7 +59,7 @@
                 {
                   "type": "anyComponentStyle",
                   "maximumWarning": "2kb",
-                  "maximumError": "5kb"
+                  "maximumError": "10kb"
                 }
               ],
               "outputHashing": "all"

diff --git a/src/dotnet/APIView/ClientSPA/src/app/_components/code-panel/code-panel.component.html b/src/dotnet/APIView/ClientSPA/src/app/_components/code-panel/code-panel.component.html
@@ -30,7 +30,9 @@
         (saveCommentActionEmitter)="handleSaveCommentActionEmitter($event)"
         (deleteCommentActionEmitter)="handleDeleteCommentActionEmitter($event)"
         (commentResolutionActionEmitter)="handleCommentResolutionActionEmitter($event)"
-        (commentUpvoteActionEmitter)="handleCommentUpvoteActionEmitter($event)"></app-comment-thread>
+        (commentUpvoteActionEmitter)="handleCommentUpvoteActionEmitter($event)"
+        (commentThreadNavaigationEmitter)="handleCommentThreadNavaigationEmitter($event)">
+      </app-comment-thread>
     </ng-container>
   </div>
 </div>

diff --git a/src/dotnet/APIView/ClientSPA/src/app/_components/code-panel/code-panel.component.scss b/src/dotnet/APIView/ClientSPA/src/app/_components/code-panel/code-panel.component.scss
@@ -60,7 +60,7 @@
         }
 
         &.user-comment-thread {
-            padding-left: calc(var(--max-line-number-width) + 50px);
+            padding-left: calc(var(--max-line-number-width) + 20px);
             border-top: 1px solid var(--border-color);
             border-bottom: 1px solid var(--border-color);
 
@@ -71,6 +71,10 @@
             }
         }
 
+        &.user-comment-thread:hover .comment-thread-navigation {
+            display: block;
+        }
+
         &.added {
             background-color: rgba(0, 255, 0, 0.25);
             .code-line-content::before {

diff --git a/src/dotnet/APIView/ClientSPA/src/app/_components/code-panel/code-panel.component.spec.ts b/src/dotnet/APIView/ClientSPA/src/app/_components/code-panel/code-panel.component.spec.ts
@@ -6,6 +6,7 @@ import { HttpClientTestingModule } from '@angular/common/http/testing';
 import { ActivatedRoute, convertToParamMap } from '@angular/router';
 import { SharedAppModule } from 'src/app/_modules/shared/shared-app.module';
 import { ReviewPageModule } from 'src/app/_modules/review-page/review-page.module';
+import { MessageService } from 'primeng/api';
 
 describe('CodePanelComponent', () => {
   let component: CodePanelComponent;
@@ -24,7 +25,8 @@ describe('CodePanelComponent', () => {
               queryParamMap: convertToParamMap({ activeApiRevisionId: 'test', diffApiRevisionId: 'test' })
             }
           }
-        }
+        },
+        MessageService
       ],
       imports: [HttpClientTestingModule,
         SharedAppModule,

diff --git a/src/dotnet/APIView/ClientSPA/src/app/_components/code-panel/code-panel.component.ts b/src/dotnet/APIView/ClientSPA/src/app/_components/code-panel/code-panel.component.ts
@@ -1,15 +1,16 @@
-import { ChangeDetectorRef, Component, EventEmitter, Input, OnChanges, OnInit, Output, SimpleChanges } from '@angular/core';
+import { ChangeDetectorRef, Component, EventEmitter, Input, OnChanges, Output, SimpleChanges } from '@angular/core';
 import { take, takeUntil } from 'rxjs/operators';
 import { Datasource, IDatasource, SizeStrategy } from 'ngx-ui-scroll';
 import { CommentsService } from 'src/app/_services/comments/comments.service';
 import { getQueryParams } from 'src/app/_helpers/router-helpers';
 import { ActivatedRoute, Router } from '@angular/router';
-import { SCROLL_TO_NODE_QUERY_PARAM } from 'src/app/_helpers/common-helpers';
+import { CodeLineRowNavigationDirection, isDiffRow, SCROLL_TO_NODE_QUERY_PARAM } from 'src/app/_helpers/common-helpers';
 import { CodePanelData, CodePanelRowData, CodePanelRowDatatype } from 'src/app/_models/codePanelModels';
 import { StructuredToken } from 'src/app/_models/structuredToken';
 import { CommentItemModel, CommentType } from 'src/app/_models/commentItemModel';
 import { UserProfile } from 'src/app/_models/userProfile';
 import { Message } from 'primeng/api/message';
+import { MessageService } from 'primeng/api';
 import { SignalRService } from 'src/app/_services/signal-r/signal-r.service';
 import { Subject } from 'rxjs';
 import { CommentThreadUpdateAction, CommentUpdatesDto } from 'src/app/_dtos/commentThreadUpdateDto';
@@ -36,6 +37,7 @@ export class CodePanelComponent implements OnChanges{
 
   @Output() hasActiveConversationEmitter : EventEmitter<boolean> = new EventEmitter<boolean>();
 
+
   noDiffInContentMessage : Message[] = [{ severity: 'info', icon:'bi bi-info-circle', detail: 'There is no difference between the two API revisions.' }];
   isLoading: boolean = true;
   codeWindowHeight: string | undefined = undefined;
@@ -46,8 +48,11 @@ export class CodePanelComponent implements OnChanges{
 
   destroy$ = new Subject<void>();
 
+  commentThreadNavaigationPointer: number | undefined = undefined;
+  diffNodeNavaigationPointer: number | undefined = undefined;
+
   constructor(private changeDetectorRef: ChangeDetectorRef, private commentsService: CommentsService, 
-    private signalRService: SignalRService, private route: ActivatedRoute, private router: Router) { }
+    private signalRService: SignalRService, private route: ActivatedRoute, private router: Router, private messageService: MessageService) { }
 
   ngOnInit() {
     this.codeWindowHeight = `${window.innerHeight - 80}`;
@@ -559,6 +564,109 @@ export class CodePanelComponent implements OnChanges{
     });
   }
 
+  handleCommentThreadNavaigationEmitter(event: any) {
+    this.commentThreadNavaigationPointer = Number(event.commentThreadNavaigationPointer);
+    this.navigateToCommentThread(event.direction);
+  }
+
+  navigateToCommentThread(direction: CodeLineRowNavigationDirection) {
+    const firstVisible = this.codePanelRowSource?.adapter?.firstVisible!.$index!;
+    const lastVisible = this.codePanelRowSource?.adapter?.lastVisible!.$index!;
+    let navigateToRow : CodePanelRowData | undefined = undefined;
+    if (direction == CodeLineRowNavigationDirection.next) {
+      const startIndex = (this.commentThreadNavaigationPointer && this.commentThreadNavaigationPointer >= firstVisible && this.commentThreadNavaigationPointer <= lastVisible) ? 
+        this.commentThreadNavaigationPointer + 1 : firstVisible;
+      navigateToRow = this.findNextCommentThread(startIndex);
+    }
+    else {
+      const startIndex = (this.commentThreadNavaigationPointer && this.commentThreadNavaigationPointer >= firstVisible && this.commentThreadNavaigationPointer <= lastVisible) ? 
+        this.commentThreadNavaigationPointer - 1 : lastVisible;
+      navigateToRow = this.findPrevCommentthread(startIndex);
+    }
+
+    if (navigateToRow) {
+      this.scrollToNode(navigateToRow.nodeIdHashed);
+    }
+    else {
+      this.messageService.add({ severity: 'info', icon: 'bi bi-info-circle', detail: 'No more active comments threads to navigate to.', key: 'bl', life: 3000 });
+    }
+  }
+
+  navigateToDiffNode(direction: CodeLineRowNavigationDirection) {
+    const firstVisible = this.codePanelRowSource?.adapter?.firstVisible!.$index!;
+    const lastVisible = this.codePanelRowSource?.adapter?.lastVisible!.$index!;
+    let navigateToRow : CodePanelRowData | undefined = undefined;
+    if (direction == CodeLineRowNavigationDirection.next) {
+      const startIndex = (this.diffNodeNavaigationPointer && this.diffNodeNavaigationPointer >= firstVisible && this.diffNodeNavaigationPointer <= lastVisible) ? 
+        this.diffNodeNavaigationPointer : firstVisible;
+      navigateToRow = this.findNextDiffNode(startIndex);
+    }
+    else {
+      const startIndex = (this.diffNodeNavaigationPointer && this.diffNodeNavaigationPointer >= firstVisible && this.diffNodeNavaigationPointer <= lastVisible) ? 
+        this.diffNodeNavaigationPointer: lastVisible;
+      navigateToRow = this.findPrevDiffNode(startIndex);
+    }
+
+    if (navigateToRow) {
+      this.scrollToNode(navigateToRow.nodeIdHashed);
+    }
+    else {
+      this.messageService.add({ severity: 'info', icon: 'bi bi-info-circle', detail: 'No more diffs to navigate to.', key: 'bl', life: 3000 });
+    }
+  }
+
+  private findNextCommentThread (index: number) : CodePanelRowData | undefined {
+    while (index < this.codePanelRowData.length) {
+      if (this.codePanelRowData[index].type === CodePanelRowDatatype.CommentThread && !this.codePanelRowData![index].isResolvedCommentThread) {
+        this.commentThreadNavaigationPointer = index;
+        return this.codePanelRowData[index];
+      }
+      index++;
+    }
+    return undefined;
+  }
+
+  private findPrevCommentthread (index: number) : CodePanelRowData | undefined {
+    while (index < this.codePanelRowData.length && index >= 0) {
+      if (this.codePanelRowData[index].type === CodePanelRowDatatype.CommentThread && !this.codePanelRowData![index].isResolvedCommentThread) {
+        this.commentThreadNavaigationPointer = index;
+        return this.codePanelRowData[index];
+      }
+      index--;
+    }
+    return undefined;
+  }
+
+  private findNextDiffNode (index: number) : CodePanelRowData | undefined {
+    let checkForDiffNode = (isDiffRow(this.codePanelRowData[index])) ? false : true;
+    while (index < this.codePanelRowData.length) {
+      if (!checkForDiffNode && !isDiffRow(this.codePanelRowData[index])) {
+        checkForDiffNode = true;
+      }
+      if (checkForDiffNode && isDiffRow(this.codePanelRowData[index])) {
+        this.diffNodeNavaigationPointer = index;
+        return this.codePanelRowData[index];
+      }
+      index++;
+    }
+    return undefined;
+  }
+
+  private findPrevDiffNode (index: number) : CodePanelRowData | undefined {
+    let checkForDiffNode = (isDiffRow(this.codePanelRowData[index])) ? false : true;
+    while (index < this.codePanelRowData.length && index >= 0) {
+      if (!checkForDiffNode && !isDiffRow(this.codePanelRowData[index])) {
+        checkForDiffNode = true;
+      }
+      if (checkForDiffNode && isDiffRow(this.codePanelRowData[index])) {
+        this.diffNodeNavaigationPointer = index;
+        return this.codePanelRowData[index];
+      }
+      index--;
+    }
+    return undefined;
+  }
+
   private updateHasActiveConversations() {
     let hasActiveConversation = false;
     for (let row of this.codePanelRowData) {

diff --git a/...View/ClientSPA/src/app/_components/review-page-options/review-page-options.component.html b/...View/ClientSPA/src/app/_components/review-page-options/review-page-options.component.html
@@ -67,6 +67,33 @@
     </ul>
 </app-page-options-section>
 
+<app-page-options-section *ngIf="showCommentsSwitch || (isDiffView && contentHasDiff)" sectionName="Find on Page">
+    <ul class="list-group">
+        <li class="list-group-item" *ngIf="showCommentsSwitch">                
+            <label class="small mx-1 fw-semibold">Comment:</label>
+            <div class="d-grid gap-2">
+                <div class="btn-group btn-group-sm" role="group" aria-label="Comment Navigation">
+                    <button type="button" class="btn btn-outline-secondary" pTooltip="Previous Comment" tooltipPosition="top" (click)="commentThreadNavaigationEmitter.emit(CodeLineRowNavigationDirection.prev)">
+                        <i class="bi bi-arrow-up"></i>Prev</button>
+                    <button type="button" class="btn btn-outline-secondary" pTooltip="Next Comment" tooltipPosition="top" (click)="commentThreadNavaigationEmitter.emit(CodeLineRowNavigationDirection.next)">
+                        <i class="bi bi-arrow-down"></i>Next</button>
+                </div>
+            </div>
+        </li>
+        <li class="list-group-item" *ngIf="isDiffView && contentHasDiff">
+            <label class="small mx-1 fw-semibold">Diff:</label>
+            <div class="d-grid gap-2">
+                <div class="btn-group btn-group-sm" role="group" aria-label="Diff Navigation">
+                    <button type="button" class="btn btn-outline-secondary" pTooltip="Previous Diff" tooltipPosition="top" (click)="diffNavaigationEmitter.emit(CodeLineRowNavigationDirection.prev)">
+                        <i class="bi bi-arrow-up"></i>Prev</button>
+                    <button type="button" class="btn btn-outline-secondary" pTooltip="Next Diff" tooltipPosition="top" (click)="diffNavaigationEmitter.emit(CodeLineRowNavigationDirection.next)">
+                        <i class="bi bi-arrow-down"></i>Next</button>
+                </div>
+            </div>
+        </li>
+    </ul>
+</app-page-options-section>
+
 <app-page-options-section sectionName="Page Settings">
     <ul class="list-group">
         <li class="list-group-item">

diff --git a/...PIView/ClientSPA/src/app/_components/review-page-options/review-page-options.component.ts b/...PIView/ClientSPA/src/app/_components/review-page-options/review-page-options.component.ts
@@ -2,7 +2,7 @@ import { Component, EventEmitter, Input, OnChanges, OnInit, Output, SimpleChange
 import { ActivatedRoute, Router } from '@angular/router';
 import { InputSwitchOnChangeEvent } from 'primeng/inputswitch';
 import { getQueryParams } from 'src/app/_helpers/router-helpers';
-import { FULL_DIFF_STYLE, mapLanguageAliases, NODE_DIFF_STYLE, TREE_DIFF_STYLE } from 'src/app/_helpers/common-helpers';
+import { CodeLineRowNavigationDirection, FULL_DIFF_STYLE, mapLanguageAliases, NODE_DIFF_STYLE, TREE_DIFF_STYLE } from 'src/app/_helpers/common-helpers';
 import { Review } from 'src/app/_models/review';
 import { APIRevision } from 'src/app/_models/revision';
 import { ConfigService } from 'src/app/_services/config/config.service';
@@ -42,6 +42,9 @@ export class ReviewPageOptionsComponent implements OnInit, OnChanges{
   @Output() showLineNumbersEmitter : EventEmitter<boolean> = new EventEmitter<boolean>();
   @Output() apiRevisionApprovalEmitter : EventEmitter<boolean> = new EventEmitter<boolean>();
   @Output() reviewApprovalEmitter : EventEmitter<boolean> = new EventEmitter<boolean>();
+  @Output() commentThreadNavaigationEmitter : EventEmitter<CodeLineRowNavigationDirection> = new EventEmitter<CodeLineRowNavigationDirection>();
+  @Output() diffNavaigationEmitter : EventEmitter<CodeLineRowNavigationDirection> = new EventEmitter<CodeLineRowNavigationDirection>();
+
 
   webAppUrl : string = this.configService.webAppUrl
 
@@ -71,6 +74,7 @@ export class ReviewPageOptionsComponent implements OnInit, OnChanges{
 
   associatedPullRequests  : PullRequestModel[] = [];
   pullRequestsOfAssociatedAPIRevisions : PullRequestModel[] = [];
+  CodeLineRowNavigationDirection = CodeLineRowNavigationDirection;
 
   //Approvers Options
   selectedApprovers: string[] = [];

diff --git a/src/dotnet/APIView/ClientSPA/src/app/_components/review-page/review-page.component.html b/src/dotnet/APIView/ClientSPA/src/app/_components/review-page/review-page.component.html
@@ -69,7 +69,9 @@
                             (apiRevisionApprovalEmitter)="handleApiRevisionApprovalEmitter($event)"
                             (reviewApprovalEmitter)="handleReviewApprovalEmitter($event)"
                             (showHiddenAPIEmitter)="handleShowHiddenAPIEmitter($event)"
-                            (disableCodeLinesLazyLoadingEmitter)="handleDisableCodeLinesLazyLoadingEmitter($event)"></app-review-page-options>
+                            (disableCodeLinesLazyLoadingEmitter)="handleDisableCodeLinesLazyLoadingEmitter($event)"
+                            (commentThreadNavaigationEmitter)="handleCommentThreadNavaigationEmitter($event)"
+                            (diffNavaigationEmitter)="handleDiffNavaigationEmitter($event)"></app-review-page-options>
                     </div>
                 </ng-template>
             </p-splitter>

diff --git a/src/dotnet/APIView/ClientSPA/src/app/_components/review-page/review-page.component.spec.ts b/src/dotnet/APIView/ClientSPA/src/app/_components/review-page/review-page.component.spec.ts
@@ -17,6 +17,7 @@ import { ReviewPageOptionsComponent } from '../review-page-options/review-page-o
 import { PageOptionsSectionComponent } from '../shared/page-options-section/page-options-section.component';
 import { SharedAppModule } from 'src/app/_modules/shared/shared-app.module';
 import { ReviewPageModule } from 'src/app/_modules/review-page/review-page.module';
+import { MessageService } from 'primeng/api';
 
 describe('ReviewPageComponent', () => {
   let component: ReviewPageComponent;
@@ -51,8 +52,9 @@ describe('ReviewPageComponent', () => {
               paramMap: convertToParamMap({ reviewId: 'test' }),
             },
             queryParams: of(convertToParamMap({ activeApiRevisionId: 'test', diffApiRevisionId: 'test' }))
-          }
-        }
+          },
+        },
+        MessageService
       ]
     });
     fixture = TestBed.createComponent(ReviewPageComponent);

diff --git a/src/dotnet/APIView/ClientSPA/src/app/_components/review-page/review-page.component.ts b/src/dotnet/APIView/ClientSPA/src/app/_components/review-page/review-page.component.ts
@@ -2,7 +2,7 @@ import { ChangeDetectorRef, Component, OnInit, ViewChild } from '@angular/core';
 import { ActivatedRoute, Params, Router } from '@angular/router';
 import { MenuItem, TreeNode } from 'primeng/api';
 import { Subject, take, takeUntil } from 'rxjs';
-import { getLanguageCssSafeName } from 'src/app/_helpers/common-helpers';
+import { CodeLineRowNavigationDirection, getLanguageCssSafeName } from 'src/app/_helpers/common-helpers';
 import { getQueryParams } from 'src/app/_helpers/router-helpers';
 import { Review } from 'src/app/_models/review';
 import { APIRevision, ApiTreeBuilderData } from 'src/app/_models/revision';
@@ -438,6 +438,14 @@ export class ReviewPageComponent implements OnInit {
     });
   }
 
+  handleCommentThreadNavaigationEmitter(direction: CodeLineRowNavigationDirection) {
+    this.codePanelComponent.navigateToCommentThread(direction);
+  }
+
+  handleDiffNavaigationEmitter(direction: CodeLineRowNavigationDirection) {
+    this.codePanelComponent.navigateToDiffNode(direction);
+  }
+
   handleHasActiveConversationEmitter(value: boolean) {
     this.hasActiveConversation = value;
   }

diff --git a/src/dotnet/APIView/ClientSPA/src/app/_components/revisions-list/revisions-list.component.ts b/src/dotnet/APIView/ClientSPA/src/app/_components/revisions-list/revisions-list.component.ts
@@ -1,7 +1,7 @@
 import { Component, Input, OnChanges, OnInit, SimpleChanges, ViewChild } from '@angular/core';
 import { FormBuilder, FormGroup, Validators } from '@angular/forms';
 import { Router } from '@angular/router';
-import { MenuItem, SortEvent } from 'primeng/api';
+import { MenuItem, MessageService, SortEvent } from 'primeng/api';
 import { FileSelectEvent, FileUpload } from 'primeng/fileupload';
 import { Table, TableFilterEvent, TableLazyLoadEvent } from 'primeng/table';
 import { Pagination } from 'src/app/_models/pagination';
@@ -75,7 +75,7 @@ export class RevisionsListComponent implements OnInit, OnChanges {
 
   constructor(private apiRevisionsService: RevisionsService, private userProfileService: UserProfileService,
     private configService: ConfigService, private fb: FormBuilder, private reviewsService: ReviewsService,
-    private router: Router) { }
+    private router: Router, private messageService: MessageService) { }
 
   ngOnInit(): void {
     this.createRevisionFilters();
@@ -483,6 +483,7 @@ export class RevisionsListComponent implements OnInit, OnChanges {
         },
         error: (error: any) => {
           this.creatingRevision = false;
+          this.messageService.add({ severity: 'error', icon: 'bi bi-info-circle', detail: 'Failed to create new API Revision', key: 'bl', life: 3000 });
         }
       });
     }