Skip to content

Commit

Permalink
Show Hidden APIs always if one of them is a diff (#8934)
Browse files Browse the repository at this point in the history
  • Loading branch information
chidozieononiwu authored Sep 16, 2024
1 parent 8d731be commit b727d85
Show file tree
Hide file tree
Showing 9 changed files with 44 additions and 24 deletions.
27 changes: 14 additions & 13 deletions src/dotnet/APIView/APIViewWeb/Helpers/CodeFileHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ public static async Task<CodePanelData> GenerateCodePanelDataAsync(CodePanelRawD
codePanelData.AddNavigation(rootNodeId, CreateRootNode($"{codeFile.PackageName} {codeFile.PackageVersion}", rootNodeId));

//Collect documentation lines from active revision
CollectDocumentationLines(codeFile.ReviewLines, codePanelData.ActiveDocumentationMap, 1, "root");
CollectDocumentationLines(codePanelData, codeFile.ReviewLines, codePanelData.ActiveDocumentationMap, 1, "root");

//Calculate the diff if diff revision code file is present
if (codePanelRawData.diffRevisionCodeFile != null)
{
var diffLines = codePanelRawData.diffRevisionCodeFile.ReviewLines;
CollectDocumentationLines(diffLines, codePanelData.DiffDocumentationMap, 1, "root", true);
CollectDocumentationLines(codePanelData, diffLines, codePanelData.DiffDocumentationMap, 1, "root", true);
// Check if diff is required for active revision and diff revision to avoid unnecessary diff calculation
bool hasSameApis = AreCodeFilesSame(codePanelRawData.activeRevisionCodeFile, codePanelRawData.diffRevisionCodeFile);
if(!hasSameApis)
Expand Down Expand Up @@ -175,7 +175,7 @@ private static NavigationTreeNode CreateRootNode(string rootName, string nodeIdH
private static void BuildNodeTokens(CodePanelData codePanelData, CodePanelRawData codePanelRawData, ReviewLine reviewLine, string nodeIdHashed, int indent)
{
// Generate code line row
var codePanelRow = GetCodePanelRowData(reviewLine, nodeIdHashed, indent);
var codePanelRow = GetCodePanelRowData(codePanelData, reviewLine, nodeIdHashed, indent);

// Add documentation rows to code panel data
if (codePanelData.ActiveDocumentationMap.ContainsKey(nodeIdHashed))
Expand Down Expand Up @@ -216,7 +216,7 @@ private static void BuildNodeTokens(CodePanelData codePanelData, CodePanelRawDat
AddDiagnosticRow(codePanelData: codePanelData, codeFile: codePanelRawData.activeRevisionCodeFile, nodeId: reviewLine.LineId, nodeIdHashed: nodeIdHashed);
}

private static CodePanelRowData GetCodePanelRowData(ReviewLine reviewLine, string nodeIdHashed, int indent)
private static CodePanelRowData GetCodePanelRowData(CodePanelData codePanelData, ReviewLine reviewLine, string nodeIdHashed, int indent)
{
CodePanelRowData codePanelRowData = new()
{
Expand All @@ -238,14 +238,14 @@ private static CodePanelRowData GetCodePanelRowData(ReviewLine reviewLine, strin
return codePanelRowData;
}

if(reviewLine.DiffKind == DiffKind.Added)
if(reviewLine.DiffKind == DiffKind.Added || reviewLine.DiffKind == DiffKind.Removed)
{
rowClasses.Add("added");
}
else if (reviewLine.DiffKind == DiffKind.Removed)
{
rowClasses.Add("removed");
rowClasses.Add(reviewLine.DiffKind.ToString().ToLower());
if (codePanelRowData.IsHiddenAPI) {
codePanelData.HasHiddenAPIThatIsDiff = true;
}
}

// Convert ReviewToken to UI required StructuredToken
foreach (var token in reviewLine.Tokens)
{
Expand Down Expand Up @@ -286,6 +286,7 @@ private static CodePanelRowData CollectUserCommentsForRow(CodePanelRawData codeP
commentRowData.CommentsObj = commentsForRow.ToList();
codePanelRowData.ToggleCommentsClasses = codePanelRowData.ToggleCommentsClasses.Replace("can-show", "show");
commentRowData.IsResolvedCommentThread = commentsForRow.Any(c => c.IsResolved);
commentRowData.IsHiddenAPI = codePanelRowData.IsHiddenAPI;
}
}
else
Expand Down Expand Up @@ -457,7 +458,7 @@ private static void MarkTreeNodeAsModified(ReviewLine line, DiffKind diffKind)
* This method collects all documentation lines from the review line and generate a CodePanelRow object for each documentation line.
* These documentation rows will be stored in a dictionary so it can be mapped and connected tp code line when processing code lines.
* */
private static void CollectDocumentationLines(List<ReviewLine> reviewLines, Dictionary<string,List<CodePanelRowData>> documentationRowMap, int indent, string parentNodeIdHash, bool enableSkipDiff = false)
private static void CollectDocumentationLines(CodePanelData codePanelData, List<ReviewLine> reviewLines, Dictionary<string,List<CodePanelRowData>> documentationRowMap, int indent, string parentNodeIdHash, bool enableSkipDiff = false)
{
if(reviewLines?.Count == 0)
return;
Expand All @@ -471,7 +472,7 @@ private static void CollectDocumentationLines(List<ReviewLine> reviewLines, Dict
bool hasNonSkippedTokens = line.Tokens.Any(t => t.SkipDiff != true);
if(line.IsDocumentation && (!enableSkipDiff ||hasNonSkippedTokens))
{
docRows.Add(GetCodePanelRowData(line, parentNodeIdHash, indent));
docRows.Add(GetCodePanelRowData(codePanelData, line, parentNodeIdHash, indent));
continue;
}

Expand All @@ -487,7 +488,7 @@ private static void CollectDocumentationLines(List<ReviewLine> reviewLines, Dict
idx++;
// Recursively process child node lines
if (line.Children.Count > 0)
CollectDocumentationLines(line.Children, documentationRowMap, indent + 1, nodeIdHashed, enableSkipDiff);
CollectDocumentationLines(codePanelData, line.Children, documentationRowMap, indent + 1, nodeIdHashed, enableSkipDiff);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ public class CodePanelData
public Dictionary<string, CodePanelNodeMetaData> NodeMetaDataObj { get; set; } = new Dictionary<string, CodePanelNodeMetaData>();
public Dictionary<string, CodePanelNodeMetaData> NodeMetaData => NodeMetaDataObj.Count > 0 ? NodeMetaDataObj : null;
public bool HasDiff { get; set; } = false;
public bool HasHiddenAPIThatIsDiff { get; set; } = false;
[JsonIgnore]
public Dictionary<string, string> LineIdToNodeIdHashed { get; set; } = new Dictionary<string, string>();
[JsonIgnore]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
<div *ngIf="isLoading" class="spinner-border m-3" role="status">
<span class="visually-hidden">Loading...</span>
</div>
<p-messages *ngIf="codePanelData && !isLoading && isDiffView && !codePanelData?.hasDiff" [(value)]="noDiffInContentMessage" [closable]="false" />
<div *ngIf="codePanelRowSource;" class="viewport {{languageSafeName!}}" (click)="onCodePanelItemClick($event)">
<p-messages class="sticky-top" *ngIf="showNoDiffInContentMessage()" [(value)]="noDiffInContentMessage" [closable]="false" />
<div *uiScroll="let item of codePanelRowSource; let index = index" class="code-line" [attr.data-node-id]="item.nodeIdHashed"
[attr.data-row-position-in-group]="item.rowPositionInGroup" [attr.data-row-type]="item.type" [ngClass]="getRowClassObject(item)">
<ng-container *ngIf="item.type === CodePanelRowDatatype.CodeLine || item.type === CodePanelRowDatatype.Documentation">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ 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';
import { computeStyles } from '@popperjs/core';

@Component({
selector: 'app-code-panel',
Expand All @@ -39,6 +38,7 @@ export class CodePanelComponent implements OnChanges{


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;
codePanelRowDataIndicesMap = new Map<string, number>();
Expand Down Expand Up @@ -422,7 +422,10 @@ export class CodePanelComponent implements OnChanges{
index++;
}
if (scrollIndex) {
this.codePanelRowSource?.adapter?.reload(scrollIndex);
let scrollPadding = 0;
scrollPadding = (this.showNoDiffInContentMessage()) ? scrollPadding + 2 : scrollPadding;

this.codePanelRowSource?.adapter?.reload(scrollIndex - scrollPadding);
let newQueryParams = getQueryParams(this.route);
newQueryParams[SCROLL_TO_NODE_QUERY_PARAM] = this.codePanelRowData[scrollIndex].nodeId;
this.router.navigate([], { queryParams: newQueryParams, state: { skipStateUpdate: true } });
Expand Down Expand Up @@ -666,6 +669,10 @@ export class CodePanelComponent implements OnChanges{
}
return undefined;
}

showNoDiffInContentMessage() {
return this.codePanelData && !this.isLoading && this.isDiffView && !this.codePanelData?.hasDiff
}

private updateHasActiveConversations() {
let hasActiveConversation = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export class ReviewPageOptionsComponent implements OnInit, OnChanges{
@Input() hasFatalDiagnostics : boolean = false;
@Input() hasActiveConversation : boolean = false;
@Input() hasHiddenAPIs : boolean = false;
@Input() hasHiddenAPIThatIsDiff : boolean = false;

@Output() diffStyleEmitter : EventEmitter<string> = new EventEmitter<string>();
@Output() showCommentsEmitter : EventEmitter<boolean> = new EventEmitter<boolean>();
Expand Down Expand Up @@ -135,6 +136,10 @@ export class ReviewPageOptionsComponent implements OnInit, OnChanges{
this.setSubscribeSwitch();
this.setReviewApprovalStatus();
}

if (changes['hasHiddenAPIThatIsDiff']) {
this.setPageOptionValues();
}
}

/**
Expand Down Expand Up @@ -256,10 +261,10 @@ export class ReviewPageOptionsComponent implements OnInit, OnChanges{
this.showCommentsSwitch = this.userProfile?.preferences.showComments ?? this.showCommentsSwitch;
this.showSystemCommentsSwitch = this.userProfile?.preferences.showSystemComments ?? this.showSystemCommentsSwitch;
this.showDocumentationSwitch = this.userProfile?.preferences.showDocumentation ?? this.showDocumentationSwitch;
this.showHiddenAPISwitch = this.userProfile?.preferences.showHiddenApis ?? this.showHiddenAPISwitch;
this.disableCodeLinesLazyLoading = this.userProfile?.preferences.disableCodeLinesLazyLoading ?? this.disableCodeLinesLazyLoading;
this.showLineNumbersSwitch = (this.userProfile?.preferences.hideLineNumbers) ? false : this.showLineNumbersSwitch;
this.showLeftNavigationSwitch = (this.userProfile?.preferences.hideLeftNavigation) ? false : this.showLeftNavigationSwitch;
this.showHiddenAPISwitch = (this.userProfile?.preferences.showHiddenApis || this.hasHiddenAPIThatIsDiff || this.showHiddenAPISwitch) ? true : false;
}

setAPIRevisionApprovalStates() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
[hasFatalDiagnostics]="hasFatalDiagnostics"
[hasActiveConversation]="hasActiveConversation"
[hasHiddenAPIs]="hasHiddenAPIs"
[hasHiddenAPIThatIsDiff]="hasHiddenAPIThatIsDiff"
(showSystemCommentsEmitter)="handleShowSystemCommentsEmitter($event)"
(showDocumentationEmitter)="handleShowDocumentationEmitter($event)"
(showCommentsEmitter)="handleShowCommentsEmitter($event)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export class ReviewPageComponent implements OnInit {
hasActiveConversation : boolean = false;
numberOfActiveConversation : number = 0;
hasHiddenAPIs : boolean = false;
hasHiddenAPIThatIsDiff : boolean = false;
loadFailed : boolean = false;

showLeftNavigation : boolean = true;
Expand Down Expand Up @@ -165,6 +166,7 @@ export class ReviewPageComponent implements OnInit {

if (data.directive === ReviewPageWorkerMessageDirective.UpdateCodePanelData) {
this.codePanelData = data.payload as CodePanelData;
this.hasHiddenAPIThatIsDiff = this.codePanelData.hasHiddenAPIThatIsDiff;
this.workerService.terminateWorker();
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export class CodePanelRowData {
export interface CodePanelData {
nodeMetaData: { [key: string]: CodePanelNodeMetaData };
hasDiff: boolean;
hasHiddenAPIThatIsDiff: boolean;
}

export class CodePanelNodeMetaData {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ addEventListener('message', ({ data }) => {

const hasHiddenAPIMessage : InsertCodePanelRowDataMessage = {
directive: ReviewPageWorkerMessageDirective.SetHasHiddenAPIFlag,
payload: hasHiddenAPI
payload: hasHiddenAPI,
};
postMessage(hasHiddenAPIMessage);

Expand Down Expand Up @@ -203,11 +203,13 @@ function buildCodePanelRows(nodeIdHashed: string, navigationTree: NavigationTree

if (bottomTokenNode.codeLines) {
bottomTokenNode.codeLines.forEach((codeLine, index) => {
codeLine.toggleDocumentationClasses = `bi ${toggleDocumentationClassPart} hide`;
setLineNumber(codeLine);
if (buildNode) {
codePanelRowData.push(codeLine);
visibleNodes.add(codeLine.nodeIdHashed);
if (shouldAppendIfRowIsHiddenAPI(codeLine)) {
codeLine.toggleDocumentationClasses = `bi ${toggleDocumentationClassPart} hide`;
setLineNumber(codeLine);
if (buildNode) {
codePanelRowData.push(codeLine);
visibleNodes.add(codeLine.nodeIdHashed);
}
}
});
}
Expand Down Expand Up @@ -246,7 +248,7 @@ function addJustDiffBuffer() {
function shouldAppendIfRowIsHiddenAPI(row: CodePanelRowData) {
if (row.isHiddenAPI) {
hasHiddenAPI = true;
return apiTreeBuilderData?.showHiddenApis;
return apiTreeBuilderData?.showHiddenApis || codePanelData?.hasHiddenAPIThatIsDiff;
} else {
return true;
}
Expand Down

0 comments on commit b727d85

Please sign in to comment.