Comment and Diff Navigation

Azure · Sep 3, 2024 · af3a56b · af3a56b
1 parent fbb7bf8
commit af3a56b
Show file tree

Hide file tree

Showing 16 changed files with 279 additions and 34 deletions.
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
@@ -4,12 +4,13 @@ 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, DIFF_ADDED, DIFF_REMOVED, 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';
@@ -35,6 +36,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;
@@ -45,8 +47,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}`;
@@ -554,6 +559,107 @@ export class CodePanelComponent implements OnChanges{
     });
   }
 
+  navigateToCommentThread(direction: CodeLineRowNavigationDirection) {
+    const findNextCommentThread = (index: number) : CodePanelRowData | undefined => {
+      while (index < this.codePanelRowData.length) {
+        if (this.codePanelRowData[index].type === CodePanelRowDatatype.CommentThread) {
+          this.commentThreadNavaigationPointer = index;
+          return this.codePanelRowData[index];
+        }
+        index++;
+      }
+      return undefined;
+    }
+
+    const findPrevCommentthread = (index: number) : CodePanelRowData | undefined => {
+      while (index < this.codePanelRowData.length && index >= 0) {
+        if (this.codePanelRowData[index].type === CodePanelRowDatatype.CommentThread) {
+          this.commentThreadNavaigationPointer = index;
+          return this.codePanelRowData[index];
+        }
+        index--;
+      }
+      return undefined;
+    }
+    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 = findNextCommentThread(startIndex);
+    }
+    else {
+      const startIndex = (this.commentThreadNavaigationPointer && this.commentThreadNavaigationPointer >= firstVisible && this.commentThreadNavaigationPointer <= lastVisible) ? 
+        this.commentThreadNavaigationPointer - 1 : lastVisible;
+      navigateToRow = findPrevCommentthread(startIndex);
+    }
+
+    if (navigateToRow) {
+      this.scrollToNode(navigateToRow.nodeIdHashed);
+    }
+    else {
+      this.messageService.add({ severity: 'info', icon: 'bi bi-info-circle', detail: 'No more comments to navigate to.', key: 'bl', life: 3000 });
+    }
+  }
+
+  navigateToDiffNode(direction: CodeLineRowNavigationDirection) {
+    const findNextDiffNode = (index: number) : CodePanelRowData | undefined => {
+      let checkForDiffNode = (this.isDiffRow(this.codePanelRowData[index])) ? false : true;
+      while (index < this.codePanelRowData.length) {
+        if (!checkForDiffNode && !this.isDiffRow(this.codePanelRowData[index])) {
+          checkForDiffNode = true;
+        }
+        if (checkForDiffNode && this.isDiffRow(this.codePanelRowData[index])) {
+          this.diffNodeNavaigationPointer = index;
+          return this.codePanelRowData[index];
+        }
+        index++;
+      }
+      return undefined;
+    }
+
+    const findPrevDiffNode = (index: number) : CodePanelRowData | undefined => {
+      let checkForDiffNode = (this.isDiffRow(this.codePanelRowData[index])) ? false : true;
+      while (index < this.codePanelRowData.length && index >= 0) {
+        if (!checkForDiffNode && !this.isDiffRow(this.codePanelRowData[index])) {
+          checkForDiffNode = true;
+        }
+        if (checkForDiffNode && this.isDiffRow(this.codePanelRowData[index])) {
+          this.diffNodeNavaigationPointer = index;
+          return this.codePanelRowData[index];
+        }
+        index--;
+      }
+      return undefined;
+    }
+
+    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 = findNextDiffNode(startIndex);
+    }
+    else {
+      const startIndex = (this.diffNodeNavaigationPointer && this.diffNodeNavaigationPointer >= firstVisible && this.diffNodeNavaigationPointer <= lastVisible) ? 
+        this.diffNodeNavaigationPointer: lastVisible;
+      navigateToRow = 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 isDiffRow(row: CodePanelRowData) {
+    return row.type === CodePanelRowDatatype.CodeLine && (row.diffKind === DIFF_REMOVED || row.diffKind === DIFF_ADDED);
+  }
+
   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
@@ -69,29 +69,21 @@
 
 <app-page-options-section sectionName="Find on Page">
     <ul class="list-group">
-        <li class="list-group-item">                
-            <label class="small mx-1 fw-semibold">Search API:</label>
-            <div class="input-group input-group-sm">
-                <input type="text" class="form-control" placeholder="Search.." aria-label="Search Codelines">
-                <button class="btn btn-outline-secondary" type="button"><i class="bi bi-arrow-down"></i></button>
-                <button class="btn btn-outline-secondary" type="button"><i class="bi bi-arrow-up"></i></button>
-            </div>
-        </li>
         <li class="list-group-item">                
             <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"><i class="bi bi-arrow-down"></i>Next</button>
-                    <button type="button" class="btn btn-outline-secondary"><i class="bi bi-arrow-up"></i>Prev</button>
+                    <button type="button" class="btn btn-outline-secondary" (click)="commentThreadNavaigationEmitter.emit(CodeLineRowNavigationDirection.next)"><i class="bi bi-arrow-down"></i>Next</button>
+                    <button type="button" class="btn btn-outline-secondary" (click)="commentThreadNavaigationEmitter.emit(CodeLineRowNavigationDirection.prev)"><i class="bi bi-arrow-up"></i>Prev</button>
                 </div>
             </div>
         </li>
-        <li class="list-group-item">
+        <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"><i class="bi bi-arrow-down"></i>Next</button>
-                    <button type="button" class="btn btn-outline-secondary"><i class="bi bi-arrow-up"></i>Prev</button>
+                    <button type="button" class="btn btn-outline-secondary" (click)="diffNavaigationEmitter.emit(CodeLineRowNavigationDirection.next)"><i class="bi bi-arrow-down"></i>Next</button>
+                    <button type="button" class="btn btn-outline-secondary" (click)="diffNavaigationEmitter.emit(CodeLineRowNavigationDirection.prev)"><i class="bi bi-arrow-up"></i>Prev</button>
                 </div>
             </div>
         </li>

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';
@@ -41,6 +41,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
 
@@ -69,6 +72,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
@@ -68,7 +68,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';
@@ -431,6 +431,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 });
         }
       });
     }
@@ -506,7 +507,7 @@ export class RevisionsListComponent implements OnInit, OnChanges {
           `Run <code>dotnet pack</code>`, 
           `Upload the resulting .nupkg file.`
         ];
-        this.acceptedFilesForReviewUpload = ".nupkg";
+        this.acceptedFilesForReviewUpload = ".nupkg, .dll";
         this.createRevisionForm.get('selectedFile')?.enable();
         this.createRevisionForm.get('filePath')?.disable();
         break;

diff --git a/...APIView/ClientSPA/src/app/_components/shared/comment-thread/comment-thread.component.scss b/...APIView/ClientSPA/src/app/_components/shared/comment-thread/comment-thread.component.scss
@@ -24,6 +24,7 @@
 
     .p-panel {
         border: none;
+        background-color: unset;
 
         p {
             font-size: 14px;

diff --git a/src/dotnet/APIView/ClientSPA/src/app/_helpers/common-helpers.ts b/src/dotnet/APIView/ClientSPA/src/app/_helpers/common-helpers.ts
@@ -9,6 +9,13 @@ export const NODE_DIFF_STYLE = "nodes";
 export const MANUAL_ICON = "fa-solid fa-arrow-up-from-bracket";
 export const PR_ICON = "fa-solid fa-code-pull-request";
 export const AUTOMATIC_ICON = "fa-solid fa-robot";
+export const DIFF_ADDED = "added";
+export const DIFF_REMOVED = "removed";
+
+export enum CodeLineRowNavigationDirection {
+  prev = 0,
+  next
+}
 
 export function getLanguageCssSafeName(language: string): string {
   switch (language.toLowerCase()) {

diff --git a/src/dotnet/APIView/ClientSPA/src/app/_modules/shared/shared-app.module.ts b/src/dotnet/APIView/ClientSPA/src/app/_modules/shared/shared-app.module.ts
@@ -21,6 +21,7 @@ import { FileUploadModule } from 'primeng/fileupload';
 import { InputTextModule } from 'primeng/inputtext';
 import { MessagesModule } from 'primeng/messages';
 import { BadgeModule } from 'primeng/badge';
+import { ToastModule } from 'primeng/toast';
 
 
 @NgModule({

diff --git a/src/dotnet/APIView/ClientSPA/src/app/_workers/apitree-builder.worker.ts b/src/dotnet/APIView/ClientSPA/src/app/_workers/apitree-builder.worker.ts
@@ -4,7 +4,7 @@ import { ApiTreeBuilderData } from "../_models/revision";
 import { CodePanelData, CodePanelNodeMetaData, CodePanelRowData, CodePanelRowDatatype } from '../_models/codePanelModels';
 import { InsertCodePanelRowDataMessage, ReviewPageWorkerMessageDirective } from '../_models/insertCodePanelRowDataMessage';
 import { NavigationTreeNode } from '../_models/navigationTreeModels';
-import { FULL_DIFF_STYLE, NODE_DIFF_STYLE, TREE_DIFF_STYLE } from '../_helpers/common-helpers';
+import { DIFF_ADDED, DIFF_REMOVED, FULL_DIFF_STYLE, NODE_DIFF_STYLE, TREE_DIFF_STYLE } from '../_helpers/common-helpers';
 
 let codePanelData: CodePanelData | null = null;
 let codePanelRowData: CodePanelRowData[] = [];
@@ -224,9 +224,9 @@ function appendToggleDocumentationClass(node: CodePanelNodeMetaData, codePanelRo
 }
 
 function setLineNumber(row: CodePanelRowData) {
-  if (row.diffKind === "removed") {
+  if (row.diffKind === DIFF_REMOVED) {
     row.lineNumber = ++lineNumber;
-  } else if (row.diffKind === "added") {
+  } else if (row.diffKind === DIFF_ADDED) {
     lineNumber++;
     diffLineNumber++;
     row.lineNumber = diffLineNumber;

diff --git a/src/dotnet/APIView/ClientSPA/src/app/app.component.html b/src/dotnet/APIView/ClientSPA/src/app/app.component.html
@@ -1,4 +1,5 @@
 <div class="virtual-body">
     <router-outlet></router-outlet>
+    <p-toast position="bottom-left" key="bl" />
 </div>
 
diff --git a/src/dotnet/APIView/ClientSPA/src/app/app.component.spec.ts b/src/dotnet/APIView/ClientSPA/src/app/app.component.spec.ts
@@ -2,10 +2,19 @@ import { TestBed } from '@angular/core/testing';
 import { RouterTestingModule } from '@angular/router/testing';
 import { AppComponent } from './app.component';
 import { HttpClientTestingModule } from '@angular/common/http/testing';
+import { ToastModule } from 'primeng/toast';
+import { MessageService } from 'primeng/api';
 
 describe('AppComponent', () => {
   beforeEach(() => TestBed.configureTestingModule({
-    imports: [RouterTestingModule, HttpClientTestingModule],
+    imports: [
+      RouterTestingModule,
+      HttpClientTestingModule,
+      ToastModule
+    ],
+    providers: [
+      MessageService
+    ],
     declarations: [AppComponent],
   }));