tbonelee commented on code in PR #5155:
URL: https://github.com/apache/zeppelin/pull/5155#discussion_r2841617779
##########
zeppelin-web-angular/src/app/pages/workspace/notebook/revisions-comparator/revisions-comparator.component.ts:
##########
@@ -10,16 +10,218 @@
* limitations under the License.
*/
-import { ChangeDetectionStrategy, Component, OnInit } from '@angular/core';
+import { DatePipe } from '@angular/common';
+import { ChangeDetectionStrategy, ChangeDetectorRef, Component, Input,
OnDestroy, OnInit } from '@angular/core';
+import { DomSanitizer, SafeHtml } from '@angular/platform-browser';
+import * as DiffMatchPatch from 'diff-match-patch';
+import { Subscription } from 'rxjs';
+
+import { NoteRevisionForCompareReceived, OP, ParagraphItem, RevisionListItem }
from '@zeppelin/sdk';
+import { MessageService } from '@zeppelin/services';
+
+interface MergedParagraphDiff {
+ paragraph: ParagraphItem;
+ firstString: string;
+ type: 'added' | 'deleted' | 'compared';
+ diff?: SafeHtml;
+ identical?: boolean;
+}
@Component({
selector: 'zeppelin-notebook-revisions-comparator',
templateUrl: './revisions-comparator.component.html',
styleUrls: ['./revisions-comparator.component.less'],
- changeDetection: ChangeDetectionStrategy.OnPush
+ changeDetection: ChangeDetectionStrategy.OnPush,
+ providers: [DatePipe]
})
-export class NotebookRevisionsComparatorComponent implements OnInit {
- constructor() {}
+export class NotebookRevisionsComparatorComponent implements OnInit, OnDestroy
{
+ @Input() noteRevisions: RevisionListItem[] = [];
+ @Input() noteId!: string;
+
+ firstNoteRevisionForCompare: NoteRevisionForCompareReceived | null = null;
+ secondNoteRevisionForCompare: NoteRevisionForCompareReceived | null = null;
+ currentFirstRevisionLabel = 'Choose...';
+ currentSecondRevisionLabel = 'Choose...';
+ mergeNoteRevisionsForCompare: MergedParagraphDiff[] = [];
+ currentParagraphDiffDisplay: MergedParagraphDiff | null = null;
+ selectedFirstRevisionId: string | null = null;
+ selectedSecondRevisionId: string | null = null;
+
+ private subscription: Subscription | null = null;
+ private dmp = new DiffMatchPatch();
+
+ get sortedRevisions(): RevisionListItem[] {
+ return [...this.noteRevisions].sort((a, b) => (b.time || 0) - (a.time ||
0));
+ }
+
+ constructor(
+ private messageService: MessageService,
+ private cdr: ChangeDetectorRef,
+ private datePipe: DatePipe,
+ private sanitizer: DomSanitizer
+ ) {}
+
+ ngOnInit(): void {
+ this.subscription = this.messageService
+ .receive(OP.NOTE_REVISION_FOR_COMPARE)
+ .subscribe((data: NoteRevisionForCompareReceived) => {
+ if (data.note && data.position) {
+ if (data.position === 'first') {
+ this.firstNoteRevisionForCompare = data;
+ } else {
+ this.secondNoteRevisionForCompare = data;
+ }
+
+ if (
+ this.firstNoteRevisionForCompare !== null &&
+ this.secondNoteRevisionForCompare !== null &&
+ this.firstNoteRevisionForCompare.revisionId !==
this.secondNoteRevisionForCompare.revisionId
+ ) {
+ this.compareRevisions();
+ }
+ this.cdr.markForCheck();
+ }
+ });
+ }
+
+ getNoteRevisionForReview(revision: RevisionListItem, position: 'first' |
'second'): void {
+ if (!revision) {
+ return;
+ }
+ if (position === 'first') {
+ this.currentFirstRevisionLabel = revision.message;
+ this.selectedFirstRevisionId = revision.id;
+ } else {
+ this.currentSecondRevisionLabel = revision.message;
+ this.selectedSecondRevisionId = revision.id;
+ }
+ this.messageService.noteRevisionForCompare(this.noteId, revision.id,
position);
+ }
+
+ onFirstRevisionSelect(revisionId: string): void {
+ const revision = this.noteRevisions.find(r => r.id === revisionId);
+ if (revision) {
+ this.getNoteRevisionForReview(revision, 'first');
+ }
+ }
+
+ onSecondRevisionSelect(revisionId: string): void {
+ const revision = this.noteRevisions.find(r => r.id === revisionId);
+ if (revision) {
+ this.getNoteRevisionForReview(revision, 'second');
+ }
+ }
+
+ onRevisionRowClick(index: number): void {
+ const sorted = this.sortedRevisions;
+ if (index < sorted.length - 1) {
+ this.getNoteRevisionForReview(sorted[index + 1], 'first');
+ this.getNoteRevisionForReview(sorted[index], 'second');
+ }
+ }
+
+ compareRevisions(): void {
+ if (!this.firstNoteRevisionForCompare ||
!this.secondNoteRevisionForCompare) {
+ return;
+ }
+ const paragraphs1 = this.firstNoteRevisionForCompare.note?.paragraphs ||
[];
+ const paragraphs2 = this.secondNoteRevisionForCompare.note?.paragraphs ||
[];
+ const merge: MergedParagraphDiff[] = [];
+
+ for (const p1 of paragraphs1) {
+ const p2 = paragraphs2.find((p: ParagraphItem) => p.id === p1.id) ||
null;
+ if (p2 === null) {
+ merge.push({
+ paragraph: p1,
+ firstString: (p1.text || '').split('\n')[0],
+ type: 'deleted'
+ });
+ } else {
+ const text1 = p1.text || '';
+ const text2 = p2.text || '';
+ const diffHtml = this.buildLineDiffHtml(text1, text2);
+ merge.push({
+ paragraph: p1,
+ diff: diffHtml.html,
+ identical: diffHtml.identical,
+ firstString: (p1.text || '').split('\n')[0],
+ type: 'compared'
+ });
+ }
+ }
+
+ for (const p2 of paragraphs2) {
+ const p1 = paragraphs1.find((p: ParagraphItem) => p.id === p2.id) ||
null;
+ if (p1 === null) {
+ merge.push({
+ paragraph: p2,
+ firstString: (p2.text || '').split('\n')[0],
+ type: 'added'
+ });
+ }
+ }
+
+ merge.sort((a, b) => {
+ const order = { added: 0, deleted: 1, compared: 2 };
+ return order[a.type] - order[b.type];
+ });
Review Comment:
I think this is a trade-off between preserving context and enabling quick
skimming.
If we had to pick one default, I’d lean toward keeping the original
paragraph order, because changes may be scattered (which costs some reading
time), but reordering the note breaks the narrative flow and makes it harder to
understand where a change happened.
The ideal UX might be: keep the original order by default, and offer a
toggle/checkbox for “show changed only” or “changed first” for people who want
a faster scan.
What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
