Skip to content

Commit 48df8a5

Browse files
calixtemancalixteman
authored
Merge pull request #20586 from marco-c/commentundo
Bug 1999154 - Add the ability to undo comment deletion
2 parents ab7d388 + bfdcaad commit 48df8a5

7 files changed

Lines changed: 259 additions & 4 deletions

File tree

‎l10n/en-US/viewer.ftl‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,7 @@ pdfjs-editor-undo-bar-message-freetext = Text removed
561561
pdfjs-editor-undo-bar-message-ink = Drawing removed
562562
pdfjs-editor-undo-bar-message-stamp = Image removed
563563
pdfjs-editor-undo-bar-message-signature = Signature removed
564+
pdfjs-editor-undo-bar-message-comment = Comment removed
564565
# Variables:
565566
# $count (Number) - the number of removed annotations.
566567
pdfjs-editor-undo-bar-message-multiple =

‎src/display/editor/comment.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,20 @@ class Comment {
314314
this.#deleted = false;
315315
}
316316

317+
/**
318+
* Restore the comment data (used for undo).
319+
* @param {Object} data - The comment data to restore.
320+
* @param {string} data.text - The comment text.
321+
* @param {string|null} data.richText - The rich text content.
322+
* @param {Date|null} data.date - The original date.
323+
*/
324+
restoreData({ text, richText, date }) {
325+
this.#text = text;
326+
this.#richText = richText;
327+
this.#date = date;
328+
this.#deleted = false;
329+
}
330+
317331
setInitialText(text, richText = null) {
318332
this.#initialText = text;
319333
this.data = text;

‎src/display/editor/editor.js‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1220,9 +1220,14 @@ class AnnotationEditor {
12201220
};
12211221
}
12221222

1223-
set comment(text) {
1223+
set comment(value) {
12241224
this.#comment ||= new Comment(this);
1225-
this.#comment.data = text;
1225+
if (typeof value === "object" && value !== null) {
1226+
// Restore full comment data (used for undo).
1227+
this.#comment.restoreData(value);
1228+
} else {
1229+
this.#comment.data = value;
1230+
}
12261231
if (this.hasComment) {
12271232
this.removeCommentButtonFromToolbar();
12281233
this.addStandaloneCommentButton();

‎src/display/editor/tools.js‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1177,6 +1177,23 @@ class AnnotationEditorUIManager {
11771177
this.#commentManager?.removeComments([editor.uid]);
11781178
}
11791179

1180+
/**
1181+
* Delete a comment from an editor with undo support.
1182+
* @param {AnnotationEditor} editor - The editor whose comment to delete.
1183+
* @param {Object} savedData - The comment data to save for undo.
1184+
*/
1185+
deleteComment(editor, savedData) {
1186+
const undo = () => {
1187+
editor.comment = savedData;
1188+
};
1189+
const cmd = () => {
1190+
this._editorUndoBar?.show(undo, "comment");
1191+
this.toggleComment(/* editor = */ null);
1192+
editor.comment = null;
1193+
};
1194+
this.addCommands({ cmd, undo, mustExec: true });
1195+
}
1196+
11801197
toggleComment(editor, isSelected, visibility = undefined) {
11811198
this.#commentManager?.toggleCommentPopup(editor, isSelected, visibility);
11821199
}

‎test/integration/comment_spec.mjs‎

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ import {
2424
highlightSpan,
2525
kbModifierDown,
2626
kbModifierUp,
27+
kbRedo,
28+
kbUndo,
2729
loadAndWait,
2830
scrollIntoView,
2931
selectEditor,
@@ -965,4 +967,213 @@ describe("Comment", () => {
965967
);
966968
});
967969
});
970+
971+
describe("Undo deletion popup for comments (bug 1999154)", () => {
972+
let pages;
973+
974+
beforeEach(async () => {
975+
pages = await loadAndWait(
976+
"tracemonkey.pdf",
977+
".annotationEditorLayer",
978+
"page-fit",
979+
null,
980+
{ enableComment: true }
981+
);
982+
});
983+
984+
afterEach(async () => {
985+
await closePages(pages);
986+
});
987+
988+
it("must check that deleting a comment can be undone using the undo button", async () => {
989+
await Promise.all(
990+
pages.map(async ([browserName, page]) => {
991+
await switchToHighlight(page);
992+
await highlightSpan(page, 1, "Abstract");
993+
const editorSelector = getEditorSelector(0);
994+
const comment = "Test comment for undo";
995+
await editComment(page, editorSelector, comment);
996+
997+
// Stay in highlight mode - don't disable it
998+
await waitAndClick(
999+
page,
1000+
`${editorSelector} .annotationCommentButton`
1001+
);
1002+
1003+
await page.waitForSelector("#commentPopup", { visible: true });
1004+
1005+
// Capture the date before deletion
1006+
const dateBefore = await page.evaluate(
1007+
() =>
1008+
document.querySelector("#commentPopup .commentPopupTime")
1009+
?.textContent
1010+
);
1011+
1012+
await waitAndClick(page, "button.commentPopupDelete");
1013+
1014+
await page.waitForSelector("#editorUndoBar", { visible: true });
1015+
await page.waitForSelector("#editorUndoBarUndoButton", {
1016+
visible: true,
1017+
});
1018+
await page.click("#editorUndoBarUndoButton");
1019+
1020+
// Check that the comment is restored by hovering to show the popup
1021+
await page.hover(`${editorSelector} .annotationCommentButton`);
1022+
await page.waitForSelector("#commentPopup", { visible: true });
1023+
const popupText = await page.evaluate(
1024+
() =>
1025+
document.querySelector("#commentPopup .commentPopupText")
1026+
?.textContent
1027+
);
1028+
expect(popupText).withContext(`In ${browserName}`).toEqual(comment);
1029+
1030+
// Check that the date is preserved
1031+
const dateAfter = await page.evaluate(
1032+
() =>
1033+
document.querySelector("#commentPopup .commentPopupTime")
1034+
?.textContent
1035+
);
1036+
expect(dateAfter)
1037+
.withContext(`In ${browserName}`)
1038+
.toEqual(dateBefore);
1039+
})
1040+
);
1041+
});
1042+
1043+
it("must check that the undo deletion popup displays 'Comment removed' message", async () => {
1044+
await Promise.all(
1045+
pages.map(async ([browserName, page]) => {
1046+
await switchToHighlight(page);
1047+
await highlightSpan(page, 1, "Abstract");
1048+
const editorSelector = getEditorSelector(0);
1049+
await editComment(page, editorSelector, "Test comment");
1050+
1051+
// Stay in highlight mode - don't disable it
1052+
await waitAndClick(
1053+
page,
1054+
`${editorSelector} .annotationCommentButton`
1055+
);
1056+
1057+
await page.waitForSelector("#commentPopup", { visible: true });
1058+
await waitAndClick(page, "button.commentPopupDelete");
1059+
1060+
await page.waitForFunction(() => {
1061+
const messageElement = document.querySelector(
1062+
"#editorUndoBarMessage"
1063+
);
1064+
return messageElement && messageElement.textContent.trim() !== "";
1065+
});
1066+
const message = await page.waitForSelector("#editorUndoBarMessage");
1067+
const messageText = await page.evaluate(
1068+
el => el.textContent,
1069+
message
1070+
);
1071+
expect(messageText)
1072+
.withContext(`In ${browserName}`)
1073+
.toContain("Comment removed");
1074+
})
1075+
);
1076+
});
1077+
1078+
it("must check that the undo bar closes when clicking the close button", async () => {
1079+
await Promise.all(
1080+
pages.map(async ([browserName, page]) => {
1081+
await switchToHighlight(page);
1082+
await highlightSpan(page, 1, "Abstract");
1083+
const editorSelector = getEditorSelector(0);
1084+
await editComment(page, editorSelector, "Test comment");
1085+
1086+
// Stay in highlight mode - don't disable it
1087+
await waitAndClick(
1088+
page,
1089+
`${editorSelector} .annotationCommentButton`
1090+
);
1091+
1092+
await page.waitForSelector("#commentPopup", { visible: true });
1093+
await waitAndClick(page, "button.commentPopupDelete");
1094+
1095+
await page.waitForSelector("#editorUndoBar", { visible: true });
1096+
await waitAndClick(page, "#editorUndoBarCloseButton");
1097+
await page.waitForSelector("#editorUndoBar", { hidden: true });
1098+
})
1099+
);
1100+
});
1101+
1102+
it("must check that deleting a comment can be undone using Ctrl+Z", async () => {
1103+
await Promise.all(
1104+
pages.map(async ([browserName, page]) => {
1105+
await switchToHighlight(page);
1106+
await highlightSpan(page, 1, "Abstract");
1107+
const editorSelector = getEditorSelector(0);
1108+
const comment = "Test comment for Ctrl+Z undo";
1109+
await editComment(page, editorSelector, comment);
1110+
1111+
// Stay in highlight mode - don't disable it
1112+
await waitAndClick(
1113+
page,
1114+
`${editorSelector} .annotationCommentButton`
1115+
);
1116+
1117+
await page.waitForSelector("#commentPopup", { visible: true });
1118+
await waitAndClick(page, "button.commentPopupDelete");
1119+
1120+
await page.waitForSelector("#editorUndoBar", { visible: true });
1121+
1122+
// Use Ctrl+Z to undo
1123+
await kbUndo(page);
1124+
1125+
// The undo bar should be hidden after undo
1126+
await page.waitForSelector("#editorUndoBar", { hidden: true });
1127+
1128+
// Check that the comment is restored by hovering to show the popup
1129+
await page.hover(`${editorSelector} .annotationCommentButton`);
1130+
await page.waitForSelector("#commentPopup", { visible: true });
1131+
const popupText = await page.evaluate(
1132+
() =>
1133+
document.querySelector("#commentPopup .commentPopupText")
1134+
?.textContent
1135+
);
1136+
expect(popupText).withContext(`In ${browserName}`).toEqual(comment);
1137+
})
1138+
);
1139+
});
1140+
1141+
it("must check that the comment popup is hidden after redo", async () => {
1142+
await Promise.all(
1143+
pages.map(async ([browserName, page]) => {
1144+
await switchToHighlight(page);
1145+
await highlightSpan(page, 1, "Abstract");
1146+
const editorSelector = getEditorSelector(0);
1147+
const comment = "Test comment for redo";
1148+
await editComment(page, editorSelector, comment);
1149+
1150+
// Show the popup by clicking the comment button
1151+
await waitAndClick(
1152+
page,
1153+
`${editorSelector} .annotationCommentButton`
1154+
);
1155+
await page.waitForSelector("#commentPopup", { visible: true });
1156+
1157+
// Delete the comment
1158+
await waitAndClick(page, "button.commentPopupDelete");
1159+
await page.waitForSelector("#editorUndoBar", { visible: true });
1160+
1161+
// Undo the deletion
1162+
await kbUndo(page);
1163+
await page.waitForSelector("#editorUndoBar", { hidden: true });
1164+
1165+
// Show the popup again by clicking the comment button
1166+
await waitAndClick(
1167+
page,
1168+
`${editorSelector} .annotationCommentButton`
1169+
);
1170+
await page.waitForSelector("#commentPopup", { visible: true });
1171+
1172+
// Redo the deletion - popup should be hidden
1173+
await kbRedo(page);
1174+
await page.waitForSelector("#commentPopup", { hidden: true });
1175+
})
1176+
);
1177+
});
1178+
});
9681179
});

‎web/comment_manager.js‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -982,9 +982,15 @@ class CommentPopup {
982982
},
983983
},
984984
});
985-
this.#editor.comment = null;
986-
this.#editor.focus();
985+
const editor = this.#editor;
986+
const savedData = editor.comment;
987987
this.destroy();
988+
if (savedData?.text) {
989+
editor._uiManager.deleteComment(editor, savedData);
990+
} else {
991+
editor.comment = null;
992+
}
993+
editor.focus();
988994
});
989995
del.addEventListener("contextmenu", noContextMenu);
990996
buttons.append(edit, del);

‎web/editor_undo_bar.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ class EditorUndoBar {
4040
stamp: "pdfjs-editor-undo-bar-message-stamp",
4141
ink: "pdfjs-editor-undo-bar-message-ink",
4242
signature: "pdfjs-editor-undo-bar-message-signature",
43+
comment: "pdfjs-editor-undo-bar-message-comment",
4344
_multiple: "pdfjs-editor-undo-bar-message-multiple",
4445
});
4546

0 commit comments

Comments
 (0)